Skip to content

Share chart button#7802

Draft
emilykl wants to merge 8 commits into
masterfrom
upload-to-cloud-proto
Draft

Share chart button#7802
emilykl wants to merge 8 commits into
masterfrom
upload-to-cloud-proto

Conversation

@emilykl
Copy link
Copy Markdown
Contributor

@emilykl emilykl commented May 14, 2026

@emilykl emilykl force-pushed the upload-to-cloud-proto branch from a487ce9 to 100da7b Compare May 14, 2026 17:16
@emilykl emilykl changed the title draft Share chart button May 20, 2026
@marthacryan marthacryan marked this pull request as ready for review May 28, 2026 20:38
@marthacryan marthacryan marked this pull request as draft May 28, 2026 20:40
@marthacryan marthacryan self-assigned this May 28, 2026

dialog.append('div')
.classed('plotly-cloud-dialog-title', true)
.text('Share with Plotly Cloud');
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All of these strings should (eventually) be wrapped in the localization function (search for Lib._( or _( to find where this is done elsewhere in the codebase)

Comment on lines +276 to +279
'Should we include a modebar button that sends this chart to a URL',
'specified by `plotlyServerURL`, for sharing the chart with others?',
'Note that this button can (depending on `plotlyServerURL` being set)',
'send your data to an external server.'
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'Should we include a modebar button that sends this chart to a URL',
'specified by `plotlyServerURL`, for sharing the chart with others?',
'Note that this button can (depending on `plotlyServerURL` being set)',
'send your data to an external server.'
'Should we include a modebar button that sends this chart to a URL',
'specified by `plotlyServerURL`, for sharing the chart with others?',
'Note that this button will (after a confirmation step)',
'send chart data to an external server.'

Comment thread src/plots/plots.js
.style('display', 'none');
// Build the request body: the chart JSON plus the plotly.js version used to
// generate it, so Cloud can host the chart with a compatible plotly.js version.
var chart = JSON.parse(plots.graphJson(gd, false, 'keepdata'));
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can avoid serializing and de-serializing by doing this I think

Suggested change
var chart = JSON.parse(plots.graphJson(gd, false, 'keepdata'));
var chart = plots.graphJson(gd, false, 'keepdata', 'object');

Comment thread src/plots/plots.js
var cloudWindow = window.open(baseUrl, '_blank');
if(!cloudWindow) {
console.error('Unable to open Plotly Cloud (the popup may have been blocked)');
gd.emit('plotly_afterexport');
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May not want to emit 'plotly_afterexport' in the failure case... maybe a new event, 'plotly_exportfail' or something?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants