-
Notifications
You must be signed in to change notification settings - Fork 328
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
Fix (responsive) navbar dropdown menu #313
Fix (responsive) navbar dropdown menu #313
Conversation
@@ -3,7 +3,6 @@ | |||
height: var(--header-height); | |||
width: 100%; | |||
padding: 0; | |||
overflow: hidden; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
where did this addition come from? we should make sure it wasn't fixing a different bug or something
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was added in #286 (comment), but I don't directly know why it would have been necessary (I tested a few of the targets that #286 was fixing, and they still seem to be working)
argh it seems like the RTD hook to auto-build the documentation isn't working? |
It was still enabled in the settings, and there was actually also a build for this PR, only didn't show up here. Seems that triggering it again by reopening the PR fixed that ;) |
Hmm, so it seems that's because we now set a fixed height for the navbar, but of course in mobile mode when toggling the navbar needs to be able to become bigger. |
TL;DR: would be great to get this merged, it's a blocker for MNE-Python using this theme. Read on for idea about an alternate implementation that works on our site but may not generalize well. Wondering what the status of this PR is... personally I am content with the approach here, although I've also been playing around with adding a background color and classes The downside to my approach is that the |
after playing around with my proposed solution some more, I'm not convinced that it's workable and hence I would advocate for merging this PR as-is. It seems to fix the problem and seems perfectly fine from a usability perspective. It would maybe be prettier if someone could figure out how to auto-shrink the width of the hamburger dropdown and anchor it to the right side under the hamburger button, but I at least couldn't find a way to make that work cleanly. |
Closes #304