Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DataViews: action modals without a button do not close when pressing the Esc key #69265

Open
oandregal opened this issue Feb 20, 2025 · 3 comments
Labels
[Feature] DataViews Work surrounding upgrading and evolving views in the site editor and beyond [Type] Bug An existing feature does not function as intended

Comments

@oandregal
Copy link
Member

oandregal commented Feb 20, 2025

Description

DataViews supports adding actions that have modals. When a modal is open, it can be closed by pressing the Esc key, but only if the modal has a button that calls the closeModal on click.

Step-by-step reproduction instructions

Modify the DataViews story for render modals:

  • Go to the DataViews story, where the render modal action is defined (link to code).
  • Change hideModalHeader to modalHeader: 'Delete action'.
  • Remove the buttons from the RenderModal component.

Run the storybook:

  • npm install && npm run storybook:dev
  • It'll automatically open a browser. Go to the DataViews default story at http://localhost:50240/?path=/docs/dataviews-dataviews--docs
  • Open the "Delete item" action and verify that it has a header with a close button.
  • Press the Esc key in the keyboard. The expected result was that it closed the modal, but it didn't.
  • Verify that pressing the X button in the modal header does close the modal.
Image
@oandregal oandregal added [Type] Bug An existing feature does not function as intended [Feature] DataViews Work surrounding upgrading and evolving views in the site editor and beyond labels Feb 20, 2025
@oandregal
Copy link
Member Author

cc @WordPress/gutenberg-components for awareness.

@himanshupathak95
Copy link
Contributor

Hey @oandregal, Thanks for raising the issue.

Upon reproduction and investigation, I found out that after removing the button, the modal is not focused. That is the reason pressing Esc key doesn't close the modal.

I observed that if we manually focus the modal after it opens, pressing Esc successfully closes the modal -

Screen.Recording.2025-02-21.at.13.43.39.mov

So, when the modal is mounted to the DOM, we can shift the focus to it, thereby activating all the keypress functionalities.

@himanshupathak95
Copy link
Contributor

Currently, focusOnMount="firstContentElement" doesn't work well when the modal lacks interactive elements. I propose updating the ActionModal component to use a priority-based focus strategy:

  • Use "firstElement" when the modal has form inputs (ensuring immediate input focus and respecting the changes made in Align naming modals #62788)
  • Fall back to "firstContentElement" for modals with headers but no inputs
  • Use true as a final fallback to focus the modal itself

Something like this should work and provide a better focus on different scenarios. After finalizing the approach and outcomes maybe I can take this up. Better approaches are always welcomed :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] DataViews Work surrounding upgrading and evolving views in the site editor and beyond [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

2 participants