fix(ContextualMenu): Context menu focuses first item on opening

[jmuzina]

Done

• Updated the ContextualMenu to focus the first non-disabled child item after opening. Then the user can tab through the items to access them via keyboard.
• Adds a keyboard focus trap to the contextual menu
• Updates the MultiSelect component to opt-out of the ContextualMenu autofocusing the first element, as this would make text search difficult (focus should move to the text input, not the first option in that case)

Future work

There is still more work that can be done to make this component more accessible & WCAG compliant. WCAG specifies that context menus should be navigable with Arrow keys, and Tab should close the context menu and focus the next item after it. This PR at least makes the context menu navigable with Tab - more work will need to be done to comply with this spec to add more compliant keyboard functionality and to return focus to the element that triggered the menu (see this issue)

I've left these changes for later task as they'll take a lot more work and I wanted to get the component at least minimally accessible in the short-term (a screen reader user raised with WPE that a context menu in 360 is almost impossible to navigate because focus does not move to it on opening and there is no focus trap)

QA

Pinging @canonical/react-library-maintainers for a review.

Storybook

To see rendered examples of all react-components, run:

yarn start

QA in your project

from react-components run:

yarn build
npm pack

Install the resulting tarball in your project with:

yarn add <path-to-tarball>

QA steps

• Open the ContextualMenu story.
• Use your keyboard to open the dropdown.
• See that focus has moved to the first link.
• Use tab and see that focus moves to the next link.
• Keep pressing tab and see that focus only moves within the context menu (focus wraps from the end to the front).
• Press escape and verify the context menu still closes. Focus should return to the button that opened the menu.
• Verify that no functional regressions have happened in the MultiSelect component, which consumes ContextualMenu.

Percy steps

• No visual changes expected

Fixes

Fixes: #1248

[webteam-app]

Demo

Jenkins

demos.haus

[jmuzina]

A possible improvement we can already add in this PR is to return focus to the element that opened the context menu when it closes.

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[fasih-mehmood]

I'm not able to close the dropdown using the keyboard. With the trapped focus, the focus switches back to the first option when pressing Tab at the last option. Pressing Esc does not close the dropdown either. Perhaps the dropdown should close if we press Tab at the last option or alternatively pressing Esc should close it. Wdyt?

[edlerd]

Did you check the CustomSelect component with this change? I suspect it might need to opt out as well -- at least if the search variant of it is in place.

[jmuzina]

I'm not able to close the dropdown using the keyboard

@fasih-mehmood Which story are you trying this out on, and which browser? In Chromium on the ContextualMenu toggle story, pressing escape closes the dropdown for me

[jmuzina]

Did you check the CustomSelect component with this change? I suspect it might need to opt out as well -- at least if the search variant of it is in place.

@edlerd No, I didn't check that one - thanks for flagging, I'll investigate

[fasih-mehmood]

I'm not able to close the dropdown using the keyboard

@fasih-mehmood Which story are you trying this out on, and which browser? In Chromium on the ContextualMenu toggle story, pressing escape closes the dropdown for me

@jmuzina I tried the ContextualMenu Toggle story on Chrome.

[ethanashaw]

You might consider cleaning up some of the effects using this pattern here, although it's not significantly better: https://react.dev/learn/you-might-not-need-an-effect#adjusting-some-state-when-a-prop-changes

I think a better option would be to actually assign focus as a part of the event handlers that toggle isOpen. I'll leave it up to you, I'm fine with the current code if you'd rather leave it.

[jmuzina]

Thanks @ethanashaw I've adjusted per your feedback to trigger the focus event from ContextualMenu's onOpen callback instead of with an effect in the ContextualMenuDropdown component, can you have another look?

[ethanashaw]

The "Inside modal" demo doesn't seem to work anymore. Other than that the code looks good, I just have one thing that might be helpful, your call again

[ethanashaw]

We might be able to get away without using an ID here if we use the dropdownProps.ref directly. I haven't tested this, but here's an example from Gemini:

import { useRef, useEffect } from 'react';

// No forwardRef wrapper needed in React 19!
function MyInput({ ref: externalRef, ...props }) {
  const internalRef = useRef(null);

  // Callback ref to sync both internal and external refs
  const mergedRef = (node) => {
    // 1. Assign to our internal ref for logic within this component
    internalRef.current = node;

    // 2. Assign to the ref passed by the parent
    if (typeof externalRef === 'function') {
      externalRef(node);
    } else if (externalRef) {
      externalRef.current = node;
    }
  };

  useEffect(() => {
    // Now you can use the node internally
    if (internalRef.current) {
      internalRef.current.style.backgroundColor = 'lightyellow';
    }
  }, []);

  return <input {...props} ref={mergedRef} />;
}

It's a bit messy because the dropdownProps.ref could be a callback or an object, but it seems like it would work if you want to avoid an ID here.

If we do this, we might also be able to consolidate a lot of these callbacks and effects into a single callback ref.

[jmuzina]

The "Inside modal" demo doesn't seem to work anymore.

@ethanashaw I noticed this too, but then I checked the production storybook and found the issue there as well, so I think it may be a pre-existing issue. I filed it as a bug: #1305

[jmuzina]

@ethanashaw just verifying, are you experiencing the problems @fasih-mehmood had earlier in this thread? Does QA work OK for you?

[ethanashaw]

@ethanashaw just verifying, are you experiencing the problems @fasih-mehmood had earlier in this thread? Does QA work OK for you?

@jmuzina, I'm actually seeing a different problem, but I investigated a bit more and realized it's a bit nuanced. If I tab to the dropdown button and use Enter to open it, the options become selected just fine. But, if I click the dropdown and then try to press Tab, the focus is stuck outside. This seemed to work fine in the previous commits.

[jmuzina]

If I tab to the dropdown button and use Enter to open it, the options become selected just fine. But, if I click the dropdown and then try to press Tab, the focus is stuck outside. This seemed to work fine in the previous commits.

Interesting, I'm not able to reproduce this on my side but from reading my implementation I can understand why it might be happening.

Here I don't call the focus function if event.detail is 0 (which happens when the dropdown is toggled by a mouse click). I can try to focus the menu itself (rather than the first item) if the toggle was triggered by a mouse, so that the next tab will be a list item. Interaction via keyboard can remain unchanged, wdyt

[ethanashaw]

I think we can put the issue I found aside for now, since the component should eventually be changed to use arrow keys anyway.

[jmuzina]

I think we can put the issue I found aside for now, since the component should eventually be changed to use arrow keys anyway.

The underlying issue here seems to be that the Modal has a focus trap that is intercepting tab events. The contextual menu items are not children of the modal (or the context menu trigger) - they are portaled under the document body. So, the Modal does not allow focus to move to the contextual menu items as it has no way of knowing it's a valid focus target.

This would require a more complex solution - we have two components with competing focus traps. Maybe we will need some focus stack provider that is able to move focus between elements that consume the focus stack; once you activate an element that consumes this (applying programmatic focus), that element should now be top of the focus stack and no other focus traps should be active. This can also enable returning focus to the element that opened the focus trap content when it is closed. This deserves a separate ticket - I can file one.

I'd like to check with at least one more WG member to see if this middle-ground solution (allowing both focus traps to operate simultaneously and accepting usability problems when one is opened inside another) - @edlerd @bartaz I'm curious about your thoughts here, what do you think?

[jmuzina]

Asking for UX review from @juanruitina and A11y review from @paul-geoghegan .

Edit: demos are currently failing to build... fixing that here: #1308