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

[website] Fix navigation menu close behavior #33203

Merged

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Jun 17, 2022

The problem was first surfaced in #33125 (comment):

I can never go to the MUI Base and MUI System docs, sometimes can't go to the Joy UI docs. See the video for why:

20220614-115713-413.mp4

I have implemented the solution suggested in #33125 (comment). I didn't try https://bjk5.com/post/44698559168/breaking-down-amazons-mega-dropdown as it feels overkill.

I can reproduce the same UX problem on https://stripe.com/en-fr and https://www.radix-ui.com/docs/primitives/components/navigation-menu we are a taking a step ahead :).

Preview: https://deploy-preview-33203--material-ui.netlify.app/

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work website Pages that are not documentation-related, marketing-focused. labels Jun 17, 2022
@mui-bot
Copy link

mui-bot commented Jun 17, 2022

No bundle size changes

Generated by 🚫 dangerJS against 8749979

onFocus={() => setSubMenuOpen(true)}
onMouseOut={() => setSubMenuOpen(false)}
onBlur={() => setSubMenuOpen(false)}
onMouseEnter={() => setSubMenuOpenUndebounce('products')}
Copy link
Member Author

Choose a reason for hiding this comment

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

@siriwatknp I think that we should default to onMouseEnter/onMouseLeave over onMouseOver/onMouseOut because the latter fires unnecessarily more often https://stackoverflow.com/questions/1638877/difference-between-onmouseover-and-onmouseenter.

style={{ zIndex: 1200, pointerEvents: subMenuOpen ? 'visible' : 'none' }}
style={{
zIndex: 1200,
pointerEvents: subMenuOpen === 'products' ? undefined : 'none',
Copy link
Member Author

Choose a reason for hiding this comment

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

The default value of pointer-events is 'auto' not 'visible'. Also 'visible' only makes sense in the context of an SVG element, not for a div. Instead, we can unset the value. https://developer.mozilla.org/en-US/docs/Web/CSS/pointer-events

Copy link
Contributor

@danilo-leal danilo-leal left a comment

Choose a reason for hiding this comment

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

Working great!

@oliviertassinari oliviertassinari merged commit a1ce9c2 into mui:master Jun 23, 2022
@oliviertassinari oliviertassinari deleted the docs-improve-unreachable-docs branch June 23, 2022 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work website Pages that are not documentation-related, marketing-focused.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants