Skip to content

Components: Add autoFocus prop to AlignmentMatrixControl, use for Cover block toolbar - #79354

Open
aduth wants to merge 4 commits into
trunkfrom
update/block-alignment-focus-race-condition
Open

Components: Add autoFocus prop to AlignmentMatrixControl, use for Cover block toolbar#79354
aduth wants to merge 4 commits into
trunkfrom
update/block-alignment-focus-race-condition

Conversation

@aduth

@aduth aduth commented Jun 19, 2026

Copy link
Copy Markdown
Member

What?

Adds a new autoFocus prop to the AlignmentMatrixControl component and updates the Cover block to use it in place of relying on Dropdown focusOnMount.

Why?

Seeks to avoid a possible race condition between AlignmentMatrixControl's asynchronous initialization and focusOnMount's asynchronous detection of focusable elements.

The extended commit description has more details:

Ariakit Composite initialization is asynchronous via requestAnimationFrame, because its roving tabindex can only be computed once all cell items are rendered. This asynchronous initialization can conflict with other autofocus behaviors which themselves may be asynchronous in different ways (e.g. useFocusOnMount scheduling via setTimeout(0)). In the browser this is usually fine, but it's not a strong guarantee, and breaks in environments like JSDOM which define their own unique behavior for scheduling (rAF as a 16ms timeout).

If useFocusOnMount runs before composite initializes, all composite items are tabbable, and the first cell is selected, but the expected behavior would be that focus is only assigned once initialized, since it may not be the first cell that's the active one.

This was motivated by #79353, which had to work around this issue by introducing a test utility overriding requestAnimationFrame to emulate the browser behavior. But the browser behavior is not a strong guarantee, and there is a real risk of a race condition.

Testing Instructions

Verify that Cover block tests pass: npm run test:unit packages/block-library/src/cover/test/edit.js (note that Cover block is the only consumer of this component)

Verify that focus shifts to the alignment matrix center cell and does not dirty post in doing so:

  1. Go to Posts > Add Post
  2. Add a Cover block
  3. Pick a color or assign an image
  4. Save the post
  5. Select the Cover block container so that the block toolbar is shown
  6. Click the "Change content position" block toolbar item (or activate it with keyboard, which is preferable because it'll show a focus indicator in the next step)
  7. Observe that the "Center" cell is selected and focused, and that the post is not dirtied ("Saved" is shown in the top-right of the editor)

Screenshots or screencast

image

Use of AI Tools

There was a lot of deep research here between Cursor Composer and Claude Opus 4.8 models. Implementations were initially authored by AI, with further refinement and iterations based on continued research. This was split from work that started in #79353.

aduth added 3 commits June 19, 2026 09:17
Ariakit Composite initialization is asynchronous via requestAnimationFrame, because its roving tabindex can only be computed once all cell items are rendered. This asynchronous initialization can conflict with other autofocus behaviors which themselves may be asynchronous in different ways (e.g. useFocusOnMount scheduling via `setTimeout(0)`). In the browser this is usually fine, but it's not a strong guarantee, and breaks in environments like JSDOM which define their own unique behavior for scheduling (rAF as a 16ms timeout).

If useFocusOnMount runs before composite initializes, all composite items are tabbable, and the first cell is selected, but the expected behavior would be that focus is only assigned once initialized, since it may not be the first cell that's the active one.
@aduth
aduth requested review from a team, ajitbohra and ellatrix as code owners June 19, 2026 13:44
@github-actions

github-actions Bot commented Jun 19, 2026

Copy link
Copy Markdown

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: aduth <aduth@git.wordpress.org>
Co-authored-by: mirka <0mirka00@git.wordpress.org>
Co-authored-by: ciampo <mciampini@git.wordpress.org>

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

@github-actions github-actions Bot added [Package] Components /packages/components [Package] Block editor /packages/block-editor labels Jun 19, 2026
@aduth aduth changed the title Components: Add autoFocus prop to AlignmentMatrixControl Components: Add autoFocus prop to AlignmentMatrixControl, use for Cover block toolbar Jun 19, 2026
@aduth aduth added the [Type] Bug An existing feature does not function as intended label Jun 19, 2026
@github-actions

Copy link
Copy Markdown

Size Change: +56 B (0%)

Total Size: 7.5 MB

📦 View Changed
Filename Size Change
build/scripts/block-editor/index.min.js 380 kB +21 B (+0.01%)
build/scripts/components/index.min.js 264 kB +35 B (+0.01%)

compressed-size-action

return (
<Dropdown
popoverProps={ { placement: 'bottom-start' } }
popoverProps={ { placement: 'bottom-start', focusOnMount: false } }

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It appears we can't really do this because disabling focusOnMount opts us out from some necessary keyboard navigation behaviors:

  1. Focus trapping when tabbing within the popover.
  2. Returning focus to the toolbar item after closing the popover with Escape.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Good catch, though I find it a bit surprising that those behaviors are linked to a prop which advertises itself as controlling an "on mount" behavior. I'll give this a think. And the absence of failing tests on this regression is separately concerning.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We need to hook it up with getWpCompatOverlaySlot() first, but this may be a good place to try swapping the Dropdown with the new Popover and see if the behavior is any better.

@ciampo

ciampo commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

As a less elegant (but simpler) alternative, could we programmatically focus the correct DOM element in the renderContent prop (it should receive thes isOpen argument)?

I also agree with @mirka's suggestion to test the new ui/Popover

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Package] Block editor /packages/block-editor [Package] Components /packages/components [Type] Bug An existing feature does not function as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants