-
-
Notifications
You must be signed in to change notification settings - Fork 17
Adds enhanced search #287
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
base: main
Are you sure you want to change the base?
Adds enhanced search #287
Conversation
let path = (page || ("api/v3/search/?q=" + projstr + "+" + encodeURIComponent(str))); | ||
if(debug) { | ||
url = "https://corsproxy.io/?https://readthedocs.org/" + path; | ||
}else{ | ||
url = "/_/" + path; | ||
}; |
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.
@slowe I did bad things to try and get the RTD preview working.
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.
and I broke "load more" in the process 😭
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.
I think I fixed it for load more, but I made it much more ugly in the process!
@slowe here's some things I noticed while testing with the RTD preview:
|
First thing to note: I'm only able to test on a local file system so I may not have the same situation as you.
This is probably down to the change to hide empty tabs. I'll have a look.
Can you give an example of when? I am not seeing the same thing so it is probably down to a difference in expectation.
By "hidden" do you mean "not loaded"?
Please can you advise on what that should be? |
To clarify what I think is happening around the "Load more" button... I make one request to the ReadTheDocs API that includes all the projects. ReadTheDocs then returns up to 50 results at a time e.g. https://readthedocs.org/api/v3/search/?q=project:sunpy+project:ndcube+project:sunraster+project:aiapy+fits I'm then doing the totalling up by project. It is only possible to know the count by project for the results that have been loaded so far. So there may only be, say, "4" results loaded in a particular project but that doesn't mean there couldn't be more in the next batch of 50 results. The "Load more" button should keep appearing until all the results have been loaded. An alternative would be to make n requests to ReadTheDocs, one for each project every time the typeahead activates. This is inefficient and, potentially, acts more like a denial-of-service attack. Is it worth the number of requests to remove a slight, potential, confusion? I would also hope that most people have found what they were looking for within the first 50. |
The styling on the "Load more" button is the same as the styling on the buttons on the front of sunpy.org as it uses the classes |
Ah ha. This is how it looks for me locally: And this is on the "website preview" link (set to But what I said about the styles is because the classes are set in
I didn't add |
Ah ok, that makes sense. It just surprised me that I could see a (4) for a project followed by load more. I assume that if there are no more pages of results to load for any project the load more wont show?
Ah I see, I think we can safely add them by default they are defined in this theme. (Which I see you have done.) @slowe Could you have a look over the changes I made to the cors proxy code if you haven't already? I think there's a couple of things left to do before we merge this, but I think they are mostly on me:
|
A more compact version of the URL building. Remove/define some variables.
Switching to classes `sd-btn sd-bg-primary sd-bg-text-primary` from https://sunpy--287.org.readthedocs.build/projects/sunpy-sphinx-theme/287/web-components.html
I've changed the default "Load more" button styles to
That's the case, yes. It basically depends on the existence (or not) of a Would it help if the icon in each project tab was a filter icon (e.g. https://icons.getbootstrap.com/icons/funnel-fill/) to visually distinguish those tabs from the "All" tab? |
Yeah, I think that's a good idea. |
This adds a first draft of an enhanced search interface that uses ReadTheDocs's API to search across multiple projects as requested by @Cadair.
This uses the existing search feature but then adapts the interface to add inline results rather than redirect to the search results page. If CORS is not enabled (with ReadTheDocs) it will redirect to the standard site search page. Alternatively, you can add
?debug
to the URL to usehttps://corsproxy.io/
to get around CORS issues.The inline search results will be arranged into tabs showing "All" results and results broken down by project. By default, the script will attempt to find projects from the page. It will look for elements with
class="nav-link"
from a menu item withid="Documentation"
. However, you can manually over-ride this behaviour by using the Javascript variableset_search_config
e.g.where:
no-results
- an optional object to set the label that gets displayed when there are no results;load-more
- an optional object to set the class/label of the "load more" button;projects
- an ordered array of projects to include (if empty this will be constructed from the ".nav-link" items found within #Documentation). The order of this array sets the order of the search results tabs.