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

Primary Navigation: Increase toctree.maxdepth to 4 for Cloud and Guides #515

Closed
wants to merge 82 commits into from

Conversation

amotl
Copy link
Member

@amotl amotl commented Aug 1, 2024

Thoughts

A few sections may require more depth. Let's probe how this could make a difference for Cloud and Guides.

Preview

Not much can be inspected by looking at the preview rendering of the crate-docs-theme project, because its docmentation doesn't provide any reasonable depth at all.

https://crate-docs-theme--515.org.readthedocs.build/en/515/

References

Better previews and reviews are better conducted on behalf of relevant downstream projects.

amotl and others added 30 commits July 18, 2024 18:41
This theme derives from [sphinx-basic-ng], but needs a few components
from [Furo]. For example, the menu builder from Furo's `navigation.py`
is made available to the HTML context via `ng_navigation_tree`.

Therefore, it needs to vendorize a few styles. It can not take the whole
thing, because Furo's colors currently interfere with the colors
provided by the theme's styles.

[Furo]: https://github.com/pradyunsg/furo
[sphinx-basic-ng]: https://github.com/pradyunsg/sphinx-basic-ng
For demonstration purposes, it displays an automatically generated
navigation tree, as well as the custom semi-static menu provided by the
theme.

The navigation tree is generated using Furo's `compute_navigation_tree`.
@matkuliak
Copy link
Contributor

matkuliak commented Aug 1, 2024

Hello,

In general im for more depth. But in some examples it can be a bit confusing, at least to me. Like in cratedb-guide#112

e.g. https://cratedb-guide--112.org.readthedocs.build/performance/inserts/methods.html

The additional items in primary navigation aren't pages but headers/titles. So it's showing the same thing as the right-hand menu, but then lacking the depth for h2 headers. Not sure where that leaves us.

@amotl
Copy link
Member Author

amotl commented Aug 1, 2024

If the toctree would expand into pages instead of same-page titles, I guess those headline titles would be displayed there. So, it gives what it advertises: More depth in general. How you design and shape it on the leaf node level is probably still up to the author.

In general, both things are possible:

Increasing the maxdepth does not change anything to that, it just provides more legroom for both variants.

@matkuliak
Copy link
Contributor

Right, I know that it happening has nothing to do with this change.

I meant this will, I think, lead to variant you mention first being more visible, which is unexpected to me if the menu is built primary to display links to pages.

@amotl
Copy link
Member Author

amotl commented Aug 2, 2024

It's really just a proposal to use at your disposal. I can easily just NOT adjust this setting for Cloud Docs.

I meant this will, I think, lead to variant you mention first being more visible, which is unexpected to me if the menu is built primary to display links to pages.

I hear you, but in general, the primary navigation can be used for anything. It doesn't have to be "primarily built to navigate to pages". I mean, it all depends on the corresponding subtree at hand, and can be different from page to page.

NB: Well, not for anything yet. The linktree directive will improve the situation in regard to what can be plucked into primary navigation, beyond project-local toctree items.

@amotl amotl force-pushed the amo/adjust-primary-nav-maxdepth branch from 2e71f81 to b159169 Compare August 2, 2024 10:51
@amotl
Copy link
Member Author

amotl commented Aug 5, 2024

What do you think, @proddata? I will apply maxdepth 4 to CrateDB Guide. Do you also want it for CrateDB Cloud Docs, or better not?

@amotl amotl force-pushed the matthias/first-bunch-of-ng-fixes branch from 8f3437c to c013bba Compare August 7, 2024 15:14
Base automatically changed from matthias/first-bunch-of-ng-fixes to amo/basic-ng August 7, 2024 15:18
Base automatically changed from amo/basic-ng to main August 7, 2024 15:30
@amotl
Copy link
Member Author

amotl commented Aug 7, 2024

The relevant commit a750f88 has been included into GH-390, and will make it into the next release when there are no other objections about it.

@amotl amotl closed this Aug 7, 2024
@amotl amotl deleted the amo/adjust-primary-nav-maxdepth branch August 7, 2024 15:31
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.

3 participants