Add custom gradient component #17603

jorgefilipecosta · 2019-09-26T10:00:28Z

Description

Issued discussing the design: #16662

This PR is a WIP with the objective of implementing a custom gradient component.

The objective is to get something similar to:

But for now, we are not yet trying to get that design pixel perfect it is more important to test the interactions.

The small gradient component should be able to:

Add new control points in a given position with a given color.
Change the color of the control point in a gradient.
Remove a gradient control point.
Move a control point position e.g: from 0% to 20%.

This version misses the ability to move a control point. The implementation is getting complex and I would like some intermediary feedback about this path before implementing it. The idea I have is simply being able to drag the control points.

Besides this small visual component we also need inputs to select between radial and linear gradients and the angle. My idea is to use our existing input components for that. We will also probably need to allow to edit the control points in a normal input for a11y reasons, using drag & drop is not accessible to everyone.

For now, I would like some feedback on what we have and thoughts if we need to change direction.

The code is not polished yet.

How has this been tested?

I added a button.
I tried to create some gradients using the custom gradients bar.

Screenshots

melchoyce · 2019-10-01T16:34:45Z

Only thing that feels weird is that you can select both a background color, and a gradient. Maybe we could consider adding a toggle that has you choose to either use a solid color, or a gradient:

(Please excuse the giant toggles, I threw this together in a couple minutes)

joyously · 2019-10-01T17:43:34Z

I think that since the gradient colors can be semi-transparent, it makes sense to be able to choose a background color and a gradient.

enriquesanchez · 2019-10-01T21:24:00Z

I agree with @melchoyce. I think that there should be a toggle to choose between solid or gradient.

Some screen from popular design apps for inspiration:

Figma

Sketch

I don't think we need to go as far as having a dropdown or long list of options here because we wouldn't have this many options. A toggle should suffice.

jorgefilipecosta · 2019-11-25T13:12:48Z

Hi @mtias,
Do you consider the tabs between solid color and gradient a blocker or we can try to land this custom gradient UI and then try to have a tabbed design?

mcsf

I still need to review in more detail, but here's some rough initial feedback.

Product-wise, it feels nice, and I think we can iterate from here.
It's not fully keyboard accessible, but I also think we can iterate afterwards, as this PR is already large.

mcsf · 2019-11-29T16:08:00Z

packages/components/src/custom-gradient-picker/control-points.js

