pydata-sphinx-theme icon indicating copy to clipboard operation
pydata-sphinx-theme copied to clipboard

Empty TOC in right sidebar

Open tupui opened this issue 1 year ago • 8 comments

When the TOC is empty, the right sidebar is still present through bd-sidebar-secondary bd-toc

181444254-c3d2ca92-5238-46cc-9a78-068a798b0317

This is observed in SciPy. To build SciPy and the doc (build should take around 3 mins with 6 cores, then 5 mins for the doc itself):

# once you cloned my branch: https://github.com/scipy/scipy/pull/16660
# create an conda env to install everything
# don't try on windows as it will mostly fail.
mamba create env -f environment.yml
python dev.py doc -j 6  # number of cores

tupui avatar Aug 02 '22 09:08 tupui

Can somebody please provide a minimal reproducible example or link? I worry that this won't get fixed if debugging requires building the entire scipy docs...

choldgraf avatar Aug 11 '22 05:08 choldgraf

I've put in a PR to MNE-Python that should generate a working CircleCI site that demos this error. Will update here once the build is done.

drammock avatar Aug 12 '22 22:08 drammock

Here is the rendered MNE-Python doc with the empty right column:

drammock avatar Aug 12 '22 22:08 drammock

Diagnosis of what's going wrong

OK looking at the docs that @drammock shared, I think that this is the problem:

  • We have two extra toc items enabled by default: edit-this-page, and sourcelink (ref). These are all added to theme_page_sidebar_items.
  • In addition, we provide configuration flags to trigger this behavior on and off (ref).
  • If people trigger edit-this-page or sourcelink to be False, then they won't show up, but their **template will still be in theme_page_sidebar_items. As a result, two empty <div>s will be created corresponding to the empty template of each.

What we can do to improve this

Short-term fix

First off, I think that @drammock and @tupui can solve their problem by defining:

html_theme_options = {
  ...
  "secondary_sidebar_items": ["page-toc"],
  ...
}

This should remove those empty <div> elements entirely.

Long-term fix

Ultimately, I think the behavior that we would want is:

  • Setting any of the config for these features to False should also remove it from the secondary_sidebar_items list. Similar to what we do with the primary sidebar if there are no navigation entries to display.
  • If there are no entries in the secondary-sidebar, then the whole sidebar should not show up at all, and allow extra space for the content.
  • Either way, we need to document all of these more clearly!

If somebody wants to give a shot at the long-term fix, I'm happy to review!

choldgraf avatar Aug 13 '22 12:08 choldgraf

I will try the work around tomorrow thanks!

tupui avatar Aug 15 '22 15:08 tupui

I can confirm that it does fix the issue (and does not seem to have unwanted side effect for me), thanks! 😃

tupui avatar Aug 17 '22 14:08 tupui

html_theme_options = {
  ...
  "page_sidebar_items": ["page-toc"],
  ...
}

agreed that this does fix things for MNE-Python docs too. I'll see if I can figure out the long-term fix.

drammock avatar Aug 17 '22 20:08 drammock

Also since there's a very easy workaround (just one line change to conf.py) I'm comfortable removing the block-release tag on this one.

drammock avatar Aug 17 '22 20:08 drammock

Just a note that I believe this will now always be the case unless you manually remove the secondary sidebar, because the searchbox.html template is included in the sidebar template list. This is expected to be empty, and is only used to place the "click to clear search highlights" button.

choldgraf avatar Jan 11 '23 12:01 choldgraf

FYI I rebuild recently with main and the fix did not work anymore.

tupui avatar Jan 11 '23 12:01 tupui

@tupui can you please provide more context than this? It is hard to understand what might be going wrong without at least a link to the configuration you're using and a reproducible example.

choldgraf avatar Jan 11 '23 12:01 choldgraf

In the discussion we said that this was a workaround to the issue

html_theme_options = {
  ...
  "secondary_sidebar_items": ["page-toc"],
  ...
}

I am saying that it's not the case anymore. Compiling SciPy (cf the PR I linked in the description) still leads to the issue. Maybe @drammock can confirm using the same example he had?

Sorry I don't have more time to put into investigating doc issues at the moment. And tbh, I don't have the skills as I know close to nothing about internals of Sphinx. So it could take me unreasonable time to track down something.

tupui avatar Jan 11 '23 13:01 tupui

I suspect that you've since upgraded to a newer version of this theme. The configuration value is now secondary_sidebar_items, check the docs here: https://pydata-sphinx-theme.readthedocs.io/en/stable/user_guide/layout.html#secondary-sidebar-right

choldgraf avatar Jan 11 '23 13:01 choldgraf

Yes I do have that, sorry for not updating the code e.g.

tupui avatar Jan 11 '23 13:01 tupui

I think that this PR will fix the issue:

  • https://github.com/pydata/pydata-sphinx-theme/pull/1115

choldgraf avatar Jan 11 '23 13:01 choldgraf

Great, thank you for having a look! Happy to test this when you want.

tupui avatar Jan 11 '23 13:01 tupui