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

[Menu] Fix autoFocus to work correctly with keepMounted #16450

Merged
merged 4 commits into from
Jul 3, 2019
Merged

[Menu] Fix autoFocus to work correctly with keepMounted #16450

merged 4 commits into from
Jul 3, 2019

Conversation

ryancogswell
Copy link
Contributor

Closes #16444

@mui-pr-bot
Copy link

mui-pr-bot commented Jul 2, 2019 •
edited
Loading

Details of bundle changes.

Comparing: 7fb9b40...21a725d

bundle parsed diff gzip diff prev parsed current parsed prev gzip current gzip
@material-ui/core 0.00% +0.01% 🔺 320,029 320,034 88,270 88,276
@material-ui/core/Paper 0.00% 0.00% 68,291 68,291 20,369 20,369
@material-ui/core/Paper.esm 0.00% 0.00% 61,573 61,573 19,161 19,161
@material-ui/core/Popper 0.00% 0.00% 28,945 28,945 10,395 10,395
@material-ui/core/Textarea 0.00% 0.00% 5,507 5,507 2,358 2,358
@material-ui/core/TrapFocus 0.00% 0.00% 3,753 3,753 1,578 1,578
@material-ui/core/styles/createMuiTheme 0.00% 0.00% 16,009 16,009 5,791 5,791
@material-ui/core/useMediaQuery 0.00% 0.00% 2,597 2,597 1,102 1,102
@material-ui/lab 0.00% 0.00% 140,210 140,210 43,456 43,456
@material-ui/styles 0.00% 0.00% 51,698 51,698 15,348 15,348
@material-ui/system 0.00% 0.00% 15,420 15,420 4,391 4,391
Button 0.00% 0.00% 84,316 84,316 25,722 25,722
Modal 0.00% 0.00% 14,427 14,427 5,087 5,087
Portal 0.00% 0.00% 3,473 3,473 1,572 1,572
Slider 0.00% 0.00% 74,889 74,889 23,293 23,293
colorManipulator 0.00% 0.00% 3,904 3,904 1,544 1,544
docs.landing 0.00% 0.00% 54,833 54,833 13,892 13,892
docs.main 0.00% 0.00% 647,269 647,274 203,922 203,928
packages/material-ui/build/umd/material-ui.production.min.js 0.00% +0.01% 🔺 293,180 293,185 84,176 84,181

Generated by 🚫 dangerJS against 21a725d

eps1lon
eps1lon reviewed Jul 2, 2019
View reviewed changes
Copy link
Member

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

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

Would be nice to have a test for it (even if we intend to break it soon).

@eps1lon eps1lon added accessibility a11y bug 🐛 Something doesn't work component: menu This is the name of the generic UI component, not the React module! labels Jul 2, 2019
eps1lon
eps1lon reviewed Jul 2, 2019
View reviewed changes
packages/material-ui/test/integration/Menu.test.js Outdated
let wrapper;

before(() => {
// Using render instead of createClientRender because createClientRender specifies an explicit
Copy link
Member

Choose a reason for hiding this comment

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

Yeah current situation isn't ideal because many tests aren't isolated. Later I would actively block setting baseElement since we can't isolate aria scopes anyway and always need to consider the whole document.

@ryancogswell
Copy link
Contributor Author

I think the two failed checks are just flakiness unrelated to my changes, but let me know if there is something I should do to address it.

@eps1lon
Copy link
Member

eps1lon commented Jul 2, 2019

Could you do a quick git commit --allow-empty -m 'poke azure'? It refuses to rebuild from the UI. (totally blaming olivier for this 😄)

@eps1lon eps1lon merged commit ee18aac into mui:master Jul 3, 2019
@eps1lon
Copy link
Member

eps1lon commented Jul 3, 2019

Very nice, thanks!

@ryancogswell ryancogswell mentioned this pull request Jul 9, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@eps1lon eps1lon eps1lon left review comments

Assignees
No one assigned
Labels
accessibility a11y bug 🐛 Something doesn't work component: menu This is the name of the generic UI component, not the React module!
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Menu keyboard navigation broken in demos
3 participants
@ryancogswell @mui-pr-bot @eps1lon