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

GradientPicker: improvements wishlist #42994

Open
ciampo opened this issue Aug 4, 2022 · 9 comments
Open

GradientPicker: improvements wishlist #42994

ciampo opened this issue Aug 4, 2022 · 9 comments
Assignees
Labels
[Feature] UI Components Impacts or related to the UI component system [Package] Components /packages/components [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended [Type] Enhancement A suggestion for improvement.

Comments

@ciampo
Copy link
Contributor

ciampo commented Aug 4, 2022

This is a wish list of improvements for the GradientPicker component (& related components) that I came up with while fixing popover regressions:

Slow rendering performance

This can be observed especially when dragging a control point in the editor:

Kapture.2022-08-04.at.22.59.30.mp4

Improve the initial state when there's no gradient set

  • The semi-transparency given to the whole gradient bar is weird, especially when clicking on an insertion point: in that case, the popover is also semi transparent, and is rendered below the rest of the controls (this is a side-effect caused by the opacity rule, which creates a new stacking context for the gradient bar)
  • I also find it weird that the "no gradient set" state is represented by the value of the gradient being set to the default one — in my mind, that value should be undefined (or null), but definitely not a gradient. The visual state of the component should be also adjusted to clearly show that there's no gradient being set
gradient-picker-initial-state.mp4

Done as part of #49146

Get rid of __experimentalHasMultipleOrigins prop

We should agree on the best way to implement this feature, and remove the experimental prop. I'm personally not a fan of props like __experimentalHasMultipleOrigins that change the meaning of other props (in this case, gradients). Maybe we don't need __experimentalHasMultipleOrigins at all, and we can just infer the information from the shape of the gradients prop

Done in #46315

Get rid of the __experimentalIsRenderedInSidebar prop

We should avoid props that introduce a tight coupling with the editor. First we should understand exactly what changes when this prop is specified, and then find alternatives to get to the same results

Component structure is messy

We should tidy up components and folder structures for the GradientPicker-related components (GradientPicker, CustomGradientPicker, ColorPalette, ColorListPicker, CircularOptionPicker, ColorIndicator....). These components feel very intertwined / tightly coupled and partially overlapping. (related to #35093 )

Sometimes clicking on the GradientPicker in the editor doesn't result in a new control point being added

I can't reproduce this unexpected behavior consistently, but it's definetly something that we should investigate (it could be related to the low rendering performance listed above)

Done as part of #49146

@ciampo ciampo added [Type] Bug An existing feature does not function as intended [Type] Enhancement A suggestion for improvement. [Feature] UI Components Impacts or related to the UI component system [Package] Components /packages/components labels Aug 4, 2022
@ciampo
Copy link
Contributor Author

ciampo commented Aug 4, 2022

cc @jorgefilipecosta who mostly worked on these components — would you have any capacity to take a look at some of these issues?

@talldan
Copy link
Contributor

talldan commented Aug 15, 2022

isRenderedInSidebar was mentioned in this discussion - https://make.wordpress.org/core/2022/08/10/proposal-stop-merging-experimental-apis-from-gutenberg-to-wordpress-core/#comment-43590.

It's in use by a plugin, so some care should be taken when refactoring it.

@uzegonemad
Copy link

Hopefully this is the appropriate place to leave this feedback. (If not, please redirect me to the appropriate place.)

  1. It would be ideal to display the percentage a gradient stop (control point) is located at.

Right now, to ensure a gradient stop is at the correct place, I have to open my browser's dev tools, select the element with the gradient, and move one of the points until dev tools show it at the correct percentage. (The use case is a sharp gradient at 50%, or 1/3. I use those to display full-width backgrounds while maintaining a content grid)

Adding a tooltip to the control point would be a huge help.

  1. It would also be useful to have an input to make manual tweaks to the position.

The use case is to provide better precision but also the ability to use other units, e.g. vw/vh or px. It looks like others have previously mocked up what this could look like in several of the comments from #16662, as well as #23816 (comment).

@ciampo
Copy link
Contributor Author

ciampo commented Aug 31, 2022

Hey @uzegonemad , thank you for your feedback!

We're not actively working on this component at the moment, but we'll take your feedback into account when we do!

@jorgefilipecosta
Copy link
Member

Hi @ciampo,

Slow rendering performance

Is this issue specific to the gradient picker, or is the site editor that is taking some time to update and reflect the new prop? With CPU throttling updating a color by dragging on the color picker and updating a gradient is not very different.

cc @jorgefilipecosta who mostly worked on these components — would you have any capacity to take a look at some of these issues?

Yes, I will try to find some availability to do some enhancements here.

@ciampo
Copy link
Contributor Author

ciampo commented Sep 6, 2022

Is this issue specific to the gradient picker, or is the site editor that is taking some time to update and reflect the new prop? With CPU throttling updating a color by dragging on the color picker and updating a gradient is not very different.

I believe it's more about the site taking a long time to update to reflect the changes in the new prop.

@seothemes
Copy link

Could we also add support for conic gradients to the wishlist?

There is good browser support now and no reason not to provide the option.

@ciampo
Copy link
Contributor Author

ciampo commented Oct 4, 2022

Could we also add support for conic gradients to the wishlist?

There is good browser support now and no reason not to provide the option.

This list was mainly aimed at improving / polishing the component in its existing state, but in general I'm not opposed to it

@seothemes
Copy link

I have created a separate issue here #44633

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] UI Components Impacts or related to the UI component system [Package] Components /packages/components [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

No branches or pull requests

5 participants