-
-
Notifications
You must be signed in to change notification settings - Fork 114
Colorpicker adjustments #600
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
Conversation
1c223d9 to
f1e9d21
Compare
dev/App.js
Outdated
| import 'brace/mode/json'; | ||
| import 'brace/theme/textmate'; | ||
| import {categoryLayout, traceTypes, chartCategory} from 'lib/traceTypes'; | ||
| import 'brace/theme/textmate'; // |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
empty comment...
src/components/fields/Colorscale.js
Outdated
|
|
||
| return ( | ||
| <Field {...this.props}> | ||
| <Field {...adjustedProps}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you just do <Field {...this.props} label={false}>?
I think if a props is defined twice, only the last one counts, so you can overwrite with an empty.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's nice, didn't think of that one thanks!
|
ok, didn't play with it, but code-wise - 💃 |
|
Hey @jackparmer I just updated the react-colorscales project page, to include all api changes: I changed the look of the colorPicker to adapt to small UIs, but I have only extended the api, so it should be ok for the projects using it. I've put up a few screenshots above, and deployed to gh-pages: Let me know if you'd like any adjustments :) |
|
Hey, cool! I love it! Only nit that caught me eye is |
|
I'm a bit confused by the UX of the new picker here... There's no clear indication of what the active scale is, or of how to "close" the picker, or why "reset" is there... I think I like this layout but I preferred the previous setup where the active scale is always present above, and the panel opens when you're fiddling with it and it updates the activate scale, and then you can close the panel when you're done fiddling. |
a317df4 to
b716024
Compare
|
ok I corrected titles, they're small caps now, just for a consistent look. @nicolaskruchten , I added back the toggle that you liked and on another UX pass, as you said, we could think of a more discoverable way to make these controls open/close. |
27f3196 to
a3159e1
Compare
| COLOR_PICKER_CONSTANTS.COLORSCALE_DESCRIPTIONS[selectedColorscaleType]; | ||
| const colorscaleOptions = COLOR_PICKER_CONSTANTS.COLORSCALE_TYPES.map( | ||
| type => ({ | ||
| label: type + ' scale', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually now that I look at it, could we make this scales with an s please? :)
| super(); | ||
|
|
||
| this.state = { | ||
| selectedColorscaleType: 'sequential', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
note: in a group of traces, this default makes less sense, and should be overrideable to categorical
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup, will look at how to select the initial state better in a subsequent pr..
but the structure is there to add it easily now :)
|
ok @nicolaskruchten did the latest color/swatch number modifications |
3c856d0 to
5d34d91
Compare
|
💃 |
5d34d91 to
e860bb2
Compare




new color picker

demo app in react-colorscales project
