Skip to content

Commit

Permalink
Resolve scrollbar visibility on navbar on long content
Browse files Browse the repository at this point in the history
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
 * For backwards compatibility on already generated HTML, the new scroll 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.
  • Loading branch information
agjohnson committed Jul 17, 2015
1 parent 5e40211 commit 4539497
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 33 deletions.
23 changes: 19 additions & 4 deletions sass/_theme_layout.sass
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
padding: 0 $base-font-size

.wy-menu-vertical
width: $nav-desktop-width
header, p.caption
height: $base-font-size * 2
display: inline-block
Expand Down Expand Up @@ -164,13 +165,14 @@
color: $white

.wy-side-nav-search
display: block
width: $nav-desktop-width
padding: $gutter / 2
margin-bottom: $gutter / 2
z-index: $z-index-popover
background-color: $link-color
text-align: center
padding: $gutter / 2
display: block
color: $section-background-color
margin-bottom: $gutter / 2
input[type=text]
width: 100%
border-radius: 50px
Expand Down Expand Up @@ -253,11 +255,18 @@
padding-bottom: 2em
width: $nav-desktop-width
overflow-x: hidden
overflow-y: scroll
overflow-y: hidden
min-height: 100%
background: $menu-background-color
z-index: $z-index-popover

.wy-side-scroll
width: $nav-desktop-width + 20px
position: relative
overflow-x: hidden
overflow-y: scroll
height: 100%

.wy-nav-top
display: none
background: $link-color
Expand Down Expand Up @@ -342,6 +351,12 @@ footer
&.shift
width: 85%
left: 0
.wy-side-scroll
width: auto
.wy-side-nav-search
width: auto
.wy-menu.wy-menu-vertical
width: auto
.wy-nav-content-wrap
margin-left: 0
.wy-nav-content
Expand Down
53 changes: 27 additions & 26 deletions sphinx_rtd_theme/layout.html
Original file line number Diff line number Diff line change
Expand Up @@ -85,38 +85,39 @@

{# SIDE NAV, TOGGLES ON MOBILE #}
<nav data-toggle="wy-nav-shift" class="wy-nav-side">
<div class="wy-side-nav-search">
{% block sidebartitle %}
<div class="wy-side-scroll">
<div class="wy-side-nav-search">
{% block sidebartitle %}

{% if logo and theme_logo_only %}
<a href="{{ pathto(master_doc) }}">
{% else %}
<a href="{{ pathto(master_doc) }}" class="icon icon-home"> {{ project }}
{% endif %}
{% if logo and theme_logo_only %}
<a href="{{ pathto(master_doc) }}">
{% else %}
<a href="{{ pathto(master_doc) }}" class="icon icon-home"> {{ project }}
{% endif %}

{% if logo %}
{# Not strictly valid HTML, but it's the only way to display/scale it properly, without weird scripting or heaps of work #}
<img src="{{ pathto('_static/' + logo, 1) }}" class="logo" />
{% endif %}
</a>
{% if logo %}
{# Not strictly valid HTML, but it's the only way to display/scale it properly, without weird scripting or heaps of work #}
<img src="{{ pathto('_static/' + logo, 1) }}" class="logo" />
{% endif %}
</a>

{% include "searchbox.html" %}
{% include "searchbox.html" %}

{% endblock %}
</div>
{% endblock %}
</div>

<div class="wy-menu wy-menu-vertical" data-spy="affix" role="navigation" aria-label="main navigation">
{% block menu %}
{% set toctree = toctree(maxdepth=4, collapse=theme_collapse_navigation, includehidden=True) %}
{% if toctree %}
{{ toctree }}
{% else %}
<!-- Local TOC -->
<div class="local-toc">{{ toc }}</div>
{% endif %}
{% endblock %}
<div class="wy-menu wy-menu-vertical" data-spy="affix" role="navigation" aria-label="main navigation">
{% block menu %}
{% set toctree = toctree(maxdepth=4, collapse=theme_collapse_navigation, includehidden=True) %}
{% if toctree %}
{{ toctree }}
{% else %}
<!-- Local TOC -->
<div class="local-toc">{{ toc }}</div>
{% endif %}
{% endblock %}
</div>
</div>
&nbsp;
</nav>

<section data-toggle="wy-nav-shift" class="wy-nav-content-wrap">
Expand Down
2 changes: 1 addition & 1 deletion sphinx_rtd_theme/static/css/theme.css

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion sphinx_rtd_theme/static/css/theme.css.map

Large diffs are not rendered by default.

18 changes: 17 additions & 1 deletion sphinx_rtd_theme/static/js/theme.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,23 @@ window.SphinxRtdTheme = (function (jquery) {
}, 25);
},
init = function () {
navBar = jquery('nav.wy-nav-side:first');
// Because generated HTML will not immediately have the
// new scroll element, gracefully handle failover by adding it
// dynamically.
navBar = jquery('div.wy-side-scroll:first');
if (! navBar.length) {
var navInner = jquery('nav.wy-nav-side:first'),
navScroll = $('<div />')
.addClass('wy-side-scroll');

navInner
.children()
.detach()
.appendTo(navScroll);
navScroll.appendTo(navInner);

navBar = navScroll;
}
win = jquery(window);
},
reset = function () {
Expand Down

0 comments on commit 4539497

Please sign in to comment.