-
Notifications
You must be signed in to change notification settings - Fork 4
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
Responsive styles for secondary navigation component #275
base: main
Are you sure you want to change the base?
Conversation
@paulrobertlloyd @benfraserdesign hmm interesting! I’m not sure I totally buy the need to switch to stacked secondary navigation items on mobile, rather than keeping it horizontal (with wrapping). Stacking them takes up way more vertical space, and if you’ve only got a few short items, or quite a big mobile phone, then they might well have fitted on a single line anyway? That said, currently it seems to switch to the Tablet layout from 321px and above - is that intentional? I had thought that 'mobile' was defined as being anything up to 640px width, with 641px to 768px being tablet and 769px+ being desktop? Also not convinced by centre-aligning the items in the tablet layout, as then the text of the first item doesn’t left-align with the rest of the page, and also the extra padding in the items looks like it ought to be clickable but isn’t (although this could be fixed by making the full width and height of the item clickable). Is it worth exploring keeping the tablet layout as is, but putting a bit more margin to the right of all the items (except the last one) to space them out a bit more? Happy to be persuaded on any of the above points though, especially if there’s research or evidence from services using this pattern (which I’m not any more)! |
Having items appear stacked on mobile:
As to the tablet layout, agree it is perhaps odd to have three different layouts, nice as the tablet variant is. Perhaps increasing the space between items above the @benfraserdesign’s original design did have the entire item be clickable, as does the current version in fact. I removed this again to be consistent with the service navigation component… which isn’t necessarily right, of course. Maybe something to ask the team behind the service navigation component, perhaps they had a good reason for not doing this? |
Have updated the PR message to correctly label the examples (hopefully). |
Taking a look at the latest changes, my input would be either: 1 or 2 |
Fixes #270, and builds on #273.
@benfraserdesign correctly reported that the responsive styles for the secondary navigation component could be improved. His PR in #273 goes someway to addressing the responsiveness, but by using table display, the navigation breaks if there are too many items and/or their labels are long.
This PR builds on this work, using the design of equally spaced navigation items with centred text labels between mobile and tablet breakpoints. Below the mobile breakpoint, and above the tablet breakpoint, the navigation more closely follows the design of the service navigation component, which wasn’t a thing when this component (or the components that inspired it) was first designed. SCSS from the service navigation component is liberally applied here.
< Mobile
Mobile > Tablet
> Tablet