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

[bugfix] Fix color scheme picker #5891

Merged
merged 2 commits into from
Sep 18, 2018
Merged

Conversation

kristw
Copy link
Contributor

@kristw kristw commented Sep 13, 2018

Support dynamic choices by passing function instead of value to fix #5889
This will ensure that the ColorSchemeControl will always get the latest schemes registered to ColorSchemeManager

@mistercrunch @williaster @graceguo-supercat

@kristw kristw changed the title Fix color scheme picker [bugfix] Fix color scheme picker Sep 13, 2018
this.props.schemes[defaultProps.value];
const { schemes } = this.props;
const schemeLookup = _.isFunction(schemes) ? schemes() : schemes;
const currentScheme = schemeLookup[key.value || defaultProps];
Copy link
Contributor

Choose a reason for hiding this comment

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

should this be defaultProps.value?

Copy link
Contributor

@williaster williaster left a comment

Choose a reason for hiding this comment

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

LGTM after resolving the defaultProps vs defaultProps.value question.

@kristw
Copy link
Contributor Author

kristw commented Sep 18, 2018

Nice catch. Thanks @williaster .

@williaster williaster merged commit 14de28a into apache:master Sep 18, 2018
@kristw kristw deleted the kristw-color-picker branch September 18, 2018 19:04
betodealmeida pushed a commit to lyft/incubator-superset that referenced this pull request Oct 12, 2018
* Fix color scheme picker

* fix .value
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.28.0 labels Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.28.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Empty "Color Scheme" dropdown
3 participants