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

852 color widget #4437

Merged
merged 7 commits into from
Oct 25, 2020
Merged

852 color widget #4437

merged 7 commits into from
Oct 25, 2020

Conversation

felixboet
Copy link
Contributor

@felixboet felixboet commented Oct 11, 2020

Fixes #852
A basic color widget - it's a version of my color-picker-widget reduced to just use the "chrome" picker, demo here (field 1&2). I could also include more or all color pickers, but I think it's nice to keep it simple as the standard color picker.

It uses react-color for the color picker and validate-color to validate the string for the color swatch and display a "?" if it's not a valid color string.

netlify-cms-widget-colorpicker-3

@felixboet felixboet requested a review from a team October 11, 2020 11:40
@erezrokah erezrokah added the type: feature code contributing to the implementation of a feature and/or user facing functionality label Oct 11, 2020
@erezrokah erezrokah marked this pull request as draft October 11, 2020 11:55
@erezrokah erezrokah marked this pull request as ready for review October 25, 2020 15:12
Copy link
Contributor

@erezrokah erezrokah left a comment

Choose a reason for hiding this comment

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

@felixboet this is amazing. Works very well.
Thank you for scoping it down for simplicity.

My changes were mostly about extracting some divs into smaller components to make the render method shorter.

Also, added very small clarifications to the docs.

@erezrokah erezrokah merged commit fed0e82 into decaporg:master Oct 25, 2020
@felixboet
Copy link
Contributor Author

thanks @erezrokah ... much cleaner that way :)

I realised one little thing from my code I am not happy with. Clicks outside the color picker are handled by the fullscreen "ClickOutsideDiv" to close the color picker. That means if you opened the color picker and want to edit another field, you have to click twice, once for closing the picker, and then for the other field. That is not consistent with other widgets like "select" or "datetime", where you can click immediately from one field to another.

I guess it's quite easy, but I don't have the experience for a quick solution... Would you like to take care of that?

@erezrokah
Copy link
Contributor

Would you like to take care of that?

Sure, how about opening a new issue for that so we can track it?

@felixboet
Copy link
Contributor Author

... and I think the widget has to be renamed, because netlify-cms-widget-color already exists ... My proposal: netlify-cms-widget-colorstring / ColorString

@erezrokah
Copy link
Contributor

netlify-cms-widget-color

Damn, forgot about that. Thank you for reminding me.
There are a few already:
https://www.npmjs.com/search?q=netlify-cms-widget-color

so netlify-cms-widget-colorstring makes sense thank you

@felixboet
Copy link
Contributor Author

Would you like to take care of that?

Sure, how about opening a new issue for that so we can track it?

Ah, you merged already. Awesome :) ... ok, i open an issue

vladdu pushed a commit to vladdu/netlify-cms that referenced this pull request Jan 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: feature code contributing to the implementation of a feature and/or user facing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Color type
2 participants