Fix missing sidebar

[drammock]

closes #752

Marking as draft because, when I went to improve the docs, I said that if users hard-code the search field in another location, Ctrl+K would focus it (which in fact is not the case... unless that's recently fixed and I just need to rebase?).

Also I noticed an edge case that if both search-button and search-field are included, Ctrl+k focuses and reveals the hidden one... not sure what is the "right" behavior in that case.

[choldgraf]

Hmm - this is the logic that is currently used:

pydata-sphinx-theme/src/pydata_sphinx_theme/theme/pydata_sphinx_theme/components/search-button.html

Lines 11 to 25 in dd9e85d

	// We'll grab the elements we need to modify for the search field
	let form = document.querySelector("form.bd-search");
	let input = form.querySelector("input");
	// Change the symbol to `meta key` if we are a Mac
	var isMac = window.navigator.platform.toUpperCase().indexOf('MAC')>=0;
	if (isMac) {
	let kbd = form.querySelector("kbd.kbd-shortcut__modifier");
	kbd.innerText = "⌘";
	};
	// Select the search input field, and focus the page on it
	input.focus();
	input.select();
	input.scrollIntoView({block: "center"});

However you're right that if there are two search bars on the HTML of the page, it will just pick up the first one it finds. If people want a search bar in the sidebar, they should probably remove the search button from the header.

[drammock]

If people want a search bar in the sidebar, they should probably remove the search button from the header.

Agreed. I'll focus on just getting the keyboard shortcut to work in the search-is-in-sidebar case then

[choldgraf]

Another thing that we could do is make the assumption that the last element that matches form.bd-search is the one we want to add the keyboard shortcut behavior to, instead of the first element. e.g. with:

  // Assume that the last form on the page is the one we want to trigger
  // The first form will generally be the hidden one, so if another exists it was manually put there
 let forms = document.querySelectorAll("form.bd-search");
 let form = forms[forms.length - 1];

[drammock]

even after this change, nox -s test is failing for me locally:

assert not index_html.select(".bd-sidebar-primary")

@choldgraf any idea why?

[choldgraf]

Hmmm, could it be because of one of the test sites?

pydata-sphinx-theme/tests/sites/base/conf.py

Line 23 in 94a9e8c

html_sidebars = {"section1/index": ["sidebar-nav-bs.html"]}

[drammock]

I don't think that explains it... the failing test line is targeting index_html not subpage_html (which is what section1/index is called in the test)

pydata-sphinx-theme/tests/test_build.py

Lines 63 to 64 in c127146

	index_html = sphinx_build.html_tree("index.html")
	subpage_html = sphinx_build.html_tree("section1/index.html")

[drammock]

I can make the test pass by adding "index": [] to the html_sidebars dict in tests/sites/base/conf.py. But this reflects what I think is undesirable behavior (which I also see with nox -s docs-live): namely, the (empty) left sidebar is still there unless I explicitly add index=[] to html_sidebars in conf.py. I think the behavior you want (and the behavior on current main) is that it should automatically go away when it's empty, right? So I think we need to put back the removed logic in sidebar-primary

[choldgraf]

I think that the suggested logic at the comment below should fix it, because if the sidebars only container the nav-items component, and nav-items was empty, then it would be removed from sidebars and this would then be empty as well.

https://github.com/pydata/pydata-sphinx-theme/pull/777/files#r915581382

[drammock]

OK. I didn't understand that suggestion the first time but now I think I get it.

[drammock]

OK, I think I still don't understand what is going on, maybe because I still haven't wrapped my head around the templating changes made while I was on leave. I've added your suggestion in layout.html and it seems to fix the homepage problem (no left sidebar even when index has the default sidebar value of sidebar-nav-bs). Yay! But I couldn't figure out where I ought to add {{ sidebar_nav_html }} in sidebar-primary... and now the right sidebar is present on, e.g., user_guide/index but is missing on, e.g., user_guide/configuration.

I need to step away from this until at least next Wednesday (big deadline tuesday). @choldgraf please feel free to push commits here if you have time; I think your grasp of Jinja and the theme structure far outstrips mine.

[choldgraf]

Something we could do to simplify this further and have similar behavior:

In layout.html, add a line like:

{# Create the sidebar links HTML here to re-use in a few places #}
{# If we have no sidebar links, pop the links component from the sidebar list #}
{%- set sidebar_nav_html = generate_nav_html("sidebar",
    show_nav_level=theme_show_nav_level|int,
    maxdepth=theme_navigation_depth|int,
    collapse=theme_collapse_navigation|tobool,
    includehidden=True,
    titles_only=True)
-%}
{% if not sidebar_nav_html %}
{% set sidebars = sidebars | reject("in", "sidebar-nav-bs.html") | list %}
{% endif %}

Then we can just re-use sidebar_nav_html in our sidebar-primary template ({{ sidebar_nav_html }}). AND, if that HTML is empty, then we remove the sidebar-nav-bs from our sidebars list. That list will then be empty if that was the only template in it, and will then be hidden.

[drammock]

I think this PR is now moot; the sidebar-related fix seems to have been rolled into #726, and the search UX enhancements are now in #807. @choldgraf feel free to re-open if I'm wrong.