Skip to content

DOC add Algolia search bar for improved website search #28478

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

Closed

Conversation

glemaitre
Copy link
Member

@glemaitre glemaitre commented Feb 20, 2024

This is a POC using docsearch from Algolia that you bring a better UX when it comes on using the search bar of the website.

This PR build on the top of the work of @Charlie-XIAO.

See #6710 for historical context.

@glemaitre glemaitre changed the base branch from main to new_web_theme February 20, 2024 10:28
Copy link

github-actions bot commented Feb 20, 2024

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

Generated for commit: c5cffcb. Link to the linter CI: here

@ArturoAmorQ
Copy link
Member

I guess you also have to add the sphinx_docsearch dependency on

  • build_tools/update_environments_and_lock_files.py
  • sklearn/_min_dependencies.py

@glemaitre glemaitre marked this pull request as draft February 20, 2024 10:56
@glemaitre
Copy link
Member Author

The issue here is that sphinx-gallery expect to have the search index build by sphinx but it is not because the class has been overwritten by sphinx-docsearch that rely on the index built outside by Algolia.

@glemaitre
Copy link
Member Author

OK. So "infrastructure" is there. However, we did not yet succeed to build the index on Algolia so any request is "not found". We need to fix first the canonical link to let the crawler from Algolia index the website.

@Charlie-XIAO
Copy link
Contributor

I merged main (with #28487) into new_web_theme to have a preview of the search results. By the way I think the style sheet is not taking effect (by maybe you are still working on it since this is a draft). Apart from CSS I have a few questions:

  • Would this require users to have Algolia accounts to build docs locally?
  • Though the pydata-sphinx-theme search bar component is removed, Ctrl/Cmd+K still triggers the original search functionality. I can only think of forcing display: none on the .search-button__wrapper element, but is there a way that we do not trigger it at all?

@glemaitre
Copy link
Member Author

Would this require users to have Algolia accounts to build docs locally?

Nop, only the index are generated and served by Algolia.

Though the pydata-sphinx-theme search bar component is removed, Ctrl/Cmd+K still triggers the original search functionality. I can only think of forcing display: none on the .search-button__wrapper element, but is there a way that we do not trigger it at all?

This is something that we should figure out and was issued in sphinx-algolia: algolia/sphinx-docsearch#14

For the moment, I would like to see if I can manage to have the crawler and the search working ;)

@betatim
Copy link
Member

betatim commented Feb 26, 2024

Just tested the preview and see the following with Firefox (124.0b3 (64-bit)) on macOS.

This is what I see after pressing apple + k on the main page:
Screenshot 2024-02-26 at 17 45 29

and after typing randomfo this is what I see:
Screenshot 2024-02-26 at 17 45 42

Didn't follow the discussion to check if this kind of visual bug is expected or not but thought I'd report it anyway ;)

@glemaitre
Copy link
Member Author

Yep the reason is explain in the message above. This is due to the fact that it triggers also the sphinx search. We would need to figure out how to solve this issue as well.

@Charlie-XIAO
Copy link
Contributor

Charlie-XIAO commented Feb 27, 2024

I found that the search box does not show up on mobile devices. Looking into the pydata_sphinx_theme structure, the navbar-persistent items are actually duplicated (one for mobile size and one for laptop size). Then there will be two same IDs, and Algolia docsearch picks the first one (the one for laptop size). I don't see an easy workaround here:

  1. sphinx-docsearch does not seem to support initializing multiple search containers.
  2. Even if we move to navbar_end or navbar_start we suffer from the same problem.

A possible solution is to not use sphinx-docsearch but use Algolia docsearch directly (this would not be too complex, because sphinx-docsearch is itself a very thin wrapper). Then we use a class (instead of ID) for the component and manually call docsearch twice.

Example

document.querySelectorAll(".class-name").forEach((container) => {
  docsearch({
    container: container,
    appId: "APP_ID",
    indexName: "INDEX_NAME",
    apiKey: "SEARCH_API_KEY",
  });
});

I'm not sure if there is a smarter approach here.

@glemaitre glemaitre deleted the branch scikit-learn:new_web_theme May 20, 2024 17:37
@glemaitre glemaitre closed this May 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

4 participants