FIX empty primary sidebar showing up

[Charlie-XIAO]

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 #1642, not sure how much.

[Charlie-XIAO]

@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]

@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]

I will give this a review today

[gabalafou]

These tests are great!

I made one request to fix some comments, but otherwise this PR looks good to me.

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

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.

In any case, we really need to document the sidebar_includehidden theme configuration variable.

[gabalafou]

I don't think this test case is at all dependent on the change made in this PR, but it seems like a nice test case to have in the test suite. 😄

[Charlie-XIAO]

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

[drammock]

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.