Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upMake the theme responsive #46
Conversation
Overall a great idea and I would also really like a responsive theme though there seem to be some unrelated changes and implementation details which should be discussed |
{% endif %} | ||
{{ super() }} | ||
{% endblock %} | ||
|
||
{%- block css %} | ||
<meta name="viewport" content="width=device-width, initial-scale=1.0"> |
septatrix
Jun 14, 2020
•
This got added in sphinx 3.1 and should work mostly without problem for people using old sphinx versions. May however not hurt to include it for now and delete it later
* { | ||
box-sizing: border-box; | ||
} |
.nav-content .search label { | ||
border: 0; | ||
clip: rect(0 0 0 0); | ||
height: 1px; | ||
margin: -1px; | ||
overflow: hidden; | ||
padding: 0; | ||
position: absolute !important; | ||
width: 1px; | ||
} |
septatrix
Jun 14, 2020
What exactly is the purpose of the label if you try to hide it (at least that what it seems so far from this code)
obulat
Jun 14, 2020
Author
This label is there for accessibility reasons: screen readers can read it, but it doesn't crowd the layout. The search box is implemented this way on mdn website.
septatrix
Jun 14, 2020
Ah I see, so this is a manual version of sr-only
like in bootstrap. I would probably just use a aria-label="{{ _('Quick search') }}"
on the input
element though
document.addEventListener('DOMContentLoaded', function () { | ||
const toggler = document.querySelector('.toggler'); | ||
const sideMenu = document.querySelector('.menu-wrapper'); | ||
const doc = document.querySelector('.document'); | ||
function closeMenu() { | ||
sideMenu.classList.remove('open'); | ||
toggler.checked = false; | ||
} | ||
toggler.addEventListener('change', function (e) { | ||
if (toggler.checked) { | ||
sideMenu.classList.add('open'); | ||
} else { | ||
closeMenu(); | ||
} | ||
}); | ||
doc.addEventListener('click', function () { | ||
if (toggler.checked) { | ||
closeMenu(); | ||
} | ||
}) | ||
}) |
septatrix
Jun 14, 2020
By moving the checkbox up in the dom tree it may be possible to implement this using only javascript and make it more accessible for people without javascript
obulat
Jun 19, 2020
Author
I replaced the checkbox with a button. Is this better for people without javascript?
septatrix
Jun 20, 2020
What I would have done is move the input as a checkbox directly next to the sidebar, reference it in the label via id and apply the styles based on whether the checkbox is checked or not. I may have time tomorrow to create a PR on your fork repo if you want to.
obulat
Jun 21, 2020
Author
I have done it like this at first (checkbox input and applying styles based on :checked property). Then I was checking for accessibility and found this article which shows that a button as menu opener has some advantages regarding accessibility: namely, it is easier to set up keyboard handling. On the other hand, I just realized that people probably don't use keyboard for navigation on mobile, and it is more important to set up No-script solution than keyboard navigation on mobile.
Also, I looked at other documentation sites: readthedocs theme and VueJS docs, and their hamburger menu doesn't work without javascript.
septatrix
Jun 23, 2020
In addition if you follow this guide you also get a working css only solution with added accesibility using javascript
Thank you for your review, @septatrix ! I would also like to have your feedback on the content for the mobile version. For example, the top navbar on mobile: Do you think logo, version switcher and searchbox are what should be there, or do we need to add/ remove something? Also, the relbar is completely removed, but I thought that it might be a good idea to add it at the bottom, without the searchbox, though. |
- Remove unnecessary `show` call for searchbox. - Remove `box-sizing:border-box` for all elements. - Add `aria-label` to search input instead of visually-hiding labels for accessibility - Some style fixes
Beware, i'm working on moving the switchers generation code to docsbuild scripts instead of cpython. See python/docsbuild-scripts#90 I did not think about switchers being located in different places, we have to check if there is something to fix either in docsbuild side or here. As the switchers are no longer to be maintained in cpython, I was removing the placeholders and the switchers variable in python/cpython#20969 I'm on my phone right know so I can't test it, but we need to agree on how to "communicate" to docsbuild the switchers places. |
Hi @obulat! Took the time to think about it, but beware, I'm particularily bad at HTML/JS:
The current
What about adding all needed placeholders in python-docs-theme, so How to test with docsbuild-script locally: $ # In a clone of docsbuild-scripts
$ mkdir -p www logs build_root
$ python3 -m venv build_root/venv/
$ build_root/venv/bin/python -m pip install -r requirements.txt
$ build_root/venv/bin/python -m pip install path_to_your_python-docs-theme
$ python3 ./build_docs.py --quick --build-root build_root --www-root www --log-directory logs --group $(id -g) --skip-cache-invalidation --language en --branch master (Currently testing it and wow, that's really nice, thanks a lot for working on this topic!) |
Hi, @julien, thank you for your comments, that's really helpful! I will try to look into it as soon as I have time. |
I think a problem which should be tackled first is overflowing stuff like |
- Menu is opened/closed using only css (invisible checkbox), works without js enabled - Accessibility added using javascript
I've added wrapping |
} | ||
/* Typography */ | ||
div.body h1, div.body h2, div.body h3, div.body h4, div.body h5, div.body p { | ||
font-family: SourceSansProRegular, Arial, sans-serif; |
z-index: 1; | ||
} | ||
.mobile-nav * { | ||
box-sizing: border-box; |
septatrix
Sep 20, 2020
If possibly I would stay with the sphinx default of not modifying the box-sizing property
I have been reading on my phone a lot lately, and was disappointed by the fact that I couldn't read python docs: the fonts are too small, and, if I try to zoom in, the text doesn't fit on the page, so I have to swipe to see the whole line.
This is my attempt to add responsiveness to python docs theme, fixing #30.
On screen widths smaller than 900 px, it:
adds a mobile navigation bar with a 'hamburger' menu button, Python version switcher and a search box.
adds a sliding menu that opens when 'hamburger' menu button is clicked. Menu contains language switching input and contents.
removes the
related
barsremoves the sidebar.
Screenshot with menu closed:


Screenshot with menu open:
Problems that need to be solved:
Normally, the mobile navigation bar should be placed in the
header
block of the layout, but python.org layout completely overrides it, so I added it in the body_tag block. To solve this, I would need to add changes to that code.I placed the switchers and other items on the menus as seemed logical to me, but feedback from the community is essential on this.
I added a javascript file to open/close sidebar in a separate file. I could also add it into the layout file, but ideally all js files should be combined and minified.