Components: Introduce getAuxiliaryColorArtefactWrapper prop for color picker

[andreiglingeanu]

Description

This pull request adds two things:

1. getAuxiliaryColorArtefactWrapper prop for the color picker component
2. Implement a serie of props for the gradient-picker that allows passing custom props to the color picker from the custom gradient UI

Reason for this being allowing client code to use a simpler version of the color & gradient pickers without completely re-implementing those from the plugin side. This will cause a lot of duplication on the plugin end and we'll also be forced to bundle react-colorful twice to make that happen.
Reasoning for this is also described in the updated README.

How has this been tested?

By consuming the new props from the code that uses the color picker and gradient picker components from the Blocksy theme.

Screenshots

Types of changes

Non-breaking additions into the existing components.

Checklist:

• My code is tested.
• My code follows the WordPress code style.
• My code follows the accessibility standards.
• I've tested my changes with keyboard and screen readers.
• My code has proper inline documentation.
• I've included developer documentation if appropriate.
• I've updated all React Native files affected by any refactorings/renamings in this PR (please manually search all *.native.js files for terms that need renaming or removal).

[ciampo]

Hey @andreiglingeanu , thank you for opening up this PR!

Reason for this being allowing client code to use a simpler version of the color & gradient pickers without completely re-implementing those from the plugin side. This will cause a lot of duplication on the plugin end and we'll also be forced to bundle react-colorful twice to make that happen.

I agree with you that re-implementing the color picker would not ideal.

I'm personally slightly skeptical about adding a whole new render prop for the auxiliary color artefact wrapper is the best solution, API-wise. Why do you need a simpler version of the color picker? And what in particular do you consider extra?

I'd love to hear also @jorgefilipecosta 's opinion.

A couple more comments:

• could you add a Storybook example to the ColorPicker component showcasing how you would potentially use this new prop? Not having any concrete way of testing your changes (or to even see how they're meant to be used) makes it hard to give this PR a proper review
• could you add an entry to the CHANGELOG?

• getAuxiliaryColorArtefactWrapper prop for the color picker component
• Implement a serie of props for the gradient-picker that allows passing custom props to the color picker from the custom gradient UI

Why are the changes described in second point necessary? Can they be split and discussed in a separate PR?

[andreiglingeanu]

@ciampo Thanks a lot for getting back to me, really appreciate it.

I'm personally slightly skeptical about adding a whole new render prop for the auxiliary color artefact wrapper is the best solution, API-wise.

Completely agree.

Why do you need a simpler version of the color picker?

We need to be able to easily swap out the auxiliary color artefact part of the color picker. For example here's a quick usage of the render prop and its visual result:

1. Screenshot of the result (swap auxiliary color artefact with a simpler input to save space in restricted places or remove it at all): https://cln.sh/UIwIj3
2. Render prop usage:

<ColorPicker
    getAuxiliaryColorArtefactWrapper={() => {
        return <input value={color} />
    }}
    color={color}
    enableAlpha
    onChange={{}}
/>

And what in particular do you consider extra?

The auxiliary color artefact itself, according to the above screenshot.

---

API wise, I agree that the proposed render prop might not be the best solution and I'm happy to hear your advise hear on what other options we might have. Something based on filters/portals or React contexts might do a better job here.

Another requirement of ours is that we need to also be able to change the color picker from the gradient picker when we render it from our components, thus the need of all the props there. With all the props the usage is quite ugly:

<GradientPickerComponent
    value={value.gradient || ''}
    onChange={(val) => {
        onChange({
            ...value,
            gradient: val,
        })
    }}
    customGradientPickerProps={{
        customGradientBarProps: {
            controlPointsProps: {
                colorPickerProps: {
                    getAuxiliaryColorArtefactWrapper: () => {
                        return <input value={'#fff'} />
                    },
                },
            },
        },
    }}
/>

If we are to implement a solution for swapping out the auxiliary color artefact using a filter or with a context based solution we should be able to avoid all this messy prop drill. Again, happy to hear your ideas here and also update the PR with the implementation.

---

So, to conclude:

1. I won't be making a storybook screen with this new render prop until we settle upon an better API. When we have that, I'll provide an implementation of the API + a storybook screen to see it in action
2. With the correct API implementation, prop drill in the gradient component will become obsolete and it won't be needed anymore. I'll update the PR to remove all these props and a 2nd PR to cover the gradient won't be needed hopefully
3. I will add the relevant Changelog entries as soon as we settle upon an API that you like here.

---

Thanks and waiting for your and @jorgefilipecosta's thoughts!

[jorgefilipecosta]

I'm not really sure about the best way to allow this extensibility scenario, in the end, I think we will always face some customization cases that are not possible.

I think of two possible alternative ideas to achieve the UI show by @andreiglingeanu:

Alternative 1:

• Add a property to the color picker that allows showing input details by default (set the default show details to true) so we would get this:
  
• Then using CSS one can remove the show details button, the hex selection, and customize the input.
• Add a property to the gradient picker called color picker props that is passed to the color pickers.

Alternative 2:

• Add a property to the color picker that totally disables the additional UI.
• Then one can create a wrapper component that uses our component and the custom input field.
• Add a component property to the gradient picker called ColorPicker that would allow changing the color picker used by gradient picker allowing one to use a custom color component.

I don't have a strong preference for any one these options and I'm open to other alternatives.

[andreiglingeanu]

@jorgefilipecosta I'd strongly prefer the #2 alternative. Mainly because I'd like to have one simple input that handles all color formats, as opposed to having this complex UI to manage the HSL/RGB values: https://cln.sh/3YVW57

If there are no objections, I'll take that's the green light for me to start the development of #2.

Thanks a lot for your inputs! 🙏

[ciampo]

Hey @andreiglingeanu, if this feature is not extra-urgent, I was thinking of coming back with a more detailed answer tomorrow, before you go ahead with one approach — hope that's ok!

[andreiglingeanu]

Sure @ciampo! Waiting for your feedback.

[ciampo]

Alright, here's some thoughts.

We need to be able to easily swap out the auxiliary color artefact part of the color picker.

I still don't understand what's the problem with the auxiliary controls that come by default with the ColorPicker — do they take too much space when expanded? Do you want to limit the user's ability to pick different color formats? Anything else?

Maybe we can just add some functionality to the ColorPicker itself, which would be a much smaller change? (cc @jasmussen who may be interested in this conversation)

---

Alternatively, I agree that Jorge's option no. 2 is the better one. We could:

• add a hideAuxiliaryControls boolean prop
• allow the ColorPicker to render children
• this way, you would be able to hide the auxiliary controls and then show whatever component you need as children of the ColorPicker

I went ahead and created a draft PR to illustrate this approach in #37864.

---

Regarding the Gradient Picker, I'm not sure we can avoid prop drilling. The main problem is how the gradient-related components are written and composed together, and consequently how the ColorPicker gets imported and used.

This seems to be the component's dependency graph:

GradientPicker
  CustomGradientPicker
    CustomGradientBar
      ControlPoints.InsertPoint
        ColorPicker

Exposing any props that targets ColorPicker would still need to drill through 4 components, and I'm not sure if there's a way to avoid this from happening (which doesn't involve refactoring the whole set of components)

[jasmussen]

Thanks for the ping, I've been involved with the design of the color picker mostly, so I'm coming at this from a visual and user experience angle, rather than a technical one. In that light, it's likely I'm missing a great deal of nuance from the conversation, I hope you'll bear with me.

High level, the color (picker while recently improved) has a ways to go with regards to refinement and polish. Before adding extensibility to it, it seems the most productive to get the component itself to a stable place. When we do get there, it should also be easier to discern where points of extensibility could be added, vs. what should be a core part of the experience.

[ciampo]

You make a very good point, @jasmussen . Given your involvement in the design process of the ColorPicker, I agree that we should first understand exactly what we are trying to solve from a visual/UX point, and focus on getting the component to a stable/complete place.

[andreiglingeanu]

Thanks a lot for your inputs, it's really appreciated.

First of all, yes, the whole auxiliary controls part of the color picker just takes too much space when it is being used in a modal. So we started to look for a way to replace it with a single input element that will handle all the color formats at once.

Generally, we were hoping with this PR to improve Blocksy's color picker & gradient compatibility with the upcoming WordPress 5.9. But it seems that now we'll have to look for a short term more hacky solution and a proper one for the long term. But I'm not sure how exactly the timeline looks and maybe there is chance to add this small rework to 5.9. Currently, with the 5.9 builds our color picker looks quite broken due to the changes done.

Generally speaking, we need to:

1. Be able to disable the auxiliary for the color picker (hideAuxiliaryControls prop sounds amazing!)
2. A way to pass that prop down to the color picker from the gradient

And what's done in #37864 sounds like an amazing start and will serve just fine for us as a short term solution. And, then, I'm more than happy to give a hand in developing a more robust long term solution.

Waiting for your thoughts!

[jasmussen]

I would hope and expect that components related to color, gradient and duotone swatches (including pickers and sliders and inputs) get refined and matured for 6.0. The recent refactor to allow colors to collapse into an ItemGroup should be seen as an expression of that being a priority. I can't speak to the technical details on whether extensibility at this point is correct, I'll defer to @ciampo for that, but a good rule of thumb is to expect the interface to continue to evolve, and design extension points in that light.

[andreiglingeanu]

@jasmussen Totally agree with the idea that you should be able to fearlessly evolve the components over time. And I also understand that introducing extensibility options can slow down this process.

But, I'm looking here for a short to mid term solution (for 5.9 specifically) and we're totally okay if we need to revisit our implementation for 6.0 if you decide to get rid of the current component API.

Goal with this is, like mentioned above, to fix the color picker experience within the Blocksy theme with the upcoming 5.9 release. If we don't do this, we expect a lot of our users to be affected by this and we really want to avoid going forward with a hacky solution. And we're more than okay if we need to rework this sometime in the future.

[jasmussen]

Coming at it mostly from a visual angle, I can only offer high level advice, and I have to defer to WordPress release leads and those with technical insights. With that in mind, it's my understanding that we are at a point in the release schedule where only bugfix category changes are able to land. So if we need to look for a 5.9 solution, we have to explain the problem very well, and pinpoint-target our fixes as much as possible. I took a look at Blocksy (impressive work!), but it wasn't fully clear to me what part is broken. It sounds like you are seeking to achieve this hex-code input as a replacement of the existing one, is that correct? What is that meant to solve?

[andreiglingeanu]

@jasmussen thanks a lot for chiming in and for the words about Blocksy!

Here's a more obvious comparison between the approaches:

WordPress 5.8.*: https://user-images.githubusercontent.com/2979628/149528527-36f362f7-78af-4896-bcdf-87e7753b4f7e.mp4

WordPress 5.9.*: https://user-images.githubusercontent.com/2979628/149528594-08fe8df9-1e87-477b-b08a-39b35128f2f1.mp4
Also, same happens in the Gutenberg screen too: https://user-images.githubusercontent.com/2979628/149528644-efb47779-ab92-48fe-ac3a-b3e5b0c851d3.mp4

Hope this shows how restricted in space we are and why we need the capability of replacing the bottom color picker part with something simpler.

If there a chance to add this change for 5.9 release — we're more than happy to help with the efforts. Please let us know if we need to raise this in a different place or if you need another issue for this.

Thanks a lot!

[ciampo]

Hey @andreiglingeanu, from a first look at the videos, it seems like the issue if that the color picker's content is overflowing the popover — could that be an issue associated with the popover container not being sized correctly, instead of the color picker itself?

PS: it would be easier if you could also post a link with detailed instructions on how the issue can be reproduced

PPS: it would be great if you could embed the actual images and videos directly in GitHub!

[andreiglingeanu]

@ciampo We can fix the overflow with CSS but this will increase the overall popup size, which we'd like to avoid. We want to replace the auxiliary controls with something simpler exactly for that reason.

[ciampo]

After discussing with @jorgefilipecosta , we think that the best solution to the problem you've shown in the videos above is for you to adapt your popover to the new ColorPicker.

As an example, this is how the new ColorPicker is being rendered inside a popover in the context of Gutenberg's Global Styles sidebar:

Screen.Capture.on.2022-01-14.at.17-30-47.mp4

---

This doesn't mean that we won't look into improving the component. We will still consider and investigate the best ways to:

• allow more flexibility in showing/hiding (and maybe proving a custom version of) the ColorPicker auxiliary controls
• allow more customization around the ColorPicker used when choosing color stops in the GradientPicker

[andreiglingeanu]

@ciampo Thanks again for your and @jorgefilipecosta's inputs, really appreciate that.

Unfortunately though, the flow with the color picker in the popover will not work for the WordPress customizer. That is why we are looking for an option to keep everything tight in one place.

---

What's the best way to continue the investigation of such a solution? Should we open a separate issue to keep the discussion focused? We understood that it's highly unlikely that this will happen for 5.9 and we're in the process of implementing another option now (importing the specific components we need from npm directly).

Thank you!

[ciampo]

Hey Andrei, I went ahead and opened these 2 issues:

• ColorPicker: allow customisation of the auxiliary controls #38065
• GradientPicker: allow custom ColorPicker #38066

Feel free to continue the conversation there — in the meantime, I'm going to close this PR.

[andreiglingeanu]

@ciampo lovely, thanks a lot!