-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Add color field type and control to DataViews #71522
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
| if ( | ||
| ! [ undefined, '', null ].includes( value ) && | ||
| ! colord( value ).isValid() | ||
| ) { | ||
| return __( 'Value must be a valid color.' ); | ||
| } |
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 add the color type to the validation story as well?
oandregal
left a comment
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.
I've left a couple of comments, other than that, it's ready. Thanks!
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
0fa48f4 to
0b2d829
Compare
|
Size Change: +3.11 kB (+0.16%) Total Size: 1.93 MB
ℹ️ View Unchanged
|
- Add new color field type with validation using colord library - Implement color control with text input and visual color picker - Support alpha channel colors (rgba, hsla) - Use proper WordPress component patterns with private APIs - Fix spacing and alignment issues for better visual integration - Add comprehensive Storybook stories for testing - Add colord dependency to dataviews package
Co-authored-by: André <583546+oandregal@users.noreply.github.com>
0b2d829 to
28bc9da
Compare
Co-authored-by: jorgefilipecosta <jorgefilipecosta@git.wordpress.org> Co-authored-by: oandregal <oandregal@git.wordpress.org>
| ValidatedTextControl, | ||
| ValidatedToggleControl, | ||
| } from './validated-form-controls'; | ||
| import { Picker } from './color-picker/picker'; |
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.
Any reason why we had to expose this, instead of just reusing ColorPicker, or maybe even using <input type="color">? It looks off because of missing styles, too. I don't think we should be exporting random subcomponents like this with no plan, it's not tenable.
| Picker in popover | ColorPicker |
|---|---|
![]() |
![]() |
There were some layout and focus style issues on the color indicator button I wanted to report, but the approach would differ if we were to use an <input type="color"> instead. Was @WordPress/gutenberg-design involved in the spec of this control? I think we should be more deliberate in the design here, since it's going to be a standard color input for the system. cc @WordPress/gutenberg-components
There is also already a similar design in BorderControl:
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.
I think I saw some mockups like this but never a full spec. Leveraging the native input is worth exploring, but I think we'd still need our own UI to support things like Alpha, unless we had a separate input for that?


Adds a new
colorfield type and control to DataViews that allows users to input color values via text input and visual color picker.How
Screenshot
Testing
Dependencies
colorddependency to dataviews package (already used on other parts of gutenberg) for color validation and manipulation