search functionality missing

[drammock]

Building docs against current main leads to a total loss of search functionality for our site: no magnifying glass, no Ctrl+k support, and no (old-school) search box. As mentioned here: #744 (comment)

might like to keep the old-style search box as an option?

It seems like this didn't happen. It's not a crisis (for MNE-Python at least) since now we pin to specific commits and update only periodically --- I was "just" testing against 26554f2 locally. But for any site that customizes their topbar / sidebar (I assume that's the relevant customization but haven't yet verified), it might be (or become) a problem if they (1) build from current main, or (2) build from latest release (assuming this isn't fixed before next release).

Not to beat a dead horse, but this is another example of the kind of breaking change that I've complained about before, e.g., in #532. Site search functionality is important, and it's now silently gone from some downstream sites with no warnings/errors in the build process.

Perhaps the TL;DR here is that #744 should have supported the old-style approach, included a test that the old-style approach still worked (e.g., one page in the docs where search is different), and made the new-style searching opt-in.

[drammock]

I should add that our conf.py has

html_sidebars = {
    'index': ['search-field.html', 'sidebar-quicklinks.html'],
}

and, in fact, now the entire sidebar is missing from our index.html (not just the search box); on other pages the sidebar is there, but the search field is missing (presumably because it's default location has changed). So I took a look at the dev docs, which sadly don't help much:

By default, the Search Bar is hidden, and will be displayed when a user [...]

followed by a note later, saying

By default the search bar is placed in the sidebar [...]

So it's not clear what the default is. The docs also say:

if you’d like the search field to be in your side-bar, add it to the sidebar templates like so:

html_sidebars = {
    "**": ["search-field.html", "sidebar-nav-bs.html", "sidebar-ethical-ads.html"]
}

...which we're already doing (at least for index), and it's not working.

[drammock]

result of git bisect indeed points to #744

$ git bisect bad
bbc855b6d7bf402c9b5195d832128e564a4e6cb6 is the first bad commit
commit bbc855b6d7bf402c9b5195d832128e564a4e6cb6
Author: Chris Holdgraf <choldgraf@berkeley.edu>
Date:   Thu Jun 23 11:47:30 2022 +0200

    ENH: Make search field an overlay with button click (#744)

 docs/conf.py                                       |  4 +-
 docs/user_guide/configuring.rst                    | 26 +++++--
 noxfile.py                                         |  2 +-
 src/pydata_sphinx_theme/assets/scripts/index.js    |  7 +-
 .../assets/styles/base/_base.scss                  |  5 +-
 .../assets/styles/components/_icon-links.scss      |  2 +
 .../assets/styles/components/_search.scss          | 91 +++++++++++++++++++++-
 .../assets/styles/components/_switcher-theme.scss  |  1 -
 .../assets/styles/pages/_search.scss               | 70 +++++++++--------
 .../assets/styles/sections/_header.scss            | 31 +++++---
 .../assets/styles/variables/_color.scss            | 12 ++-
 .../components/search-button.html                  | 48 ++++++++++++
 .../components/search-field.html                   |  2 +
 .../theme/pydata_sphinx_theme/search.html          | 45 +++++++++++
 .../sections/sidebar-primary.html                  | 11 ++-
 .../theme/pydata_sphinx_theme/theme.conf           |  4 +-
 .../components/sidebar-nav-bs.html                 |  6 ++
 .../_templates_single_sidebar/sidebar-nav-bs.html  |  6 --
 tests/test_build.py                                | 52 ++++++-------
 tests/test_build/sidebar_ix.html                   | 15 +---
 20 files changed, 320 insertions(+), 120 deletions(-)
 create mode 100644 src/pydata_sphinx_theme/theme/pydata_sphinx_theme/components/search-button.html
 create mode 100644 src/pydata_sphinx_theme/theme/pydata_sphinx_theme/search.html
 create mode 100644 tests/sites/sidebars/_templates_single_sidebar/components/sidebar-nav-bs.html
 delete mode 100644 tests/sites/sidebars/_templates_single_sidebar/sidebar-nav-bs.html

[choldgraf]

It sounds like you've uncovered a bug, so indeed this would be good to fix before a release. Let's add it to the milestone.

[choldgraf]

I have a suspicion of what's going on. If you look at our sidebar code here:

pydata-sphinx-theme/src/pydata_sphinx_theme/theme/pydata_sphinx_theme/sections/sidebar-primary.html

Line 11 in e1d4009

{% if sidebars and sidebar_nav_links %}

We use an AND for:

• sidebars (are any sidebars defined)
• nav links (are there any navigation links)

This means that if sidebars is defined, but there are no nav links, then it will still (incorrectly) return False. I think that's the problem.

Basically the behavior that we want is:

• If there are no sub-pages (nav links) in the sidebar. AND
• There are no other sidebar items other than the nav links

Then hide the sidebar entirely

[drammock]

I have a suspicion of what's going on.

thanks for the hint! I'll take a look

[drammock]

so if I'm understanding the desired logic, I think it's just a matter of changing and to or here? Locally that fixes the problem for the MNE-Python site, along with adding search-button to navbar_end in our conf.py. Tomorrow I'll put together a PR that does that, and that also updates the docs and think about how to write a regression test for this.

[choldgraf]

Actually, my feeling is that we can just remove the and sidebar_nav_links logic entirely, so the check would just be "if sidebars". The reason we were checking for the navbar links was so that the sidebar would hide itself if the only item in it were navbar links, and if those links were empty. But most users are now going to get the search field via an icon pop-up in the header now, rather than in the sidebar anyway, so I suspect it is OK if we remove that special-casing.

so basically remove all of this:

pydata-sphinx-theme/src/pydata_sphinx_theme/theme/pydata_sphinx_theme/sections/sidebar-primary.html

Lines 4 to 10 in e1d4009

	{% set sidebar_nav_links = 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) %}

as well as and sidebar_nav_links

[drammock]

addressed by #726 and #807