Skip to content

Conversation

alec-mitnik
Copy link
Contributor

@alec-mitnik alec-mitnik commented Jul 7, 2025

Fixes the following for the Editor Menubar:

  • Opening a menu via clicking now correctly updates the tabindex values of the menubar items (instead of the submenu items).
  • Opening a menu via hover now correctly updates the tabindex values of the menubar items (instead of updating nothing due to erroneously passing in a menu rather than an ID).
  • Opening a menu via Enter, Space, or Arrow Up/Down now updates the tabindex values of the menubar items, since it's possible for the parent menuitem to have received keyboard focus other than through a way that would have already updated the tabindex values (e.g. by pressing on it and dragging off it before releasing).

Updated Menubar Editor Preview Link

Current Menubar Editor Example


WAI Preview Link (Last built on Thu, 07 Aug 2025 02:06:50 GMT).

Fixes the following:
- Opening a menu via clicking now correctly updates the tabindex values of the menubar items (instead of the submenu items).
- Opening a menu via hover now correctly updates the tabindex values of the menubar items (instead of updating nothing due to erroneously passing in a menu rather than an ID).
- Opening a menu via Enter, Space, or Arrow Up/Down now updates the tabindex values of the menubar items, since it's possible for the parent menuitem to have received keyboard focus other than through a way that would have already updated the tabindex values (e.g. by pressing on it and dragging off it before releasing).
@alec-mitnik alec-mitnik changed the title Focus handling fixes Editor Menubar Focus handling fixes Jul 7, 2025
Copy link

@Imran-imtiaz48 Imran-imtiaz48 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This update to the MenubarEditor class shows thoughtful enhancements in keyboard navigation and focus management, especially for popup menus. You've correctly ensured that when triggering a submenu with the Enter or space key, both the parent and the child menu receive proper focus, which is crucial for accessibility and screen reader support. The addition of setFocusToFirstMenuitem and setFocusToLastMenuitem within relevant conditional blocks improves the user experience by aligning with standard navigation expectations. Also, ensuring the parent menu item is focused when toggling popups reflects good attention to detail. Overall, these changes are clean, purposeful, and contribute positively to usability and accessibility. Well done.

@css-meeting-bot
Copy link
Member

The ARIA Authoring Practices (APG) Task Force just discussed PR 3309: Proposed changes to focus handling in editor menubar.

The full IRC log of that discussion <jugglinmike> Topic: PR 3309: Proposed changes to focus handling in editor menubar
<jugglinmike> github: https://github.com//pull/3309
<jugglinmike> Matt_King: jongund was assigned to this, but he isn't present today
<jugglinmike> Matt_King: This is a set of three changes to the way that JavaScript is updating the tab index in the editor menubar
<jugglinmike> Matt_King: The first two are in response to mouse/pointer actions, and the third is in response to keyboard actions
<jugglinmike> Matt_King: I tested it from the perspective of a keyboard user, and I didn't see any changes in the behavior
<jugglinmike> Matt_King: I'm not clear whether change is expected here
<jugglinmike> Matt_King: And I wasn't able to test the mouse-related changes
<jugglinmike> arigilmore: I can try taking a look
<jugglinmike> Matt_King: That would be great, arigilmore--thank you!
<jugglinmike> Matt_King: This is all about when people are using both the mouse and the keyboard, is that right?
<jugglinmike> arigilmore: yes, when you use a mouse, clicking and hovering should change it. And also (from the keyboard) tab, enter, space, up, and down
<jugglinmike> Matt_King: I want to make sure we don't regress specific existing behavior: if you have your mouse in an area where a sub-menu would appear (if you opened it)--so it's in an empty space--and then you use the keyboard to open the menu. If the mouse happens to be sitting on the place where the submenu item would appear, then the underlying submenu item gets expanded automatically
<jugglinmike> Matt_King: This happens by accident quite often
<jugglinmike> arigilmore: Is there a related pull request?
<jugglinmike> Matt_King: We can probably find it if we search for pull requests related to "menu" and "hover"
<jugglinmike> arigilmore: maybe it's this one--
<jugglinmike> arigilmore: Oh, no, this one is from 2018. That's quite old
<jugglinmike> Matt_King: I think the one we're looking for was opened in the last year or so
<jugglinmike> Jem: I've assigned arigilmore to 3309

@ariellalgilmore
Copy link
Contributor

Hi @alec-mitnik do you mind clarifying the issue you are solving here? What steps should we reproduce to see the current issue that this is solving?

@alec-mitnik
Copy link
Contributor Author

Hi @alec-mitnik do you mind clarifying the issue you are solving here? What steps should we reproduce to see the current issue that this is solving?

