search

getgrav / grav-theme-gateway

Gateway incorporates elegant style with user friendly customizer options making it perfectly suited for a variety of Grav users. With rock solid development and flexible integration, the Gateway theme is sure to be a favorite for first time Grav users and experienced developers alike.
https://getgrav.org
GNU General Public License v2.0
12 stars 9 forks source link

toggling sticky top navbar makes page jump to top on small screens #11

Open phivk opened 7 years ago

phivk commented 7 years ago

On small screens (when the top nav bar is presented with a hamburger menu icon) and the user has scrolled enough for the navbar to be positioned fixed to top, clicking the top right "menu"+icon (li.toggle-topbar.menu-icon) element makes the the page 'jump to top'.

This looses the orientation on the page and can be confusing to the user. A more straightforward behaviour would be simply to expand the nav bar element while keeping the scroll position the same.

I've looked into user/themes/gateway/js/foundation.js and found the scrolltop option in the settings object of Foundation.libs.topbar. I've tried setting scrolltop to false, which results in expected behaviour when the top nav bar is in fixed positioning (scrolled), but causes the nav bar element to be positioned to the top of the page when clicked while it is not initially in fixed positioning (not scrolled).

To summarise the states that are buggy:

scrolltop \ position fixed (scrolled) relative (not scrolled)
true ❌ page jumps to top ✅
false ✅ ❌ nav bar element at page top
phivk commented 7 years ago

the issue of 'nav bar element at page top' (when scrolltop is false and user has not scrolled) seems to be caused by these lines in user/themes/gateway/js/foundation.js:

if (self.is_sticky(topbar, topbar.parent(), settings)) {
    topbar.parent().addClass('fixed');
}

Removing these lines fixes the 'nav bar element at page top' issue (bottom right in above table) for me.

phivk commented 7 years ago

as for the 'page jumps to top' issue, I guess this is might be a feature rather than a bug, considering it's presented like this in the settings:

scrolltop : true, // jump to top when sticky nav menu toggle is clicked

I still consider the 'nav bar element at page top' issue a bug. I'm happy to submit a PR with my fix for it if maintainers agree?

gravaholic commented 6 years ago

Hi, what is the solution?