Adds conditional rendering for top level link and refactors Mobile and Desktop components into discreet renderers #75
Conversation
rosschapman
changed the title
rosschapman
force-pushed
the
OUR415-87-header-nav-mobile-styling
branch
from
bb0d44d
to
fd52f23
Compare
rosschapman
changed the title
app/components/ui/Navigation.tsx
Outdated
| const activatePushPanelButtonIcon = () => { | ||
| if (mobileNavigationSidebarIsOpen && mobileSubNavigationSidebarIsOpen) { | ||
| return "fa-arrow-left"; | ||
| } | ||
| if (mobileNavigationSidebarIsOpen) { | ||
| return "fa-xmark"; | ||
| } | ||
|
|
||
| return ` fa-bars`; | ||
| }; |
There was a problem hiding this comment.
@adriencyberspace I'm ok with multiple returns, what about you?
There was a problem hiding this comment.
Works for me. You could also use a switch but not a big deal with only 3 cases.
There was a problem hiding this comment.
I'm currently blocked on being able to create a 2-tiered navigation but a couple of things I'm seeing right now:
- If you have two dropdown menus, they both open no matter which top level link you click. I think only the selected menu should open.
- Also a style issue is that the links don't extend the full width of the menu but should, which is noticeable on hover or focus
|
I think we should hide mobile on desktop size screens. i.e. if I open it on a smaller screen, it should disappear when I make my screen larger. 
|
|
@adriencyberspace my latest commit fixes the issues with multiple Navigation Menu types in the nav. Regarding #75 (comment) I propose we consider that in a polish phase. I'd like to better understand the scenario of a user resizing their browser that much. Please take another look thank you! Monosnap.screencast.2024-07-09.11-09-06.mp4 |
|
@rosschapman Off the bat, I'm seeing that the top level and single level links do not have uniform styling but do in the mockup. Actual: Mockup: Aside from that, can you make the mobile absolute positioned? I know it came in from SF Service Finder, but I can't recall seeing a mobile nav that pushes content over on any other site and I think it affects UX. |
|
I also still think these links should be full container width: 
|
|
@adriencyberspace These smaller style issues are fixed with 7d5efd2:
Regarding doing a slide-out vs push-out nav...hmmm. That will be a bit more involved. Let's park that for another iteration. I'll need to get some more design guidance on that to figure out how the overlay control looks/works. Although |
Description