On desktop size, when clicking on an aria-current="page" navigation item all the nav items shift (see video) and the focus trap is engaged for the nav items. This happens even if the current item doesn't have a submenu.
The issue can be replicated in Docs and Storybook as well. https://primer.style/brand/components/SubNav Clicking on an active nav item will shift items to the right, clicking again will shift them back to the starting position. Try using Tab after you first click on an active item, the focus trap is engaged and the focus cannot leave the nav items.
The reason this happens
When an active item is clicked two empty focusable spans with a class .sentinel are added which are not absolutely positioned, and they interfere with the margins of the items (see screenshot).
Focusable span.sentinel elements are added to the ul element as first and the last child. (This is not semantically correct btw, since the only children ul should accept are li) On focus of these elements focus is immediately shifted to the first or last nav item. This is the most performant focus trap tactic.
I wonder why these span elements are not absolutely positioned with an inline style when created in @primer/behaviors - this would solve the shifting and won't interfere with tab order.
But then again the focus trap shouldn't be activated if there is no submenu.
Solution
Since we are using React I think we have flexibility here to easily implement our own custom focus trap system for this case. Or continue to use the existing system but restructure the component in a way that the mobile menu has a unique trigger button visible only on mobile and within the container of the nav. This will increase the DOM depth by one level but it's still not a major issue since this component is not too deep. This way we can solve some of the A11y issues outlined here:
Describe the bug
On desktop size, when clicking on an
aria-current="page"navigation item all the nav items shift (see video) and the focus trap is engaged for the nav items. This happens even if the current item doesn't have a submenu.The issue can be replicated in Docs and Storybook as well. https://primer.style/brand/components/SubNav Clicking on an active nav item will shift items to the right, clicking again will shift them back to the starting position. Try using Tab after you first click on an active item, the focus trap is engaged and the focus cannot leave the nav items.
The reason this happens
When an active item is clicked two empty focusable spans with a class
.sentinelare added which are not absolutely positioned, and they interfere with the margins of the items (see screenshot).This behavior is due
@primer/behaviors/src/focus-trap.tshttps://github.com/primer/behaviors/blob/19997006736fd6ca0aee6f6478ea27d7dd2a3e64/src/focus-trap.ts#L82-L95It is implemented in
useFocusTrapthat is used bySubNavhere https://github.com/primer/brand/blob/19161533eb0d90a73167bc57e8ffd8ef8579d647/packages/react/src/hooks/useFocusTrap.ts#L63Focusable
span.sentinelelements are added to theulelement as first and the last child. (This is not semantically correct btw, since the only childrenulshould accept areli) On focus of these elements focus is immediately shifted to the first or last nav item. This is the most performant focus trap tactic.I wonder why these
spanelements are not absolutely positioned with an inline style when created in@primer/behaviors- this would solve the shifting and won't interfere with tab order.But then again the focus trap shouldn't be activated if there is no submenu.
Solution
Since we are using React I think we have flexibility here to easily implement our own custom focus trap system for this case. Or continue to use the existing system but restructure the component in a way that the mobile menu has a unique trigger button visible only on mobile and within the container of the nav. This will increase the DOM depth by one level but it's still not a major issue since this component is not too deep. This way we can solve some of the A11y issues outlined here:
Screenshots
https://github.com/user-attachments/assets/3e627f0f-ba12-401e-9cd5-80c71fa6ce3b