Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable plotly server URL by default #4690

Merged
merged 5 commits into from
Mar 26, 2020
Merged

Conversation

archmoj
Copy link
Contributor

@archmoj archmoj commented Mar 26, 2020

Resolves #4689
This PR changes the default value for config.plotlyServerURL option to empty string in 514aef4.
Please also note that empty string would be ignored in Plotly.sendDataToCloud in f661184,
Therefore in order to Edit in Chart Studio (using modebar buttons editInChartStudio or sendDataToCloud) the config.plotlyServerURL is required.

@plotly/plotly_js

@archmoj archmoj added this to the v1.53.0 milestone Mar 26, 2020
'and the showLink/sendData on-graph link'
'When set it determines base URL for',
'the \'Edit in Chart Studio\' (aka sendDataToCloud) mode bar button',
'and the showLink/sendData on-graph link.'
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we at least include the url for CSC in the description? I guess that should be https://chart-studio.plotly.com? Something like "To enable sending your data to Plotly's public cloud, you need to set both plotlyServerURL to 'https://chart-studio.plotly.com' and showSendToCloud to true"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call. Done in 7d5492d using https://plotly.com

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Extra info added in 9959cb8.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks - I'd take https://plotly.com out, since I believe that's not going to work, and just include https://chart-studio.plotly.com

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's not use https://plotly.com as this will confuse people, let's only use https://chart-studio.plotly.com as the example.

Let's also not call it "Plotly's public cloud" but rather "Chart Studio Cloud" please.

Finally, let's replace sendDataToCloud (which is not one of the options!) with showEditInChartStudio/showSendToCloud which are slight variants on each other...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Revised in d81e598.

@nicolaskruchten
Copy link
Contributor

Therefore in order to Edit in Chart Studio the config.plotlyServerURL is required.

Yes please.

Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

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

💃 Thanks!

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.

Change default value of config.plotlyServerURL to ""
3 participants