Skip to content
This repository was archived by the owner on Jun 3, 2024. It is now read-only.

Graph config defaults don't match plotly.js #514

Closed
alexcjohnson opened this issue Apr 5, 2019 · 4 comments
Closed

Graph config defaults don't match plotly.js #514

alexcjohnson opened this issue Apr 5, 2019 · 4 comments
Labels
dash-type-bug Something isn't working as intended

Comments

@alexcjohnson
Copy link
Collaborator

Graph sets its own config defaults here - but this has no mechanism to keep up to date with plotly.js, and in fact it's out of date already in at least one respect, scrollZoom, as of plotly/plotly.js#3422

We should either remove those defaults entirely, or programmatically match them to plotly.js.

There's a related problem: if you override a piece of the config, the rest of the default gets dropped. So for example right now if you were to set something unrelated like showLink: true, the scrollZoom: false would be dropped - and set to the new plotly.js default!

cc @nicolaskruchten @michaelbabyn - discovered while researching plotly/plotly.js#3738

@alexcjohnson alexcjohnson added the dash-type-bug Something isn't working as intended label Apr 5, 2019
@nicolaskruchten
Copy link
Contributor

Cc @wbrgss

@alexcjohnson
Copy link
Collaborator Author

Did a little investigation of the current Graph.defaultProps values vs plotly.js:

  • Only one value is provided but does NOT match plotly.js, the aforementioned scrollZoom.
  • A whole bunch of entries aren't present at all:
    • plotlyServerURL
    • responsive
    • showSendToCloud
    • toImageButtonOptions
    • watermark
    • setBackground
    • logging
    • globalTransforms
    • locale
    • locales

For the moment I'll make a PR to remove the defaults and add documentation for the new entries. Later - unless it's quick and I can get it in for the upcoming release - I'll make another PR to test that these keys continue to match when we upgrade plotly.js.

@alexcjohnson
Copy link
Collaborator Author

Actually I'm not going to include setBackground, logging, or globalTransforms - these either won't work or aren't serializable, if you wanted to use them you'd need to subclass Graph.

@alexcjohnson
Copy link
Collaborator Author

Closing - #515 fixes the bug by removing the nested defaults, and includes a test that propTypes for Graph has the right keys from the plotly.js schema.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
dash-type-bug Something isn't working as intended
Projects
None yet
Development

No branches or pull requests

2 participants