Avoid custom styling for any Button in the editor UI

[afercia]

Description

Sometimes, some base components like the Buttons use custom styling in the editor UI, thus overriding the default styling of the base components.

As recently discussed in other issues and PRs, custom styling should be avoided. By overriding the default styling:

• It just makes the UI inconsistent.
• It adds cognitive load, as users wouldn't have a clue why some buttons are styled in a different way.
• It adds maintenance cost.

Ideally a better approach would be:

• Establish whether some new styling proves to have many good use cases to improve the user experience. Custom styling should not be introduced for a unique case or for a few instances.
• If there's a decent amount of good use cases where a new style would benefit users, consider to introduce a new variant in the base component instead of implementing ad-hoc, custom, local CSS overrides.

Cc @DaniGuardiola

A few examples:

I'd agree that the Featured image button and the button to Add a background image (for example in a Group block) are, in a way, special cases because when the image is set they contain the image preview.
But...
when the image is not set, they are just normal buttons. I don't see any good reason why they should use a custom style. Screenshot:

This styling with a grey border (actually a box-shadow) around the button is custom. It's not any of the default Button component variants: Default, Primary, Secondary, Tertiary, Link, Destructive. It's custom, redundant CSS that introduces visual inconsistency in the UI.

Note: the only non-primary default styling that sets a border on the Button is the Secondary variant. It looks like this:

All teh Button variants can be quicly checked in the Storybook.

One more example:

When the featured image is set, the Replace and Remove buttons evolved from standard variants:

to a custom implementation:

As far as I can tell, this is a unique styling in the editor UI. I don't see any good reason to not use one of the standard styling variants instead of an ad-hoc (maybe more pleasant?) look and feel. But that's subjective anyways. CSS overrides of the base components should be avoided at all costs. As said, if there's a good use case for some new styling, that should be considered as a new variant. I'd also argue that destructive actions in WordPress should be red. That went lost with the custom styling.

Overall, I'd suggest to only use the base components styling everywhere in the editor. The pros of having a consistent styling for all controls in the UI and having less and more maintainable code largely outweighs any other consideration related to subjective aesthetics.

Step-by-step reproduction instructions

• Edit a post.
• Observe the Set featured image button in the Post Settings panel uses some custom styling.
• Set a featured image.
• Observe the Replace and Remove buttons use some custom styling.
• Observe the Remove button is not red.
• Add a Group block.
• In the Block Settings panel, go to the Styles tab.
• Observe the Add background image button uses some custom styling.

Screenshots, screen recording, code snippet

No response

Environment info

No response

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

Yes

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

Yes

[afercia]

CC also @ciampo and @mirka

[DaniGuardiola]

Completely agree. I think we can take the following steps:

1. Identify all cases of custom styles.
2. Loop design and product folks in. See if there are strong reasons to differ from base styles. If so, identify and consolidate these cases into variants, implemented directly in base components.
3. Remove custom styles.

cc @WordPress/gutenberg-design @WordPress/gutenberg-components

[richtabor]

There's a clear reason for the featured image buttons. They went from no styling to thoughtful placement.

And for featured image, I'd treat them more as dropzones rather than buttons.

[afercia]

There's a clear reason for the featured image buttons.

Could you please add more context? What the 'clear reason' is? I'm not sure this kind of statements without any context are any helpful in a collaborative open source project where contributors who land on an issue and are maybe considering to contributes need to understand the reasoning and decisions that were made before actually being able to contribute. Tickets and issues are part of a project history and documentation. As such, I'd encourage everyone to avoid assumptions and always document and clarify their reasoning.

And for featured image, I'd treat them more as dropzones rather than buttons.

Considering a <button> element as a dropzone is already very arguable in terms of semantics and accessibility because that's not what buttona are meant for. Also, while that may apply for the 'set' and 'add' buttons, the replace and remove buttons are not dropzones and there's no funcitonal reason to style them differently.

We agreed in other conversations that the editor is already very complicated. I'm not sure adding unique cases, ad-hoc styling and cognitive load is the best way to make it more simple. Unique styling just adds more complexity.

[richtabor]

Could you please add more context? What the 'clear reason' is? I'm not sure this kind of statements without any context are any helpful in a collaborative open source project where contributors who land on an issue and are maybe considering to contributes need to understand the reasoning and decisions that were made before actually being able to contribute. Tickets and issues are part of a project history and documentation. As such, I'd encourage everyone to avoid assumptions and always document and clarify their reasoning.

The clear reason is a lack of styling from the screenshot you provided. Common sense, not reasoning/discussions etc. You'd be hard pressed to find a that low effort design in many successful software projects.

[afercia]

The clear reason is a lack of styling from the screenshot you provided. Common sense, not reasoning/discussions etc. You'd be hard pressed to find a that low effort design in many successful software projects.

I'm not sure I understand your feedback. Not even sure the tone is fair, but that could be because I'm not a native English speaker.

Anyways, I asked to clarify the statement There's a clear reason for the featured image buttons. and I still don't see the clear reason. On the other hand, I think I clearly explained why base component like Buttons should never be overridden with custom styles or ad-hoc implementations.

[jameskoster]

Replacing / removing a featured image is a relatively uncommon task. Placing the buttons 'on top' of the image avoids adding unnecessary clutter to the UI, indirectly making more common flows easier to navigate. The existing button style variations are not suited to this use case, hence the customisation.

Perhaps there's a case to add a new Button variant for situations like this. It may be useful elsewhere, e.g. the focal point picker:

[richtabor]

Anyways, I asked to clarify the statement There's a clear reason for the featured image buttons. and I still don't see the clear reason.

As pictured in your screenshot above of the previous replace/remove buttons for the featured image, the buttons lack any finesse of styling, layout, and positioning. It's not a thoughtful execution, nor supportive of the experience.

Although the buttons currently have a different look to others, they are much more intentional and thoughtfully executed, not just tossed in.

[richtabor]

Perhaps there's a case to add a new Button variant for situations like this. It may be useful elsewhere, e.g. the focal point picker:

Maybe so.

[afercia]

As pictured in your screenshot above of the previous replace/remove buttons for the featured image ...

Those are not the previous buttons. They are the buttons used on current trunk. Screenshot taken today from trunk:

... the buttons lack any finesse of styling

I agree. And it's because they're custom, ad-hoc, not very well thought implementation.

Placing the buttons 'on top' of the image avoids adding unnecessary clutter to the UI, indirectly making more common flows easier to navigate.

@jameskoster While I do agree avoiding unnecessary clutter does have value, I'd argue that it's a personal opinion anyways. This UI change has not been user tested, not even a simple A/B testing to prove that it's an improvement. It's only based on personal opinions, at the cost of introducing some inconsistency in the design, maintenance cost, etc. My take on these cases is: if there's a case for a new variant for many valid cases then the base component should provide the new variant. Otherwise, refrain from introducing inconsistent design and exceptions. Instead, this is a unique case in the editor. It's completely inconsistent and its value hasn't been proved yet.