Skip to content

[dialog][drawer][focus trap] Fix initial focus target#48280

Merged
mj12albert merged 4 commits intomui:masterfrom
mj12albert:fix/initial-focus
Apr 18, 2026
Merged

[dialog][drawer][focus trap] Fix initial focus target#48280
mj12albert merged 4 commits intomui:masterfrom
mj12albert:fix/initial-focus

Conversation

@mj12albert
Copy link
Copy Markdown
Member

@mj12albert mj12albert commented Apr 13, 2026 •
edited by ZeeshanTamboli
Loading

Alternative to #37672 that avoids breaking changes

A private data attribute is used to mark the correct focus target

Testing steps:

  1. Requires a Dialog with fullScreen, scroll="body", no autoFocusing elements in the content, and very long content (helps observe keyboard scrolling)
  2. Click the trigger to open
  3. Immediately ArrowDown/PageDown/End to scroll

Expected: it scrolls, focus ring can be seen around the dialog popup (Stackblitz)

Before the fix: doesn't scroll because the wrong thing is focused (Stackblitz)

Fixes #42989

@mj12albert mj12albert added scope: dialog Changes related to the dialog. scope: drawer Changes related to the drawer. scope: focus trap Changes related to the focus trap. labels Apr 13, 2026
@mui-bot
Copy link
Copy Markdown

mui-bot commented Apr 13, 2026 •
edited
Loading

Netlify deploy preview

https://deploy-preview-48280--material-ui.netlify.app/

Bundle size report

Bundle Parsed size Gzip size
@mui/material 🔺+211B(+0.04%) 🔺+73B(+0.05%)
@mui/lab 0B(0.00%) 0B(0.00%)
@mui/system 0B(0.00%) 0B(0.00%)
@mui/utils 0B(0.00%) 0B(0.00%)

Details of bundle changes

Generated by 🚫 dangerJS against 725a4a7

@code-infra-dashboard
Copy link
Copy Markdown

code-infra-dashboard bot commented Apr 15, 2026 •
edited
Loading

Bundle size

Bundle Parsed size Gzip size
@mui/material 🔺+213B(+0.04%) 🔺+77B(+0.05%)
@mui/lab 0B(0.00%) 0B(0.00%)
@mui/private-theming 0B(0.00%) 0B(0.00%)
@mui/system 0B(0.00%) 0B(0.00%)
@mui/utils 0B(0.00%) 0B(0.00%)

Details of bundle changes

Deploy preview

https://deploy-preview-48280--material-ui.netlify.app/

Check out the code infra dashboard for more information about this PR.

@mj12albert mj12albert marked this pull request as ready for review April 15, 2026 16:50
ZeeshanTamboli
ZeeshanTamboli reviewed Apr 16, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@ZeeshanTamboli ZeeshanTamboli left a comment

Choose a reason for hiding this comment

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

It works but does this solution feel like a workaround?

Comment thread packages/mui-material/src/Dialog/Dialog.js Outdated
@ZeeshanTamboli ZeeshanTamboli added the type: bug It doesn't behave as expected. label Apr 16, 2026
@mj12albert
Copy link
Copy Markdown
Member Author

mj12albert commented Apr 16, 2026 •
edited
Loading

It works but does this solution feel like a workaround?

It's how Floating UI does it 😅
(explicit, easy to understand, quite error-proof)

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Apr 16, 2026
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Apr 16, 2026
@mj12albert
Copy link
Copy Markdown
Member Author

mj12albert commented Apr 17, 2026 •
edited
Loading

Possibly this also fixes:

Doesn't fix directly but should help

ZeeshanTamboli
ZeeshanTamboli approved these changes Apr 17, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@ZeeshanTamboli ZeeshanTamboli left a comment •
edited
Loading

Choose a reason for hiding this comment

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

One TS related comment. Rest looks good.

Comment thread packages/mui-material/src/utils/focusable.ts Outdated
@ZeeshanTamboli
Copy link
Copy Markdown
Member

ZeeshanTamboli commented Apr 17, 2026

Possibly this also fixes:

Can you check?

@mj12albert mj12albert added v7.x needs cherry-pick The PR should be cherry-picked to master after merge. labels Apr 17, 2026
@mj12albert mj12albert enabled auto-merge (squash) April 18, 2026 15:25
@mj12albert mj12albert merged commit 3dcaf4b into mui:master Apr 18, 2026
19 checks passed
@github-actions
Copy link
Copy Markdown

github-actions bot commented Apr 18, 2026

Cherry-pick PRs will be created targeting branches: v7.x

github-actions bot pushed a commit that referenced this pull request Apr 18, 2026
@mj12albert @web-flow
[dialog][drawer][focus trap] Fix initial focus target (#48280)
86b074f
@github-actions github-actions bot mentioned this pull request Apr 18, 2026
Merged
@mj12albert mj12albert deleted the fix/initial-focus branch April 18, 2026 17:43
@mj12albert mj12albert mentioned this pull request Apr 18, 2026
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ZeeshanTamboli ZeeshanTamboli ZeeshanTamboli approved these changes

@silviuaavram silviuaavram Awaiting requested review from silviuaavram

@siriwatknp siriwatknp Awaiting requested review from siriwatknp

Assignees

No one assigned

Labels

accessibility a11y needs cherry-pick The PR should be cherry-picked to master after merge. scope: dialog Changes related to the dialog. scope: drawer Changes related to the drawer. scope: focus trap Changes related to the focus trap. type: bug It doesn't behave as expected. v7.x

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[material-ui][Dialog] Keyboard scrolling doesn't work in fullscreen mode

3 participants

@mj12albert @mui-bot @ZeeshanTamboli