Make WordPress Core

Opened 2 months ago

Last modified 38 hours ago

#60808 new defect (bug)

Twenty Twenty-Four: Black outline around the Navigation block submenu-expanding button

Reported by: jordesign's profile jordesign Owned by:
Milestone: 6.6 Priority: normal
Severity: normal Version: 6.5
Component: Bundled Theme Keywords: 2nd-opinion needs-testing has-patch
Focuses: accessibility Cc:

Description

This appears to be a regression introduced in https://core.trac.wordpress.org/changeset/57739

Brought over from the Gutenberg repo: https://github.com/WordPress/gutenberg/issues/59944


Description

When a sub-menu item is present, when clicking on the sub-menu item or on the parent menu item, a black border is shown around the items.

https://cldup.com/kNOVprCJ_d.png

Video: https://cloudup.com/cm8LVp_p6kN

Step-by-step reproduction instructions

  1. go to the site editor
  2. add a navigation block
  3. add an item and a sub item
  4. publish changes and visit the site
  5. click on the sub-menu item and on the parent item
  6. see the black border

Environment info

WordPress 6.5 RC2
2024 theme

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

Attachments (4)

60808.diff (551 bytes) - added by sabernhardt 2 months ago.
switches :focus to :focus-visible
details-block-focus.png (3.6 KB) - added by wildworks 2 months ago.
Custom focus outline causes an unnatural outline to be displayed when clicking the summary element in the Details block
60808-2.diff (590 bytes) - added by wildworks 6 weeks ago.
Add CSS for focus control to navigation submenu block
additional-css.png (8.1 KB) - added by wildworks 4 weeks ago.
The styles defined in theme.json are displayed in the global style additional CSS panel

Download all attachments as: .zip

Change History (37)

#1 @poena
2 months ago

  • Focuses accessibility added
  • Keywords 2nd-opinion added

Hi
This is not a bug. While it is a change, it is an intentional accessibility improvement.

Last edited 2 months ago by poena (previous) (diff)

#2 @get_dave
2 months ago

Noting that this applies to all links not just Navigation submenus.

@poena If you have a moment, are you able to advise on where it was added so I can take a look at the rationale behind it?

I've not seen this approach taken before and would be open to learning more to improve my own knowledge.

#3 @poena
2 months ago

Yes, I was on my mobile and it was already linked from the Gutenberg issue, so I did not make the time add it.

The original issue was #60334.

In version 2.2 of WCAG, there is a new AAA level requirement that says that the focus style should be at least 2px thick.

https://www.w3.org/WAI/WCAG22/Understanding/focus-appearance.html
https://www.w3.org/TR/WCAG22/#focus-appearance

WordPress does not officially require WCAG 2.2, only version 2.1, and at level AA, not level AAA. There are discussions and arguments for following this requirement as a best practice in the ticket.

#4 @tomxygen
2 months ago

Although it may be intentional, it is very ugly.
Any chance you could suggest me some CSS to get rid of it?

@sabernhardt
2 months ago

switches :focus to :focus-visible

#5 @sabernhardt
2 months ago

  • Component changed from Themes to Bundled Theme
  • Keywords has-patch added
  • Summary changed from Twenty Twenty Four: Black border around the navigation items when sub items are present to Twenty Twenty-Four: Black outline around the Navigation block submenu-expanding button
  • Version changed from 6.4.3 to trunk

If the submenus are set to open on click, the focus outline is not ideal when someone uses a mouse on the toggle button.

@wildworks suggested :focus-visible (ticket:60535#comment:19), which I should have given more consideration. It applies the 2px solid style when navigating by keyboard, matching the focused element's text color unless the site defines it differently. However, :focus-visible does not add the outline in situations such as clicking the submenu toggle button or opening the context menu ("right-click").

#6 @sabernhardt
2 months ago

  • Keywords needs-testing added

#7 @sabernhardt
2 months ago

  • Milestone changed from Awaiting Review to 6.5.1

This ticket was mentioned in Slack in #accessibility by sabernhardt. View the logs.


2 months ago

#9 @wildworks
2 months ago

60808.diff appears to work as expected for me.

However, :focus-visible does not add the outline in situations such as clicking the submenu toggle button or opening the context menu ("right-click").

WCAG describes it as "When the keyboard focus indicator is visible, an area of the focus indicator meets all the following". In other words, it seems like a proper focus outline should only be visible when using the keyboard, and not a requirement when using the mouse.

However, I think it's basically a good thing that the focus is displayed even when using the mouse, so if anyone knows of a good approach, I'd like to hear your ideas.

Another thing I noticed about this issue is that when I click on the summary element in the Details block, an unnatural outline appears. This is due to the fact that overflow: hidden; is applied to the block. However, this may be a problem that should be fixed separately on the Details block side.

@wildworks
2 months ago

Custom focus outline causes an unnatural outline to be displayed when clicking the summary element in the Details block

#10 @sabernhardt
2 months ago

Yes, the missing/incomplete Details block focus outline is a separate issue and could have happened in any theme since 6.3. The bottom line shows when the element is open, but the summary focus does not appear at all when closed. Do you want to open a new Gutenberg issue for that?

#11 @wildworks
2 months ago

Do you want to open a new Gutenberg issue for that?

Yes, I reported it here: https://github.com/WordPress/gutenberg/issues/60269

#12 @poena
8 weeks ago

I believe comment:9 is correct. would like to ask Amber @alh0319 for their opinion, as the original reporter.

#13 @sabernhardt
8 weeks ago

#60923 was marked as a duplicate.

#14 @sabernhardt
8 weeks ago

#60923 was marked as a duplicate.

This ticket was mentioned in Slack in #core by jorbin. View the logs.


8 weeks ago

#16 @davidbaumwald
7 weeks ago

  • Milestone changed from 6.5.1 to 6.5.2

Milestone renamed

#17 @jorbin
7 weeks ago

  • Milestone changed from 6.5.2 to 6.5.3

#18 @alh0319
6 weeks ago

I don't think we should change to :focus-visible from :focus. I think @wildworks is right when saying that:

However, I think it's basically a good thing that the focus is displayed even when using the mouse

There may be cases where people are mouse users but have limited mobility or on a mobile phone, where having the focus indicator helps them to know that they are clicking on the correct element, particularly if tap targets are close together. (Parkinsons, for example.)

Let me alert the broader accessibility team to this ticket and see if we can get several opinions.

This ticket was mentioned in Slack in #accessibility by amberhinds. View the logs.


6 weeks ago

This ticket was mentioned in Slack in #core by jorbin. View the logs.


6 weeks ago

#21 @jorbin
6 weeks ago

  • Keywords needs-patch added; has-patch removed

Based on the idea that switching to :focus-visible wouldn't be beneficial, I think we will need a new patch

@wildworks
6 weeks ago

Add CSS for focus control to navigation submenu block

#22 @wildworks
6 weeks ago

I submitted a 60808-2.diff trying a more limited solution. This accomplishes the following for the Navigation Submenu block with "Open on click" enabled:

  • When clicking or right-clicking a menu that has a submenu via the mouse, the outline will be displayed as before.
  • When a submenu is opened, the menu has aria-expanded="true" applied. This will apply the newly added CSS from the patch and the menu's focus outline will disappear.
  • If the focus is moved using only the tab key on the keyboard, the focus outline of the menu will not disappear even if a submenu is opened.

This ticket was mentioned in Slack in #core by jorbin. View the logs.


5 weeks ago

#24 @sabernhardt
5 weeks ago

  • Keywords has-patch added; needs-patch removed
  • Milestone changed from 6.5.3 to 6.6

This ticket was mentioned in Slack in #core by jorbin. View the logs.


5 weeks ago

#26 @jorbin
5 weeks ago

  • Milestone changed from 6.6 to 6.5.3

Moving back into the 6.5.3 milestone since this would be a nice thing to fix.

@sabernhardt if you think we should punt, can you please share some rationale?

#27 @wildworks
4 weeks ago

I started thinking that solving this problem via theme.json might not be ideal. Because styles defined in the css field can be overridden by the user, but I don't think this style should be overridden.

Unfortunately, there is currently no way to enqueue styles only to the frontend via theme.json.

What do you think about using the wp_enqueue_scripts() hook instead?

@wildworks
4 weeks ago

The styles defined in theme.json are displayed in the global style additional CSS panel

#28 @poena
4 weeks ago

I disagree with adding and enqueueing a stylesheet because it's a convention that block themes are trying to move away from; it is part of reducing the complexity of theme building.

This ticket was mentioned in Slack in #core by jorbin. View the logs.


4 weeks ago

#30 @jorbin
4 weeks ago

  • Milestone changed from 6.5.3 to 6.6

With time running out to work on 6.5.3 and a lack of an agreed-upon direction, I'm moving this to 6.6. If a solution presents itself, I think it would be worth considering including in either a different minor or in an out-of-cycle theme-only release.

This ticket was mentioned in Slack in #accessibility by joesimpsonjr. View the logs.


4 weeks ago

#32 @karmatosed
3 days ago

I support what @poena is saying here. What are the other options based on that?

This ticket was mentioned in Slack in #core-themes by karmatosed. View the logs.


38 hours ago

Note: See TracTickets for help on using tickets.