Components: Ensure Consistent Button Focus Styling Across ItemGroup Button and Button

[im3dabasia]

What?

Closes #67747

This PR standardizes focus styles for buttons rendered via ItemGroup Button and the Button component, ensuring visual consistency.

Why?

Currently, buttons in the repository use different approaches for focus styles—some rely on :focus, while others use :focus-visible. This inconsistency leads to uneven focus behavior, impacting both UI consistency and accessibility.

A similar issue was highlighted in this PR comment, where the focus styles in the trunk were deemed superior to the newer changes. Additionally, after the merge of this PR, ItemGroup was replaced with a Button, resolving part of the inconsistency.

How?

• Updated ItemGroup Button styles to align with the Button component’s focus behavior.
• Applied :focus:not(:disabled) for a more consistent and accessible focus style.

Screenshots or screencast

1. The issue: In :focus, the CSS is set to none, as shown here. This is inconsistent with the Button component, which has a box-shadow and outline applied in the :focus pseudo-state. (See the 3rd screenshot for reference.)

---

2. The focus styles are only applied when using :focus-visible, meaning the button does not receive focus styling when clicked with a mouse.

---

3. This is the Button component in its :focus state. We aim to replicate this behavior in the ItemGroup button for consistency.

---

4. The solution: After adjusting the CSS, we have ensured that both the Button and ItemGroup button now behave consistently. :focus styles have been added to ItemGroup, and :focus-visible styles have been removed.

---

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: im3dabasia <[email protected]>
Co-authored-by: afercia <[email protected]>
Co-authored-by: ciampo <[email protected]>
Co-authored-by: mirka <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[im3dabasia]

Hey @afercia ,

When you have a moment could you please review my PR.

Thanks

[afercia]

@im3dabasia thanks for your PR. We need to find another example for the testing instructions because #68672 changed the Duotone button and now it's not an ItemGroup button any longer. Changes look good to me, will test shortly.
When you have a chance, could you please add a changelog entry to the components package changelog?

[afercia]

Sorry I wasn't clear. What I meant in #67747 is that the focus should be consistent only when the Item is rendered as a Button components, that is for example: <Item as="button" { ...toggleProps }>. The as prop is passed here so that it could be used to distinguish the focus style.

[im3dabasia]

I have corrected it. Previously, the style was applied to Items as 'button' or as 'a'.

After 0355271 the focus styles will be applied only on the button and otherwise it would default to the old style ie focus-visible for the other ItemGroup.

[afercia]

So far this PR changes the focus style unconditionally. All ItemGroup <Item> components would use the new focus style. As an accessibility specialist I would have no objections and I'd recommend it but I'm afraid it would be too much for the design team.
I'd suggest to use :focus only when the Item has the as="button" prop and use :focus-visible otherwise.

[im3dabasia]

Thank you @afercia , for the correction in the log. I appreciate it !

[afercia]

I thought it was quicker to fix this way 🙂

[afercia]

This PR works as intended. However, I would like to get some more feedback before merging. See following comment.

[afercia]

My bad, I think I misunderstood that most of the Item components of the ItemGroup usages receive the as prop with a button value. In fact, with this PR most items now show the focus style also when clicking the button.

For example, all the navigation items in the Site editor navigation panel will now show the focus style on :focus instead of :focus-visible and many Items in the Styles panel as well. This is exactly what this PR is supposed to do, I'm just willing to make clear the extent of this change.

From an accessibility perspective, focus indication should always be visible also when using a pointing device (mouse). Instead, :focus-visible should be used rarely and only for very good reasons. That's why I still think this is a good change but I just want to be sure we are all aligned.

Also, I think the all the base components in the @wordpress/components package should use a consistent focus style otherwise the styling is based on assumptions because we don't know where the base components will be used. If desired, the focus style should be changed in the usage of the base components in the editor or other applications built by other consumers of the package but I don't think that opinionated styling should be built-in. Cc @WordPress/gutenberg-components

One place where this can be tested is in the Site Editor > Styles > Typography > Fonts:

The list of items now shows the focus style also when clicking.
To test the alternative behavior when the as prop is a, change this line:

https://github.com/WordPress/gutenberg/blob/trunk/packages/edit-site/src/components/global-styles/font-family-item.js/#L32

to <Item as="a" href="#" ...

then test navigating and activating the items with mouse and keyboard and observe the focus style is not shown when clicking.

[ciampo]

Will try to address the points raised:

Re. the goal of this PR:
If I recall, the Item component didn't have styles for button and a by design (the style function is in fact called unstyledButton), I guess with the assumption that a consumer could instead specify as={ Button } in case they wanted Button styles (although I don't have a lot of time to experiment with the code myself).

Re. using :focus instead of :focus-visible, are there official docs about the fact that "focus indication should always be visible also when using a pointing device (mouse)"? I couldn't find any clear mention of that (and I actually found a few sources identifying :focus-visible as an improvement over :focus).
In any case, I guess there are design implications too, which should not be disregarded. I imagine :focus-visible was chosen specifically to avoid showing a focus ring when clicking those buttons, since the focus ring would probably quickly "flash" before animating to the new view.

Re. using consistent focus styles, that is the general intent. In fact, even before this PR, I believe the Item focus styles were using the same var(--wp-admin-border-width-focus), the same COLORS.theme.accent and the same outline settings as other components.

[im3dabasia]

Thank you @ciampo for your feedback.

If achieving consistency is our goal, we could also consider the following approach:

We could adjust the original <Button> component to use :focus-visible instead of :focus. This way, we would have consistent styling between both button types - ItemGroup as="button" and the <Button> component - by using the same :focus-visible approach.

What do you think about this alternative change? It would maintain consistency while avoiding the potential design drawbacks of the current approach.

Looking forward to your thoughts, @afercia and @ciampo.

[mirka]

We've been discussing the focus-visible strategy in #65088, and I'd say we've achieved a consensus that we shouldn't prefer focus-visible globally by default. Rather, the more sensible strategy we've agreed on is to only apply it over the default focus in selective cases, when there is good enough reason to do so.

[im3dabasia]

We've been discussing the focus-visible strategy in #65088, and I'd say we've achieved a consensus that we shouldn't prefer focus-visible globally by default. Rather, the more sensible strategy we've agreed on is to only apply it over the default focus in selective cases, when there is good enough reason to do so.

That being said, this PR aims to mimic the :focus being used in the <Button> component for the ItemGroup as "button". So we are aligning with the previous consensus. However, I should add that currently we are adding focus to just the ItemGroup as a "button" only. For ItemGroup as an "a" anchor tag, we have retained the old :focus-visible.

If we agree, we can have the same set of CSS applied to both the ItemGroup as button and as anchor.

cc: @mirka @afercia

[im3dabasia]

@afercia,

Since this PR has been approved, would it be appropriate to merge it into trunk? Please let me know if any changes might be needed.

CC: @Mamaduka , @t-hamano, @ciampo

[t-hamano]

I think we need to carefully discuss again whether :focus-visible or :focus should be used in this component. This is because this component originally used :focus but was changed to :focus-visible by #51787.

Note that this PR affects not only the global styles sidebar, but also the site editor sidebar. After applying this PR, you'll see that the focus outline appears when you click on a menu item:

beb6577fc053593f4fe3764e646ca31f.mp4

Perhaps from a design perspective, the blue outline is considered too noticeable against the black background.