Skip to content
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

When set to show sticky nav at top of page, sticky nav doesn't #1660

Closed
benlk opened this issue Apr 11, 2019 · 1 comment
Closed

When set to show sticky nav at top of page, sticky nav doesn't #1660

benlk opened this issue Apr 11, 2019 · 1 comment
Assignees
Labels
Milestone

Comments

@benlk
Copy link
Collaborator

benlk commented Apr 11, 2019

The relevant setting is Theme Options > Navigation >
"Hide the main navigation on article pages and display only the sticky navigation on article pages".

The reason the sticky nav is not showing is because we aren't showing the sticky nav thoroughly enough.

https://github.com/INN/largo/blob/72cc8433a1be1de4f22789e1ec89496d54a653d9/less/inc/navbar-sticky.less#L49-L51

It's set to display block, but the default state of .sticky-nav-holder is

https://github.com/INN/largo/blob/72cc8433a1be1de4f22789e1ec89496d54a653d9/less/inc/navbar-sticky.less#L8-L16

By default, it only shows up with the .show class,

https://github.com/INN/largo/blob/72cc8433a1be1de4f22789e1ec89496d54a653d9/less/inc/navbar-sticky.less#L30-L33.

However, making the change to allow it to show at the top by default:

  @media (min-width: 769px) {
    display: none;
    body &.main_nav_hide_article,
    body &.sticky_nav_display {
      display: block;
      visibility: visible;
      opacity: 1;
    }   
  }

means that it won't disappear on cue when scrolling farther down the page.

What this really needs is the addition of show to the partial if it's in a sticky_nav_display situation, either through PHP or JS.

@benlk benlk added category: styles affects lots of styles, requiring visual testing type: bug labels Apr 11, 2019
@benlk benlk mentioned this issue Apr 11, 2019
3 tasks
@benlk benlk modified the milestone: 0.6.4 Apr 26, 2019
@benlk benlk added this to the 0.6.4 milestone May 22, 2019
@benlk benlk self-assigned this Jul 12, 2019
@benlk
Copy link
Collaborator Author

benlk commented Jul 12, 2019

A more-thoroughly-thought-out solution:

  • the presence or absence of the sticky nav holder should only be toggled by the presence or absence of the show class
  • CSS relating to the presence or absence of the sticky nav holder should be limited to:
    • default state hide
    • &.show: show
    • all other showing/hiding CSS removed
      • main_nav_hide_article
      • sticky_nav_display
  • Appropriate PHP to ensure that the template starts with the show class in the HTML upon initial page load when it's set to display at top.

Work on this is ongoing in #1662.

@benlk benlk closed this as completed Jul 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant