Fix solutions menu not totally visible #722
Conversation
…menu styles and transition also on mobile
| const MenuBarsImage = () => ( | ||
| <svg | ||
| width="20" | ||
| height="12" | ||
| viewBox="0 0 20 12" | ||
| fill="none" | ||
| xmlns="http://www.w3.org/2000/svg" | ||
| > | ||
| <path | ||
| d="M1 2H19C19.2652 2 19.5196 1.89464 19.7071 1.70711C19.8946 1.51957 20 1.26522 20 1C20 0.734784 19.8946 0.48043 19.7071 0.292893C19.5196 0.105357 19.2652 0 19 0H1C0.734784 0 0.48043 0.105357 0.292893 0.292893C0.105357 0.48043 0 0.734784 0 1C0 1.26522 0.105357 1.51957 0.292893 1.70711C0.48043 1.89464 0.734784 2 1 2ZM19 10H1C0.734784 10 0.48043 10.1054 0.292893 10.2929C0.105357 10.4804 0 10.7348 0 11C0 11.2652 0.105357 11.5196 0.292893 11.7071C0.48043 11.8946 0.734784 12 1 12H19C19.2652 12 19.5196 11.8946 19.7071 11.7071C19.8946 11.5196 20 11.2652 20 11C20 10.7348 19.8946 10.4804 19.7071 10.2929C19.5196 10.1054 19.2652 10 19 10ZM19 5H1C0.734784 5 0.48043 5.10536 0.292893 5.29289C0.105357 5.48043 0 5.73478 0 6C0 6.26522 0.105357 6.51957 0.292893 6.70711C0.48043 6.89464 0.734784 7 1 7H19C19.2652 7 19.5196 6.89464 19.7071 6.70711C19.8946 6.51957 20 6.26522 20 6C20 5.73478 19.8946 5.48043 19.7071 5.29289C19.5196 5.10536 19.2652 5 19 5Z" | ||
| fill="white" | ||
| /> | ||
| </svg> | ||
| ); |
There was a problem hiding this comment.
We can use the icon from MUI instead of adding extra code for the same icon.
| useEffect(() => { | ||
| if (mobileMenuOpen) { | ||
| document.body.style.overflow = 'hidden'; | ||
| } else { | ||
| document.body.style.overflow = ''; | ||
| } | ||
|
|
||
| return () => { | ||
| document.body.style.overflow = ''; | ||
| }; | ||
| }, [mobileMenuOpen]); |
There was a problem hiding this comment.
Prevent body scroll when the menu is open. At this moment, only the menu scroll really matters.
There was a problem hiding this comment.
This is alright. I think we should look at doing the menu without JS though. We should eventually move the menu next to the button and just have that be a hidden checkbox. Then we can do all this styling based on if the checkbox is checked.
| {isMobile ? ( | ||
| <Slide | ||
| direction="left" | ||
| in={mobileMenuOpen || !!activeMenu} | ||
| timeout={{ | ||
| enter: slideTimeout, | ||
| exit: slideTimeout, | ||
| }} | ||
| > | ||
| {menuContent} | ||
| </Slide> | ||
| ) : ( | ||
| menuContent | ||
| )} |
There was a problem hiding this comment.
I added a slide transition to the mobile menu. I think it's more fluid and has a better UX than just showing up instantly when clicking.
There was a problem hiding this comment.
Feels a bit better on the phone for sure!
|
Visit the preview URL for this PR (updated for commit 84de5f5): https://estuary-marketing--pr722-brenosalv-bug-721-th-vzuprc6i.web.app (expires Tue, 18 Mar 2025 04:59:44 GMT) 🔥 via Firebase Hosting GitHub Action 🌎 Sign: 76f6b095a0752e5d9c6c890267f9fdc3e392161e |
| useEffect(() => { | ||
| if (mobileMenuOpen) { | ||
| document.body.style.overflow = 'hidden'; | ||
| } else { | ||
| document.body.style.overflow = ''; | ||
| } | ||
|
|
||
| return () => { | ||
| document.body.style.overflow = ''; | ||
| }; | ||
| }, [mobileMenuOpen]); |
There was a problem hiding this comment.
This is alright. I think we should look at doing the menu without JS though. We should eventually move the menu next to the button and just have that be a hidden checkbox. Then we can do all this styling based on if the checkbox is checked.
| /> | ||
| </svg> | ||
| ); | ||
| const slideTimeout = 300; |
There was a problem hiding this comment.
I would reduce this. The menu slide in feels a little bit slow. For animations I think they need to be really fast on stuff that is super important like letting the user navigate.
There was a problem hiding this comment.
Makes sense, half of this looks better.
#721
Changes
Tests / Screenshots
Solutions menu totally visible:
All the links from Resources menu are now visible as well:
