FIX empty primary sidebar showing up

Charlie-XIAO commented 6 months ago

Fixes #1662. For more information see the referenced issue.

I also added a minimal reproducible site for testing, not sure if it is proper.

A side effect is that we may lose some speed gain in https://github.com/pydata/pydata-sphinx-theme/pull/1642, not sure how much.

Charlie-XIAO commented 5 months ago

@drammock is it possible that you or some other maintainer review this one, if you have time? An empty primary sidebar really affects the look of the website a lot so I'm hoping that this can be included into the next release. Thank you very much 🙏

trallard commented 5 months ago

@gabalafou since you have been doing a lot of work on the TOC navigation and overall accessibility of the theme, could you have a look at this PR (mostly, I want to make sure this does not interfere/introduce potential gotchas against all the work on #1564)

gabalafou commented 5 months ago

I will give this a review today

Charlie-XIAO commented 5 months ago

@gabalafou Thanks for your review! I've addressed your comment.

drammock commented 5 months ago

Based on the description of PR #1642, I think that this PR will almost completely undo the performance gains of that PR.

agreed

It would be good if @drammock could weigh in and explain a line he wrote in his PR description:

The logic is that finding an ancestor and including hidden toctrees together guarantee that a non-empty toc subtree exists for the current page, and we can postpone the toctree.resolve() step until later when it's actually added to the page.

I'm wondering if the logic held for SciPy but just doesn't hold in general, or if there was some misunderstanding about what includehidden meant.

It's entirely possible that I am just wrong about it. My reasoning was:

either we're on an :orphan: page, or we're not
if we're on an :orphan: page there will be no ancestor page, ancestorname will be None, and we can suppress the sidebar
if this page is the site root, or the search results page, or some other "special" page, it will also not have an ancestor and we can suppress the sidebar
if we're not on an orphan or special page, that means this page must show up in a toctree directive on some other "higher-level" page. if that is true, then what matters is:
- what is startdepth, and is current page at least 1 level below startdepth (this is I think handled in the get_ancestor_pagename function, but I didn't write that code so I'm not 100% sure/confident that it works the way I think it does)
- is the global includehidden flag True, in which case we can be sure that this page should show up in the sidebar toctree (regardless of whether it's added in a toctree that does or does not have the :hidden: flag).

One of those last two sub-bullets is probably wrong. Maybe the theme-level includehidden setting is not getting passed through correctly? Sadly I'm about to be AFK for ~1.5 weeks so I can't dig any deeper at the moment.

My earlier look at @djhoese's site showed an empty toc-item DIV getting added; if someone were to dig deeper while I'm away, that's where I'd start I guess.

pydata / pydata-sphinx-theme

FIX empty primary sidebar showing up #1682