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

support hide metadata and update docs #40

Merged
merged 5 commits into from
Mar 3, 2022
Merged

Conversation

2bndy5
Copy link
Collaborator

@2bndy5 2bndy5 commented Mar 2, 2022

This resolves #39 and closes #37

I updated the docs to guide users on how to hide a certain page's ToC and navigation menu. While I was updating customization.rst, I also added a hint about sphinx auto-implementing navigation.indexes feature from mkdocs-material.

docs/customization.rst Outdated Show resolved Hide resolved

.. code-block:: rst

:tocdepth: 0
Copy link
Owner

Choose a reason for hiding this comment

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

I also noticed that tocdepth is not actually respected by nav_adapt.py, other than handling the special case of tocdepth == 0.

Possibly we could support :hide: toc as well for consistency with mkdocs and since it is clearer than having to specify tocdepth.

Copy link
Owner

Choose a reason for hiding this comment

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

I guess one challenge with :hide: as a field element is that it seems that sphinx creates a dictionary --- is there a way to support multiple :hide: metadata values?

Copy link
Collaborator Author

@2bndy5 2bndy5 Mar 2, 2022

Choose a reason for hiding this comment

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

is there a way to support multiple :hide: metadata values?

This is why I chose to keep the implemented tocdepth metadata. I suppose we could do

hide_list = meta.get("hide").split(",")
hide_list = [hide.strip() for hide in hide_list]
if "navigation" in hide_list:
    # ...
if "toc" in hide_list:
    # ...

Copy link
Owner

Choose a reason for hiding this comment

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

Actually I think we should go with something like :hide-navigation: rather than :hide: navigation --- that way we are more consistent with how these document metadata fields are normally treated (as key-value assignments).

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Would you like to preserve the tocdepth option for backward compatibility? Or do you consider it broken enough to get rid of it?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

metadata fields are normally treated (as key-value assignments).

So what value do you propose? A bool? Or does sphinx use a default value in case none is specified?

Copy link
Owner

Choose a reason for hiding this comment

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

I think they are just parsed as strings, but for bool flags the convention seems to be to just leave the value blank:

https://www.sphinx-doc.org/en/master/usage/restructuredtext/field-lists.html#special-metadata-fields

So just:

:hide-navigation:

As far as tocdepth --- thinking about it more, I think we should just keep using tocdepth for this purpose, rather than introducing a new :hide-toc: field. At some point we should actually make non-zero values of tocdepth have their proper effect, but for now it seems reasonable to leave it as is.

I think currently, on the master document only the toc depth is limited to 2, and on all other pages the toc depth is unlimited. This difference is due to the fact that nav_adapt.py uses different logic in those two cases for obtaining the local toc.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can implement both hide-toc while preserving tocdepth. In testing the new metadata idea, the users would have to specify

:hide-naviagation: anyValue

otherwise, no value has no effect.

.. this has no effect
:hide-naviagation:

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

NM, I got it to work if the metadata is specified at all. If it isn't there, then the feature is not enabled for that page.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm going forward with the hide-toc idea because it better matches mkdocs-material convention (& its more descriptive). People migrating from mkdocs to sphinx may get confused about the toctree directive's option maxdepth. To be clear, :tocdepth: 0 will still work (just like hide-toc).

docs/customization.rst Outdated Show resolved Hide resolved
docs/customization.rst Outdated Show resolved Hide resolved
@2bndy5
Copy link
Collaborator Author

2bndy5 commented Mar 3, 2022

woah! I just tried switching the direction to rtl, and there's a bunch of problems... It would seem that the side nav menus don't swap sides despite the direction specified, but I think there's more than a few bugs happening here.

I'll remove the "( or XXX side in rtd languages direction)" from the last commit's change to docs. We can work that out when the direction is fixed.

@jbms jbms merged commit 06a3959 into jbms:main Mar 3, 2022
@jbms
Copy link
Owner

jbms commented Mar 3, 2022

Thanks!

@2bndy5 2bndy5 deleted the support-hiding-nav branch March 3, 2022 05:00
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.

Hide toc or navigation seems not work [Reminder] about navigation.indexes not needed
2 participants