Sure, let me try to be more clear about the current behavior being addressed for each bullet point in the pull request description.

Opening a menu via clicking:

  1. Start with a fresh load of the page, and try traversing the editor menubar using keyboard navigation (tab key) to see that only the first menu, "Font," is part of the navigation sequence.
  2. Use a mouse click to open a different menu, such as "Text Align."
  3. Traverse the editor menubar again using keyboard navigation (tab key), and you'll see that this new menu is now part of the navigation sequence, but the first menu still remains a part of the sequence as well. Expected behavior would be for only the most recently opened menu to be part of the navigation sequence, even if opened with a mouse click.

Opening a menu via hovering:

  1. Start with a fresh load of the page, and try traversing the editor menubar using keyboard navigation (tab key) to see that only the first menu, "Font," is part of the navigation sequence.
  2. With focus on the menu, open it with the keyboard (enter/space/up/down).
  3. Use the mouse to hover over a different menu, such as "Text Align," causing it to open (and close the first menu).
  4. Traverse the editor menubar again using keyboard navigation (tab key), and you'll see that this new menu is now part of the navigation sequence, but the first menu still remains a part of the sequence as well. Expected behavior would be for only the most recently opened menu to be part of the navigation sequence, even if opened by hovering with the mouse.

Opening a menu via keyboard:

  1. Start with a fresh load of the page, and try traversing the editor menubar using keyboard navigation (tab key) to see that only the first menu, "Font," is part of the navigation sequence.
  2. Bring focus to a different menu, such as "Text Align," without using the keyboard or causing it to open. This can be done by hovering over it with the mouse, holding down the mouse button, dragging off of the menubar, and releasing.
  3. With focus now on the new menu, open it with the keyboard (enter/space/up/down).
  4. Traverse the editor menubar again using keyboard navigation (tab key), and you'll see that this new menu has not become part of the navigation sequence, while the first menu still remains a part of the sequence. Expected behavior would be for only the most recently opened menu to be part of the navigation sequence, no matter how it was opened or had originally received focus.

Copy link
Contributor

@ariellalgilmore ariellalgilmore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation. It LGTM! seems like the deploy preview isn't working properly, but I see it working locally

@css-meeting-bot
Copy link
Member

The ARIA Authoring Practices (APG) Task Force just discussed PR 3309: Proposed changes to focus handling in editor menubar.

The full IRC log of that discussion <jugglinmike> Topic: PR 3309: Proposed changes to focus handling in editor menubar
<jugglinmike> github: https://github.com//pull/3309
<jugglinmike> Matt_King: Once I read the detailed explanation (thanks arigilmore for requesting that), I understood that these are great changes
<jugglinmike> arigilmore: I could only see the changes when running it locally. Nothing was different on the deploy preview
<jugglinmike> arigilmore: Adam_Page expects that something is broken with the deploy preview system
<jugglinmike> arigilmore: But it all looks good when running locally
<jugglinmike> Matt_King: And it says that all checks have run and that all checks have passed
<jugglinmike> arigilmore: The code looks good to me
<jugglinmike> Matt_King: Do we need any other kind of review?
<jugglinmike> Matt_King: It doesn't change look or appearance
<jugglinmike> arigilmore: Right. it just alters the behavior in certain circumstances
<jugglinmike> Matt_King: I would like to be able to test this before we do the final merge
<jugglinmike> howard-e: This is an issue with the Netify cache
<jugglinmike> howard-e: I recently had an idea about what is going wrong, and I'm going to investigate that theory today
<jugglinmike> howard-e: But for the short-term, I can manually trigger the build for this pull request
<jugglinmike> arigilmore: We were observing two elements with "tabindex" set to zero
<jugglinmike> Matt_King: Thank you so much for your work on this; this is great!

@howard-e
Copy link
Contributor

howard-e commented Aug 5, 2025

@ariellalgilmore thanks for reporting the deploy issue! It should be updated now

@ariellalgilmore
Copy link
Contributor

thanks @howard-e! everything looks good!

@mcking65 mcking65 changed the title Editor Menubar Focus handling fixes Editor Menubar Example: Fix tabindex management to ensure only the most recently opened menu is in the page tab sequence Aug 12, 2025
@mcking65 mcking65 merged commit ccfa79e into w3c:main Aug 12, 2025
14 checks passed
@mcking65
Copy link
Contributor

@alec-mitnik thank you for this fabulous contribution! It will be in the next publication; date is still TBD.

@mcking65 mcking65 added this to the Next publication milestone Aug 12, 2025
@alec-mitnik alec-mitnik deleted the patch-1 branch August 12, 2025 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants