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.

Sign up for GitHub

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

Jump to bottom

Close patterns modal on insertion and focus on inserted pattern #68975

Merged
merged 4 commits into from
Feb 27, 2025
Merged

Close patterns modal on insertion and focus on inserted pattern #68975

merged 4 commits into from
Feb 27, 2025
+8 −3

Conversation

yogeshbhutkar
Copy link
Contributor

What?

Closes #68728
Alternate to #68730

This PR updates the Patterns Modal to close upon the insertion of a Pattern and moves the focus back to the inserted pattern.

Why?

  • The current behavior of the Pattern Explorer modal was an unintended change introduced in:
    Inserter: Remove the dialog behaviour #63059.
  • Fixes the accessibility bug where using the keyboard to insert a pattern from the modal correctly aligns focus with the inserted item.

How?

  1. Added interface store integration for modal management.
  2. Updated pattern insertion flow to respect the modal state.
  3. Added close modal functionality after pattern insertion.

Testing Instructions

  1. Navigate to the post-edit page.
  2. From the Block Inserter navigate to the Patterns tab.
  3. Open the Patterns Modal by clicking Explore all patterns.
  4. Click on a Pattern from the Modal to choose and observe the modal closes on selection.
  5. Also, observe the focus move back to the inserted pattern.

Screencast

pr-demo.mov

@yogeshbhutkar yogeshbhutkar mentioned this pull request Jan 31, 2025
Closed
Mamaduka
Mamaduka reviewed Jan 31, 2025
View reviewed changes
packages/block-editor/src/components/inserter/block-patterns-tab/index.js Outdated
Comment on lines 29 to 32
const isModalActive = useSelect( ( select ) =>
select( interfaceStore ).isModalActive( PATTERNS_MODAL_NAME )
);
const { openModal, closeModal } = useDispatch( interfaceStore );
Copy link
Member

Choose a reason for hiding this comment

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

I think this global model stage management method should be served for high-level packages like editor and not used by block-editor.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Given this, I believe we should consider closing #54199 as unplanned. 🤔✅

@Mamaduka
Copy link
Member

Can we just pass down the onModalClose prop and call it alongside the onClickPattern in the PatternList component?

@yogeshbhutkar
Copy link
Contributor Author

Yes, we can. Maybe I overengineered with this one 😅

@yogeshbhutkar yogeshbhutkar force-pushed the fix-68728/pattern-modal-regression branch from 27666cf to 980821c Compare January 31, 2025 06:35
@Mamaduka Mamaduka added [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Regression Related to a regression in the latest release [Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced labels Jan 31, 2025
@yogeshbhutkar yogeshbhutkar marked this pull request as ready for review January 31, 2025 07:23
@github-actions GitHub Actions
Copy link

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: yogeshbhutkar <[email protected]>
Co-authored-by: Mamaduka <[email protected]>
Co-authored-by: t-hamano <[email protected]>
Co-authored-by: afercia <[email protected]>
Co-authored-by: carolinan <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@yogeshbhutkar
Copy link
Contributor Author

The parent issue remains unresolved, and this PR aims to restore the previous behavior to fix the regression. I’d appreciate any feedback on whether this is a viable path forward or if there’s a better approach. The current implementation is based on this comment.

@yogeshbhutkar yogeshbhutkar mentioned this pull request Feb 27, 2025
Closed
6 tasks
@Mamaduka
Copy link
Member

@yogeshbhutkar, let's rebase on top of the latest trunk. I'll try to have a look later today, but I think it is also okay to merge this bug after Beta 1.

@yogeshbhutkar yogeshbhutkar force-pushed the fix-68728/pattern-modal-regression branch from b6b5bce to 9395980 Compare February 27, 2025 07:00
Mamaduka
Mamaduka approved these changes Feb 27, 2025
View reviewed changes
Copy link
Member

@Mamaduka Mamaduka left a comment

Choose a reason for hiding this comment

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

Thank you, @yogeshbhutkar!

This works as expected ✅

@Mamaduka Mamaduka merged commit df6f07d into WordPress:trunk Feb 27, 2025
60 checks passed
@github-actions github-actions bot added this to the Gutenberg 20.4 milestone Feb 27, 2025
@yogeshbhutkar yogeshbhutkar deleted the fix-68728/pattern-modal-regression branch February 27, 2025 12:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Mamaduka Mamaduka Mamaduka approved these changes

@ellatrix ellatrix Awaiting requested review from ellatrix ellatrix is a code owner

Assignees
No one assigned
Labels
[Feature] Patterns A collection of blocks that can be synced (previously reusable blocks) or unsynced [Focus] Accessibility (a11y) Changes that impact accessibility and need corresponding review (e.g. markup changes). [Type] Regression Related to a regression in the latest release
Projects
None yet
Milestone
Gutenberg 20.4
Development

Successfully merging this pull request may close these issues.

Snackbar: Display Snackbar on top of Modal Screen Overlay
2 participants
@yogeshbhutkar @Mamaduka