patronage / bubs-timber

Gulp + Timber + WP
Other
5 stars 1 forks source link

44 plain default mobile nav #65

Closed yuvilio closed 7 years ago

yuvilio commented 7 years ago

Per discussion in #44, this PR switches the current header navbar nav with a basic starter out of the box mobile nav . It's aims are two fold:

  1. Remove dependency on bootstrap navbar and it's setup. Utilizing navbar added more constrictions than benefits (shown below).

  2. Give a starter option for a basic responsive nav, solving typical needs (mobile nav expand)

Before:

After (with this PR):

desktop-nav-demo

mobile-nav-demo

As always, this is just an out of the box default option. Feel free to overwrite, if the design you're working with implies a different approach.

ccorda commented 7 years ago

Left a few notes.

One implementation note is that IMO based on common denominator in sites we build, hamburger nav should only be on mobile (xs). For sm-and-up, menu should be visible. For small, I'm ok if nav drops to its own line like this:

<div class="col-md-4 logo"></div>
<div class="col-md-8 nav"></div>

I'd push back on the JS to rearrange markup for mobile nav. Possible I'm misunderstanding the utility, but I don't think we need it. You can get collapsible nav without resorting to JS.

yuvilio commented 7 years ago

I think the js re-arrange is not a big deal, but it's agreed that it's not a must. Can go redudant nav markup (visible-xs, hidden-xs) route instead.

Note that the hamburger menu is indeed only visible in mobile. In sm and up the menu is fully visibly (per screenshots above).

ccorda commented 7 years ago

Looks good Yuval, :shipit: