olefredrik / FoundationPress

FoundationPress is a WordPress starter theme based on Foundation 6 by Zurb
https://foundationpress.olefredrik.com
MIT License
2.7k stars 867 forks source link

Cleanup Navigation #1365

Open derweili opened 5 years ago

derweili commented 5 years ago

FoundationPress includes a bunch of navigation styling in the navigation.scss

This is special styling without any documentation. (Which causes problems for users using FoundationPress for the first time #1364)

FoundationPress is a Starter Theme is for WordPress implementing the foundation-sites framework Because of this I don't see any reason why FoundationPress should implement own styles different to the foundation-sites standards unless WordPress requires those styles.

I recommend following changes:

zkriszti commented 5 years ago

Ole, thank you for this clean WP-starting theme. However, talking about navigation, I think it is not really good practice making the menu responsive by creating two menu locations, rendering both in the DOM, and then just set "display: none" in CSS for whichever is not necessary in the current view. There should be only one menu location in the header, as we are talking about the same functional item ("header menu") regardless of size of viewport. Then apply CSS styling with media queries to this one. This is simple to solve by using flexbox (order), but if FoundationPress needs to support legacy browsers, it is still achievable by using floats.

zkriszti commented 5 years ago

Also, putting the hamburger icon into title-bar and the menu into top-bar makes changing responsive menu toggling (ie. make a different breakpoint only for the menu) a cumbersome task.

nitrokevin commented 5 years ago

As foundationpress is based on foundation, it needs to follow how foundation works, you need to suggest those changes to the foundation-sites team :)