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

Color picker – Add full gradient support #497

Merged
merged 98 commits into from
Mar 11, 2020

Conversation

barklund
Copy link
Contributor

@barklund barklund commented Mar 5, 2020

Full gradient support added in this PR.

With full test coverage for color picker, color input and all color-related util functions!

See it in action here: https://youtu.be/V3qsoONXUwk

Fully fixes #263.

@barklund barklund changed the title Feature/263 gradient implementation Color picker – Add full gradient support Mar 5, 2020
@barklund
Copy link
Contributor Author

barklund commented Mar 9, 2020

Replying to various comments by @spacedmonkey:

Weird thing I noticed, I couldn't undo gradient changes. As in, I made 5 points in the gradient when I pressed undo, all the changes were reverted. Is this expected?

Works for me. I can undo and redo each separate gradient change just fine - done with mouse or keyboard. Basically, if the actual page layout updates when you change any individual thing, each individual change will always create an entry in the history. It simply can't not happen.

I noticed that the arrow keys do not work, as it doesn't matter what directory, the marker always goes left.

Well spotted.

I am also going to post my thoughts I posted on the gutenberg gradient picker as they are very similar in design and functionality. WordPress/gutenberg#17603 (comment)

You have many separate comments there, trying to address them individually:

Should we add more testing to this? Maybe some basic e2e or intergration tests?

I want to add a lot more testing to the color picker, but it's very time intensive. I am working on it, though. It'll be mostly integration testing for this complex component.

@spacedmonkey for further comments and discussion, please if at all possible make them inline, as it's so much easier to have a conversation going in an inline thread rather than in disjoint comments on the main PR. I have linked to all relevant parts of the PR inline above. Thanks 😃

@barklund barklund changed the base branch from feature/263-gradient-picker to master March 10, 2020 01:32
Copy link
Collaborator

@swissspidy swissspidy left a comment

Choose a reason for hiding this comment

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

Impressive work! 💪

Edit: Just noticed a small thing while looking at another PR, but I'd say this should be handled in a new PR.

Let's get this beast in first!

@swissspidy swissspidy merged commit 4940f42 into master Mar 11, 2020
@swissspidy swissspidy deleted the feature/263-gradient-implementation branch March 11, 2020 09:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement New feature or improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Color Picker - Gradient Editing
5 participants