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

Left ToC open on mobile during page load #366

Closed
chrisjsewell opened this issue Aug 5, 2021 · 4 comments · Fixed by #454
Closed

Left ToC open on mobile during page load #366

chrisjsewell opened this issue Aug 5, 2021 · 4 comments · Fixed by #454
Labels
bug Something isn't working 🏷️ design Related to look and feel of the theme

Comments

@chrisjsewell
Copy link
Member

chrisjsewell commented Aug 5, 2021

Description

When the book theme is loaded on narrow screens, the left navigation bar is briefly visible, and then collapses.

Instead, it should automatically be invisible on narrow screens, and pressing the "show navbar" button should show it. Basically, similar to how Furo works:

)

Why does this happen

This happens because we are currently using Bootstrap Collapse functionality to toggle the left navigation bar. Clicking the button will use the bootstrap classes .collapse, .collapsing, and .show to trigger two things:

  • the visibility of the sidebar
  • the icon that's displayed in the hamburger menu

The problem with this is that we want a different default behavior on wide vs. narrow screens. On wide screens, the sidebar should be visible by default, with the option to hide. On narrow screens we want it hidden by default, with option to show (and ideally, hiding can be done by clicking anywhere outside the sidebar, not just on the toggle button).

How could we solve this?

I think the easiest way to solve this would be to do a few things:

  • Stop relying on Bootstrap for this
  • Use one of these approaches to showing the sidebar:
    • a simple class like .show-toggle and a simple onclick function like getElementById('site-navigation').classList.toggle("show-toggle")
  • The visibility of the sidebar can then be made different with @media queries in our CSS (so that checked means "show it" on mobile, but means "hide it" on wide screens)
@chrisjsewell chrisjsewell added the bug Something isn't working label Aug 5, 2021
@chrisjsewell
Copy link
Member Author

Yeh the JS is here:

var initTriggerNavBar = () => {

this could, and should, be achieved by CSS @media

@chrisjsewell chrisjsewell added the 🏷️ design Related to look and feel of the theme label Aug 5, 2021
@choldgraf
Copy link
Member

choldgraf commented Aug 6, 2021

Yeah it's always been like this, and agreed it is definitely sub optimal. I feel like part of the answer is finding a more graceful way to handle the opening and closing logic (IMO it would be great if we could avoid JS altogether but I wasn't sure how to do this at the time)

@choldgraf
Copy link
Member

I tried to see if this would be a simple fix, but I got bogged down in the bootstrap JS/CSS stuff and ran out of time, so instead I've updated the top comment with some more explanation of what I think is going on here so it's easier to pick back up (myself or others) in the future

@pradyunsg
Copy link
Member

FWIW, another option would be to have JS inline at the top of body / relevant tag, that adds the relevant class to it, to trigger the hiding (if the hiding worked that way)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working 🏷️ design Related to look and feel of the theme
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants