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

improving usability of lower-left rtd menu #983

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

maltfield
Copy link

@maltfield maltfield commented Jul 23, 2020

This commit makes several changes to the rtd theme's version.html template such that the lower-left rtd menu now has:

  1. Support to display nav links to alternate languages by defining them as a list of tuples (just like versions and downloads)

  2. Adds a new variable 'display_lower_left' so that the lower-left menu can be toggled independently, while the "On Read the Docs" section is toggled with the READTHEDOCS boolean.

  3. Adds a conditional to the 'versions' section to display the current version in bold (same as readthedocs.io does)

  4. Add a conditional to the 'downloads' section to only display the downloads section if there's a non-empty downloads list

For more information about this change, see:

maltfield and others added 2 commits Jul 23, 2020
This commit makes several changes to the rtd theme's version.html template such that the lower-left rtd menu now has:

 1. Support to display nav links to alternate languages by defining them as a list of tuples (just like versions and downloads)

 2. Adds a new variable 'display_lower_left' so that the lower-left menu can be toggled independently, while the "On Read the Docs" section is toggled with the READTHEDOCS boolean.

 3. Adds a conditional to the 'versions' section to display the current version in bold (same as readthedocs.io does)

 4. Add a conditional to the 'downloads' section to only display the downloads section if there's a non-empty downloads list

For more information about this change, see:

 * https://tech.michaelaltfield.net/2020/07/23/sphinx-rtd-github-pages-2/
@Blendify Blendify requested review from agjohnson and Nov 25, 2020
@Blendify
Copy link
Member

@Blendify Blendify commented Nov 25, 2020

These changes make sense not sure if it conflicts in any way with Read the docs.

@@ -1,4 +1,4 @@
{% if READTHEDOCS %}
{% if READTHEDOCS or display_lower_left %}
{# Add rst-badge after rst-versions for small badge style. #}
<div class="rst-versions" data-toggle="rst-versions" role="note" aria-label="versions">
<span class="rst-current-version" data-toggle="rst-current-version">
Copy link
Member

@Blendify Blendify Nov 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should only be displayed on readthedocs

Copy link
Author

@maltfield maltfield Nov 25, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Blendify I'm not sure exactly what this comment means. The whole purpose of this PR is to add the ability to display the lower-left menu when not on readthedocs.

By default, it will not be displayed. If the user specifies html_context['display_lower_left'] = True in their sphinx config (or it is in-fact readthedocs), then it is displayed. This is not an uncommon desired functionality. See:

This has been tested by the way, and you can see my changes downstream in-action at the following websites:

This PR was created to push these changes back upstream so it would benefit the rest of the community.

Copy link
Member

@stsewd stsewd left a comment

This makes sense to me as well. This is mainly used at rtd like a "default" or "fallback" (and it only contains the versions at the moment that the build happened), the final footer (with all the current versions/translations) is injected with this content https://github.com/readthedocs/readthedocs.org/blob/master/readthedocs/api/v2/templates/restapi/footer.html (in case if that fails the user can still navigate the default one).

I think we should document this variables

@Blendify Blendify added this to the 1.0 milestone Mar 30, 2021
Blendify added a commit that referenced this issue Mar 30, 2021
The removes all the if checks for read the docs.

On read the docs builds this content is now injected. However, we want to support non read the docs builds and provide a way to generate the version menu.

This diff includes changes from #578 and #983
@agjohnson agjohnson modified the milestones: 1.0, 1.1 Mar 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants