search

DoSomething / forge

🎨 The DoSomething.org pattern library.
http://forge.dosomething.org
MIT License
48 stars 14 forks source link

Updating the main navigation region styles. #577

Closed weerd closed 4 years ago

weerd commented 4 years ago

This PR updates the navigation to maintain the use of the mobile (hamburger) navigation on medium size devices and only switch to the full navigation from large+.

Related to work needed for Pivotal Ticket #167527420 which separates the "Log in" link in the navigation into two distinct links; one for registering and one for logging in.

Responsive nav update

Additional Information

I found the easiest way to address the fixes for this was to tackle updating the Forge _navigation.scss rules. I removed the use of the $mobile variable, which was used to only apply styles when on mobile device (specified by max-width: 759px), in favor of going with a more mobile-first approach and editing the styles as the viewport increases.

The $mobile breakpoint worked great, but now that the mobile nav needed to continue on medium size devices, it would have conflicted with the $medium breakpoint and how styles are triggered.

In general I found that it's much easier to consider starting with small size and making changes as the device size increases, rather than having a breakpoint like $mobile (aka, $small), that only applied rules for that size and below. While it can seem convenient at the time, for future edits and changes in designs it really complicated things. If we had taken the small to medium to large approach (using just min-widths), it would have been a simple edit to change the breakpoint variables and pretty much call it a day.

weerd commented 4 years ago

There are some additional edits that need to happen for the Phoenix specific navigation, but I'll tackle those in the Phoenix repo.

weerd commented 4 years ago

@lkpttn this is the solution via Forge update.

weerd commented 4 years ago

This is also keeping in mind that there's a major refactoring coming up for the Phoenix Nav to implement "Mega Nav", but until that's finalized, this seemed like the correct approach to address the most pressing nav updates!

weerd commented 4 years ago

Is the Phoenix nav the only place we use this responsive hamburger or are we just comfortable with releasing this breakpoint update broadly?

Really only used on Phoenix, but also comfortable releasing it broadly.