fix(contextual-menu): allow Escape key to close menu inside modal (#1…

[NamanMehta16]

…305)

Done

• ContextualMenu now closes when pressing Escape inside a Modal.
• Added handleKeyDown logic and ensured focus handling works with portals.
• Updated tests to cover ESC key behavior inside Modals.

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 a Modal with a ContextualMenu inside.
• Open the menu and press ESC.
• Ensure the menu closes and focus returns to the trigger button.

Percy steps

• No visual changes expected.

Fixes

Fixes: # .

[webteam-app]

NamanMehta16 is not a collaborator of the repo

[edlerd]

Thanks for the contribution, one clarifying question below.

[edlerd]

Code LGTM.

The comments are a bit confusing. Maybe remove or rephrase to communicate better the reason for the code below them.

[edlerd]

Suggested change

	// Ensure portal is closed when isOpen becomes false,
	// even if keydown handler is not reached

I don't understand how the comment relates to the code below.

[edlerd]

Suggested change

// capture phase helps when modal stops propagation

Not sure what this is supposed to mean. It seems confusing to me.

[NamanMehta16]

Thanks for the review and feedback. I have addressed the lint errors and refined the comments to better explain the changes. Happy to make any further updates if needed.

[edlerd]

Thanks for the update. This looks mostly good to merge. Just some thoughts on the comments above and some linter failures below. Please take a look.

[edlerd]

Thanks for the update and sorry for the confusion. The closeOnEsc is already a feature of the contextual menu component. It defaults to true so we don't need an additional hook for triggering the close on esc key press. In the default Storybook for it the escape key already works as a closing trigger.

[NamanMehta16]

Hi @edlerd,
Thanks for the clarification about closeOnEsc.
I just wanted to check, is this ticket still valid, or is there something I might be misunderstanding?
#1305

[jmuzina]

I believe the ticket #1305 is still valid (its reproduction steps still demonstrate contextual menus inside modals not closing when ESC is pressed, even with closeOnEsc set). The issue isn't that escape never closes the menu, it's that it doesn't close the menu in this specific case when it should.

[edlerd]

I believe the ticket #1305 is still valid (its reproduction steps still demonstrate contextual menus inside modals not closing when ESC is pressed, even with closeOnEsc set). The issue isn't that escape never closes the menu, it's that it doesn't close the menu in this specific case when it should.

Yes, that is a bug in the modal component, not the contextual menu. See my comment over there.