+				return [];
+			}
+			return map( parsedGradient.colorStops, ( colorStop ) => {
+				if ( ! colorStop || ! colorStop.length || colorStop.length.type !== '%' ) {


colorStop.length.type !== '%'

Is this right?

Edit: Ah, I just saw getGradientWithColorStopAdded, gotcha. I do feel, though, that there is a lot of new domain to assimilate across these files. Maybe it's a necessary evil — in which case we could create some new JSDoc types and document a bit — maybe it's something we can reduce.

This structure comes from the gradient-parser package which provides an AST for gradients. I agree it is not easy to understand the structure. I added a link to the package where the structure is documented. I made sure all the functions that need knowledge about this structure are defined on the same fil. I added JSDOCS to getMarkerPoints function which returns an array derived from this structure. I hope these changes make things simpler to understand.

packages/components/src/custom-gradient-picker/serializer.js

jorgefilipecosta · 2019-12-04T13:02:01Z

Hi @mcsf, I tried to address your review. I also did a refactor to the components to simplify its logic using a reduce/state machine-based approach. I hope these changes make the code easier to review.

mtias · 2019-12-04T18:46:52Z

@jorgefilipecosta I would like to avoid this to hang in a few plugin releases (lack of tabbed interface) if we don't address it soon as it adds significant weight to the inspector settings.

mcsf

This is a pretty big PR and I can't review it thoroughly! But I've done a few passes and it looks good overall. The items I raised need to be addressed, but after that go ahead and merge so that we can iterate more quickly. The API itself for CustomGradientPicker is small and nice. :)

mcsf · 2019-12-04T21:45:33Z

packages/components/src/custom-gradient-picker/utils.js

+}
+
+/**
+ * Get the connection state.


Stray comment? :)

packages/components/src/custom-gradient-picker/utils.js

packages/components/src/custom-gradient-picker/control-points.js

mcsf · 2019-12-05T18:21:34Z

packages/components/src/custom-gradient-picker/control-points.js

+	const onMouseUp = () => {
+		if ( window && window.removeEventListener ) {
+			window.removeEventListener( 'mousemove', onMouseMove );
+			window.removeEventListener( 'mouseup', onMouseUp );


Should we also remove the listeners on unmount? Relying only on a pending mouseup event to take care of the cleanup feels fragile.

mcsf · 2019-12-05T19:03:21Z

To address separately:

Mispositioned popover (Firefox Beta, macOS stable).

Squashed commits: [4ee9051258] Avoid moving over other control points [479d199cfe] Add mechanism that allows themes disabling custom gradients. [37bc638743] Polish gradients further, and the gradient stops. Polish some of the gradient stop dimensions. Tweak the gradient presets to have fewer in the same hue. [9778bef873] Polish, better gradients, better spacing. This commit improves a few things: - It better spaces the items, so that there can be 6 swatches on a line. And works both with and without scrollbar. - It polishes the gradient customizer so that the handles are perfectly placed in the curves of the slider. - It removes a few of the more muddy gradient presets. [0c23c3d962] Improved a11y [c0fd4d6b97] Code refactor (+2 squashed commits) Squashed commits: [cc33664] Refactor and improvements [f044457] Add: Custom gradient component

Co-Authored-By: Miguel Fonseca <miguelcsf@gmail.com>

jorgefilipecosta · 2019-12-05T23:00:03Z

Thank you for the reviews @mcsf.
I merged the component so we can iterate, add more functionality to it fix problems, etc in different PR's, but before merging I removed its usage by default. So even though the PR is merged and the component is available it is not visible/used yet. On can easily make the component visible by reverting dbb5f8b.
I plan to make the component visible when we land the tabbed UI suggested by Matias.

youknowriad · 2019-12-09T12:11:45Z

A README and some stories for the component would be appreciated :)

youknowriad · 2019-12-09T12:13:40Z

Should this component be named just GradientPicker (equivalent to the existing ColorPicker)

jorgefilipecosta · 2019-12-09T12:19:12Z

A README and some stories for the component would be appreciated :)

Hi @youknowriad,
We have a GradientPicker component that will contain a picker for named gradients and custom ones. I named this component CustomGradientPicker because it will focus only on the UI to create/edit a custom gradient.

jorgefilipecosta requested review from adamsilverstein, aduth, ajitbohra, atimmer, chrisvanpatten, daniloercoli, ellatrix, etoledom, gziolo, nerrad, noisysocks, oandregal, ntwb, SergioEstevao, Soean, swissspidy, talldan, TimothyBJacobs and youknowriad as code owners September 26, 2019 10:00

jorgefilipecosta force-pushed the add/preset-gradient-functionality-button-block branch from babd035 to fdd3f52 Compare September 26, 2019 10:04

jorgefilipecosta removed request for adamsilverstein, TimothyBJacobs, ellatrix and etoledom September 26, 2019 10:07

mapk added the Needs Design Feedback Needs general design feedback. label Oct 1, 2019

afercia added the Needs Accessibility Feedback Need input from accessibility label Oct 1, 2019

jorgefilipecosta force-pushed the add/custom-gradient-component branch from c24228f to f044457 Compare October 1, 2019 20:30

jorgefilipecosta force-pushed the add/custom-gradient-component branch 2 times, most recently from 835598f to 31588dc Compare November 25, 2019 12:11

mcsf reviewed Nov 29, 2019

View reviewed changes

jorgefilipecosta force-pushed the add/custom-gradient-component branch 2 times, most recently from a44e038 to 6455760 Compare December 4, 2019 12:47

mcsf approved these changes Dec 5, 2019

View reviewed changes

jorgefilipecosta and others added 7 commits December 5, 2019 22:18

Refactor to state based

4a51675

Add tests to the gradient serializer

09a6ceb

Update packages/components/src/custom-gradient-picker/control-points.js

c6d2ae7

Co-Authored-By: Miguel Fonseca <miguelcsf@gmail.com>

Update packages/components/src/custom-gradient-picker/control-points.js

e6e40ed

Co-Authored-By: Miguel Fonseca <miguelcsf@gmail.com>

Clean up listeners on unmout

7c6c5c0

Revert include custom gradient picker on gradient picker component

dbb5f8b

jorgefilipecosta force-pushed the add/custom-gradient-component branch from a6bb970 to dbb5f8b Compare December 5, 2019 22:22

jorgefilipecosta merged commit 5e68cdc into master Dec 5, 2019

jorgefilipecosta deleted the add/custom-gradient-component branch December 5, 2019 22:56

youknowriad added this to the Gutenberg 7.1 milestone Dec 9, 2019

jorgefilipecosta mentioned this pull request Feb 4, 2020

Backport PR's to WordPress core (5.4 Release) #20035

Closed

7 tasks

jorgefilipecosta added the Backport to WP 6.6 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Feb 4, 2020

jorgefilipecosta removed the Backport to WP 6.6 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Feb 12, 2020

spacedmonkey mentioned this pull request Mar 9, 2020

Color picker – Add full gradient support GoogleForCreators/web-stories-wp#497

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add custom gradient component #17603

Add custom gradient component #17603

jorgefilipecosta commented Sep 26, 2019 •

edited

Loading

melchoyce commented Oct 1, 2019

joyously commented Oct 1, 2019

enriquesanchez commented Oct 1, 2019 •

edited

Loading

jorgefilipecosta commented Nov 25, 2019

mcsf left a comment

mcsf Nov 29, 2019

jorgefilipecosta Dec 4, 2019

jorgefilipecosta commented Dec 4, 2019

mtias commented Dec 4, 2019

mcsf left a comment

mcsf Dec 4, 2019

mcsf Dec 5, 2019

mcsf commented Dec 5, 2019

jorgefilipecosta commented Dec 5, 2019

youknowriad commented Dec 9, 2019

youknowriad commented Dec 9, 2019

jorgefilipecosta commented Dec 9, 2019 •

edited

Loading

Add custom gradient component #17603

Add custom gradient component #17603

Conversation

jorgefilipecosta commented Sep 26, 2019 • edited Loading

Description

How has this been tested?

Screenshots

melchoyce commented Oct 1, 2019

joyously commented Oct 1, 2019

enriquesanchez commented Oct 1, 2019 • edited Loading

jorgefilipecosta commented Nov 25, 2019

mcsf left a comment

Choose a reason for hiding this comment

mcsf Nov 29, 2019

Choose a reason for hiding this comment

jorgefilipecosta Dec 4, 2019

Choose a reason for hiding this comment

jorgefilipecosta commented Dec 4, 2019

mtias commented Dec 4, 2019

mcsf left a comment

Choose a reason for hiding this comment

mcsf Dec 4, 2019

Choose a reason for hiding this comment

mcsf Dec 5, 2019

Choose a reason for hiding this comment

mcsf commented Dec 5, 2019

jorgefilipecosta commented Dec 5, 2019

youknowriad commented Dec 9, 2019

youknowriad commented Dec 9, 2019

jorgefilipecosta commented Dec 9, 2019 • edited Loading

jorgefilipecosta commented Sep 26, 2019 •

edited

Loading

enriquesanchez commented Oct 1, 2019 •

edited

Loading

jorgefilipecosta commented Dec 9, 2019 •

edited

Loading