Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plugins like search need to be namespaced in overrides #4581

Closed
4 of 5 tasks
ben519 opened this issue Oct 31, 2022 · 8 comments
Closed
4 of 5 tasks

Plugins like search need to be namespaced in overrides #4581

ben519 opened this issue Oct 31, 2022 · 8 comments

Comments

@ben519
Copy link
Sponsor

ben519 commented Oct 31, 2022

Contribution guidelines

I've found a bug and checked that ...

  • ... the problem doesn't occur with the mkdocs or readthedocs themes
  • ... the problem persists when all overrides are removed, i.e. custom_dir, extra_javascript and extra_css
  • ... the documentation does not mention anything about my problem
  • ... there are no open or closed issues that are related to my problem

Description

For my site, I need to insert Sign Up / Log In buttons into the header, so I've been overriding header.html. I recently noticed that the search bar in my header disappeared, and after some investigation I discovered that overriding header.html with an exact replicate of src/partials/header.html also breaks the header.

I guess this is not technically a bug, but I would argue that one should be able to copy any source .html partial into an overrides/ directory, and (without making any modifications) it should behave exactly the same.

I can fix my particular issue by changing {% if "search" in config.plugins %} to {% if "material/search" in config.plugins %}, but this feels hacky.

Expected behaviour

I would expect that overriding header.html with a replicate of src/partials/header.html should behave the same as not making the override at all.

Actual behaviour

Suppose you have search enabled...

mkdocs.yml

site_name: ABC

theme:
  name: material
  custom_dir: overrides

plugins:
  - search

nav:
  - Home: index.md
  - Dogs:
    - dogs/index.md
    - Golden Retrievers:
      - Golden Retrievers: dogs/golden-retrievers/index.md
      - Movies: dogs/golden-retrievers/movies.md

If you override header.html with a replicate of src/partials/header.html, the search bar will not appear.

Steps to reproduce

I have created a minimal reproducible example of this here.

  1. clone the repo
  2. mkdocs build && mkdocs serve

Notice the search bar is missing from the header.

Package versions

  • Python: 3.10.6
  • MkDocs: 1.4.1
  • Material: 8.5.7+insiders.4.26.1

Configuration

site_name: ABC

theme:
  name: material
  custom_dir: overrides

plugins:
  - search

nav:
  - Home: index.md
  - Dogs:
    - dogs/index.md
    - Golden Retrievers:
      - Golden Retrievers: dogs/golden-retrievers/index.md
      - Movies: dogs/golden-retrievers/movies.md

System information

  • Operating system: Mac OS 12.6
  • Browser: Version 106.0.5249.119 (Official Build) (arm64)
@squidfunk
Copy link
Owner

Thanks for reporting! So this is actually an error that comes from a temporary transitional inconsistency between Insiders and the community edition. The transition from search to material/search was introduced with the advent of MkDocs 1.4.1, and allows themes to bundle built-in plugins with the same names as third-party plugins by name-spacing them, which we're doing in Material for MkDocs (i.e. tags and search). This is tracked in #4504. It's an important change before the new search plugin is released.

Why is it not merged yet? Because it's a pretty fundamental change that might break some builds (as yours). I've discussed this with @oprypin and he suggested to first test-drive this on a smaller group (Insiders) before merging it here. I'm glad we did this. When the next funding goal is hit, which should be a matter of weeks, the PR is merged and a new major release (version 9) will include upgrade instructions for template overrides.

For my site, I need to insert Sign Up / Log In buttons into the header, so I've been overriding header.html. I recently noticed that the search bar in my header disappeared, and after some investigation I discovered that overriding header.html with an exact replicate of src/partials/header.html also breaks the header.

What's wrong here is that you're copying the header partial from the community edition (this repository), not Insiders. If you use the header.html partial from Insiders, overriding will work as expected, as it uses material/search.

In the meantime I'm sorry for the inconvenience this is causing. I'm closing this issue as there's nothing for us to do here.

@oprypin
Copy link
Contributor

oprypin commented Nov 8, 2022

@squidfunk

@oprypin and he suggested to first test-drive this on a smaller group (Insiders) before merging it here

When the next funding goal is hit

Hmm I think maybe it's better to switch sooner. As demonstrated by this report, by now we're more likely to get an issue from the discrepancy than an issue from the actual rename.

And meanwhile I got a general inquiry towards why the original pull request "didn't do anything" 😅
mkdocs/mkdocs#2998 (comment)

@squidfunk
Copy link
Owner

squidfunk commented Nov 9, 2022

The problem is that I would consider this a breaking change, thus a major version is necessary, and I don't want to release two major versions in close proximity. The search funding goal should be hit any time now. Furthermore, you're only impacted if you've overridden header.html or content.html, as those are the only templates including plugin checks.

Edit: ah okay, I see, the user wants to use the default plugin while keeping Material for MkDocs installed. As said, I can't make the merge right now, so the best idea is to temporarily uninstall Material for MkDocs and/or use a virtual environment.

@harabat
Copy link

harabat commented Jan 7, 2023

@squidfunk do you think it'd be helpful to change the title of this issue to "MkDocs Material plugins like search need to be name-spaced in overrides files" or maybe even explicitly mention this breaking change in the "Changes to customizations" paragraph in Material for MkDocs 9 – Beta (#4714) (like you did the .title change)?

Maybe I missed some explicit reference to this elsewhere, but it took me a long while to find this solution after upgrading, and I was only able to find it because OP mentioned it in passing here.

@squidfunk
Copy link
Owner

Yes, it's a good idea mentioning it. I'll also change the title

@squidfunk squidfunk changed the title header.html is not easily overridden. Copying src/partials/header.html into overrides/partials/ breaks the header. Plugins like search need to be namespaced in overrides Jan 7, 2023
@ghost
Copy link

ghost commented Jun 12, 2023

Hi !
I'm experiencing this issue right now as well. I need to override the header and to keep the search active, but I obviously can't right now. I tried to access the insider repo to get the header.html from there, but as I figured, I'm not an insider, and have no rights to access this repo.
How can I solve this on my side at the moment, please ?

@squidfunk
Copy link
Owner

The new search was released with Material for MkDocs 9, so the comments in this thread are outdated as it's not exclusive to Insiders anymore. Make sure that you adjust your overrides to check for material/search, that should be enough:

{% if "material/search" in config.plugins %}
<label class="md-header__button md-icon" for="__search">
{% include ".icons/material/magnify.svg" %}
</label>

@ghost
Copy link

ghost commented Jun 12, 2023

Oh awesome ! Thank you very much !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants