-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
DOC use Algolia for the search bar #29138
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
Conversation
Apart from the choice of rendering and color, I think that the PR is ready for a review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @glemaitre, this looks very nice! I left some minor comments on styling:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We might also need to take care of algolia/sphinx-docsearch#35. I've tried several ways and the only one working is:
html_theme_options = {
# ...
"navbar_start": ["navbar-logo", "searchbox"],
"navbar_persistent": [],
# ...
}
All other places either: (1) has two copies for different screen sizes while algolia can add the search input box only for one, or (2) do not persist when scrolling the page thus not proper for a search box.
Remarks regarding navbar_persistent
: sphinx_docsearch
overwrites this field forcefully, so can simply remove it (REF).
Also a minor typo in conf.py
:
Typo: # Renable built-in search disable by sphinx-docsearch
Should be: # Renable built-in search disabled by sphinx-docsearch
An additional question: It seems that the search results in the preview docs link to pages in the stable version doc? |
Yes. I think that it will be one of the limitation. The Algolia crawler is configured to crawl the stable website to populate the index. I assume that if I had the dev website into the index, we will double reference for each entry. I think this is more reasonable to have the current setting. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks @glemaitre for integrating Algolia search into the theme! I like the instant search experience which I think is widely available nowadays but missing for most Python documentation websites. Just some final suggestions (styling issues that I overlooked and some simplification) ;)
hmm, that's a pity. How do other projects deal with this then?
What's your own feeling about these raised issues? |
From the project that I found, either they are doing as it is in this PR or they usually have a single version. The only one that do something good is the bootstrap version. But I'm not sure how to find the way they do it. I have to check but it might be in the config of the index generation.
They are justified but as I don't know how to solve those and I have difficulties to envisage what it takes to get there. |
@Charlie-XIAO what's your take on the thoughts here? |
Actually I found some "facets" feature that seems to answer this question. I'll try to make work the crawler with it. |
OK, so for the versioning, you need to use meta-tags such that the crawler use it when you do the crawling. https://docsearch.algolia.com/docs/required-configuration/#introduce-global-information-as-meta-tags This is the way bootstrap is doing it: ![]() (e.g. source from https://getbootstrap.com/docs/5.3/getting-started/introduction/) It might be something that we want to ask |
Another thing I realize (while working on a very unrelated PR) is how this is going to work in development environment. Right now, we can search in the local build of the docs and the results point to the local build, and I guess this will break that. It also certainly won't work w/o internet, but I guess that's rarely the case. At least with slow internet, I tested (with firefox tools) and this still works. |
I you build locally, I don't have the credential for Algolia so the docsearch is deactivated. Since we are still building the sphinx index, it would actually be great to switch to it. I don't know if as a side effect right now this is actually the case. |
Apart from instance search, are there other benefits we want from Algolia search? If not probably it's easier to implement instant search based on the current sphinx search framework than to do all those customizations on Algolia docsearch, but I haven't investigated yet. |
More than instant search, this is more about fuzzy search on my side. But both features are really nice. |
Okay then I may still investigate how to customize Algolia first. Not sure if it's possible to add search context but an "all results" page is definitely possible. |
I made a small demo on how to create an redirect to an Algolia "all results" page. 2024-07-28.121142.mp4If we want to switch to Algolia, I do think we need to work on refining the search results though. |
And responding to some earlier comments:
Actually there is short context, if the result type is "content". But they will be ordered after the ones whose title matches the query. For instance if you search "a", then all pages having "a" in titles will appear before those having "a" in contents, and since the search widget only lists the top ones you can see only those with no context. There is indeed a
See demo in #29138 (comment); this overwrites the default sphinx search page, and can be linked from the footer of the Algolia search widget in each page. It is implemented with Algolia instantsearch.
This I haven't found a good way. The Algolia docsearch behavior is that "Enter" is "select", so it might be bad (and hard) to override it (in a decent way). I might be able to bind some other key though, e.g. Alt+Enter for going to the "all results" page.
I haven't investigated and don't have access to the indices, but see Guillaume's comment #29138 (comment)
We can use an environement variable (e.g. |
Thanks @Charlie-XIAO. The full search page is already something that would have take me ages and a lot of frustration to do :) With this addition, it seems that we are getting closer to the expectations. |
Ideally by default nothing would be "selected" really, and enter w/o arrow keys would show the page. If the user "selects" something explicitly with arrow keys, then it makes sense for enter to show that page. |
I agree but it's kind of beyond my knowledge how to achieve this - will try to investigate a bit more. BTW @glemaitre do you mind my directly pushing my updated version into this PR? |
Absolutely not :) Feel free to push any improvement. I wanted just to spin something out with the tool. |
I'm not sure why CircleCI failed to checkout: https://app.circleci.com/pipelines/github/scikit-learn/scikit-learn/60621/workflows/0d66d144-1d21-4da8-9094-190a4927aab0/jobs/282498
|
That's a very cryptic error @Charlie-XIAO . I re-ran the workflow, but same issue. You could open a new PR and see if that helps (we can close this one then) |
So CI is good in #29666, I think we can move the discussion to there so we can see the built artifacts. |
Closing to keep the discussions in the other PR. |
Take over #28478
Give another try to the Algolia docsearch as a search bar.