-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Resolve scrollbar visibility on navbar on long content #215
Conversation
I'm not really qualified to review this, but would love if someone else has thoughts. Changing the HTML format of all of our docs is a relatively large change, but if it's required for this functionality to work, so be it. |
The JS to dynamically modify the HTML if it's out of sync with the new CSS seems like a pretty... exciting approach. ;-) Presumably this is only an issue on RTD, right? For people building with the theme locally, the HTML and the CSS should always be from the same version of the theme, so tricks like this shouldn't be necessary. If that is the case, it seems like the JS to dynamically tweak the HTML should be in RTD itself, not in the theme code. RTD could also do a couple of things to reduce the need for things like this:
|
This would be an issue for RTD, but also possibly for forks of this theme that aren't pinned to 0.1.7. I lean towards keeping it here, as RTD is the main use case for this theme. It will definitely have to live on RTD either way however. Our opinion is we shouldn't force a rebuild on documentation for this. Versioned assets are on our roadmap, but would require some significant changes to be able to handle all the cases that we need to handle on RTD. If we change the HTML and don't provide graceful degradation here, this should be a backwards incompatible version. |
4539497
to
db85a3b
Compare
This resolves readthedocs#200, where a scrollbar was sometimes visible on the navbar. This unfortunately wasn't addressable with just CSS, as outlined in readthedocs#206. Because we need the element to be scrollable, we can't set `overflow: hidden` on the nav element. This fixes this issue by: * Adding a `wy-side-scroll` element over the fixed position nav element and under the menu item elements * `wy-side-scroll` is set to 320px width, while the fixed position nav elements and menu item elements are 300px, clipping the scrollbar with `overflow-x: hidden` on the fixed position element * Javascript is set to scroll the new scroll element instead of the parent fixed position element This was tested to be working in both cases on a variety of platforms: Linux FF, Chrome, Windows IE, OSX Chrome and Safari, iPhone 5.1, and Android 4.2.
db85a3b
to
60eb0af
Compare
Okay, i've just moved the javascript into RTD's extension of this javascript as a module in a separate branch. I'm going to target this for a backwards incompatible release, which will be v1.0 based on semvar. |
Resolve scrollbar visibility on navbar on long content
This fixes #200, where a scrollbar was sometimes visible on the navbar. This
unfortunately wasn't addressable with just CSS, as outlined in #206. Because we
need the element to be scrollable, we can't set
overflow: hidden
on the navelement.
This fixes this issue by:
wy-side-scroll
element over the fixed position nav element andunder the menu item elements
wy-side-scroll
is set to 320px width, while the fixed position nav elementsand menu item elements are 300px, clipping the scrollbar with
overflow-x: hidden
on the fixed position elementfixed position element
is added dynamically if it is missing, and children of the fixed position
element are moved there.
This was tested to be working in both cases on a variety of platforms: Linux FF,
Chrome, Windows IE, OSX Chrome and Safari, iPhone 5.1, and Android 4.2.