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

[Menu][Divider] Do not allow focus on Divider when inside Menu list #38102

Merged
merged 7 commits into from
Jul 28, 2023

Conversation

divyammadhok
Copy link
Contributor

@divyammadhok divyammadhok commented Jul 22, 2023

Fixes #37306

Description

The issue was that if a list of menu items also contains a divider, and all menu items are disabled, tabindex=0 will be set on the divider.

@mui-bot
Copy link

mui-bot commented Jul 22, 2023

Netlify deploy preview

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

Bundle size report

Details of bundle changes (Toolpad)
Details of bundle changes

Generated by 🚫 dangerJS against 9ece875

@divyammadhok divyammadhok marked this pull request as draft July 22, 2023 21:04
@divyammadhok divyammadhok force-pushed the menu-disabled-focus-fix branch from 87dbbce to 9e9bec3 Compare July 22, 2023 21:48
@divyammadhok divyammadhok marked this pull request as ready for review July 22, 2023 21:57
@zannager zannager added component: divider This is the name of the generic UI component, not the React module! component: menu This is the name of the generic UI component, not the React module! labels Jul 24, 2023
@zannager zannager requested a review from mnajdova July 24, 2023 11:35
Copy link
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.

@divyammadhok The logic makes sense. Could you kindly add a test case to verify the fix? It can be added in packages\mui-material\src\Menu\Menu.test.js. Thanks.

@ZeeshanTamboli ZeeshanTamboli added bug 🐛 Something doesn't work package: material-ui Specific to @mui/material PR: needs test The pull request can't be merged labels Jul 28, 2023
@ZeeshanTamboli ZeeshanTamboli changed the title [Menu][Divider] Divider gets tabIndex 0 when all menu items are disabled [Menu][Divider] Do not focus on Divider when inside Menu list Jul 28, 2023
@ZeeshanTamboli ZeeshanTamboli removed the PR: needs test The pull request can't be merged label Jul 28, 2023
@ZeeshanTamboli ZeeshanTamboli changed the title [Menu][Divider] Do not focus on Divider when inside Menu list [Menu][Divider] Do not allow focus on Divider when inside Menu list Jul 28, 2023
Copy link
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.

@divyammadhok Thanks for the contribution!

@ZeeshanTamboli ZeeshanTamboli merged commit f502467 into mui:master Jul 28, 2023
@divyammadhok
Copy link
Contributor Author

@ZeeshanTamboli Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: divider This is the name of the generic UI component, not the React module! component: menu This is the name of the generic UI component, not the React module! package: material-ui Specific to @mui/material
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Menu][Divider] Divider gets tabIndex 0 when all menu items are disabled
4 participants