WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.52k stars 4.21k forks source link

ColorGradientSettingsDropdown cannot be used outside of ToolsPanel anymore #42692

Open corentin-gautier opened 2 years ago

corentin-gautier commented 2 years ago

Description

Since #41091 <ColorGradientSettingsDropdown> cannot be used outside a <ToolsPanel> since it's always wrapped inside a <ToolPanelItem>

It also means that __experimentalIsItemGroup is not used anymore (still used here)

Is that an intended behaviour ?

Step-by-step reproduction instructions

  1. use <ColorGradientSettingsDropdown> outside a <ToolsPanel>
  2. It will throw an error (linked to panelId)

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

Yes

First reported in #41438

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

rmorse commented 2 years ago

I left a comment on that ticket too - shame such a nice (and the new standard) UI is now unavailable to us.

@jorgefilipecosta can you help? 🙏

jorgefilipecosta commented 2 years ago

Hi @corentin-gautier, @rmorse, thank you for reporting this issue.

What in your use cases make you want to avoid the wrapping of ColorGradientSettingsDropdown in a tools panel? The dropdown could easily be used in third-party blocks as for example, the social-links does packages/block-library/src/social-links/edit.js:

            <InspectorControls __experimentalGroup="color">
                { colorSettings.map(
                    ( { onChange, label, value, resetAllFilter } ) => (
                        <ColorGradientSettingsDropdown
                            key={ `social-links-color-${ label }` }
                            __experimentalHasMultipleOrigins
                            __experimentalIsRenderedInSidebar
                            settings={ [
                                {
                                    colorValue: value,
                                    label,
                                    onColorChange: onChange,
                                    isShownByDefault: true,
                                    resetAllFilter,
                                    enableAlpha: true,
                                },
                            ] }
                            panelId={ clientId }
                            { ...colorGradientSettings }
                        />
                    )
                ) }
                { ! logosOnly && (
                    <ContrastChecker
                        { ...{
                            textColor: iconColorValue,
                            backgroundColor: iconBackgroundColorValue,
                        } }
                        isLargeText={ false }
                    />
                ) }
            </InspectorControls>

Would not something similar to the sample above fit your needs?

aaronrobertshaw commented 2 years ago

There is a little more history related to the use of the color dropdown inside a ToolsPanel in https://github.com/WordPress/gutenberg/pull/40084

corentin-gautier commented 2 years ago

@jorgefilipecosta I ended up using the tools panel, but at the moment it looks weird (no top border)

Here's the UI for context :

Screenshot 2022-09-02 at 10 08 26
aaronrobertshaw commented 2 years ago

@corentin-gautier, I know this doesn't solve the issue with the lack of a top border in your screenshot above but could you add the icon color control into the Color panel?

This is what the Social Links block does via <InspectorControls __experimentalGroup="color">. The __experimentalGroup defines which SlotFill in the inspector controls sidebar the children are rendered into. In this case, that would be the color slot i.e. the Color panel.

This approach would also benefit from keeping all color controls together in a single panel and location. What do you think?

rmorse commented 2 years ago

Hey @aaronrobertshaw + @jorgefilipecosta

Thanks for your feedback on this.

My only concern is that sometimes it feels more natural to add a color selector in another panel. I understand this breaks convention somewhat, but in certain scenarios I do (/did) think its better (such as when the color picker is not always enabled).

An example: I had a custom border panel, whereby a border of 0 would hide the color picker, and setting it to anything other than 0 it would appear. As the color picker is dependant on the border width, it's confusing to have it appear in the color panel (ie, somewhere else without informing the user) - and it seems logical to have it available where it gets enabled.

As mentioned, I know this is breaking convention and it seems the block editor UI is going down the path of all colors should be under one panel, but I have a codebase that was written before the ToolsPanel, and was quite happy swapping out the picker for the new improved picker, until this recent breaking change.

To make the necessary changes to follow convention, and add the picker under the colors ToolsPanel, is going to be a pretty big undertaking - and arguably for this particular control not the best implementation.

Currently, all other controls work in regular Panel's, it just seems this particular control won't work unless it's inside a ToolsPanel - which is a breaking change introduced recently - and based on the behaviour of other controls, seems not to follow the same convention.

I think it would make sense to decouple the control itself from the panel its contained in.

As another use case - I'm building a custom UI in another plugins admin pages.. we also have a sidebar (no block editor though), and we're re-using tons of block editor components to build that UI. Again, because we can't use the ToolsPanel (and in this case, we don't have a color slotfill), we can't use this control - but we can use all the other block editor controls.

I hope that clarifies things a bit and give some insight as to why and how we've been using this control previously in other contexts.

aaronrobertshaw commented 2 years ago

Thanks for expanding on what you are trying to achieve @rmorse 👍

Furthermore, I can still see some blocks wishing to add arbitrary color controls in a semantic way outside of a ToolsPanel though. For example, the Cover block has overlay and opacity controls. The overlay control would fit nicely within the color support tools panel however the opacity looks a little out of place. If this were a 3rd party block, the author might wish to keep these two related controls together in a separate panel.

Similar arguments to those in your last comment were made previously on the PR I linked earlier for context (https://github.com/WordPress/gutenberg/pull/40084).

An example: I had a custom border panel, whereby a border of 0 would hide the color picker, and setting it to anything other than 0 it would appear. As the color picker is dependant on the border width, it's confusing to have it appear in the color panel (ie, somewhere else without informing the user) - and it seems logical to have it available where it gets enabled.

The block editor currently doesn't include border color controls within the Colors panel. There is specific control for configuring borders which includes its own border color dropdown. That might also be of interest to you and the UI you're developing or maintaining.

I think it would make sense to decouple the control itself from the panel its contained in.

In terms of our best path forward, I'd defer to @jorgefilipecosta who might have more insight as to what would be required to rollback some of the recent changes or provide a separate color dropdown component.

I'd be happy to help with which course of action we decide upon after the 6.1 release.

corentin-gautier commented 2 years ago

@aaronrobertshaw I did try the group settings but as @rmorse said, it's very confusing to have it appear on another panel (mine is conditioned on the icon being an svg). Plus the color panel could be hidden at the time (by the scroll for example)

I'm actually not sure about this, having all the colors in the same panel could be good or bad given different scenarios