-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
feat: add pagefind search #4247
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?
Conversation
ddc20a4 to
3d3d32c
Compare
|
Definitely needs more styling, and I put it at the top of the sidebar since that made sense to me, but it might be better elsewhere. Left a TODO - I'm not sure how/when the nodes attributes are getting cleared after But give it a whirl! |
|
The search box looks a little big to me, and it displays "Python Enhancement Proposals" instead of the page titles. But it already looks promising. |
|
@miketheman I'm not familiar with pagefind. I tried the preview site and it works well. Thank you! |
|
Thanks gang! Definitely needs some styling, and mobile theme integration, since that doesn't appear to work on my phone yet. |
3d3d32c to
6ff3ea9
Compare
|
Ready for a second round of review. Default styling removed, so that inline/inherited styles work "just well enough". |
|
The titles are now correct, but it seems the numbering resets every 10 entries. |
Thanks for checking! Can you share a search that repeats, and maybe a screenshot of the results? |
|
@nineteendo Thanks! That looks to be a Safari-specific behavior, and poking around the HTML it looks like the numbers are accurate, but the first digit is being visually trimmed by not having enough room to display. I'd appreciate it if you can confirm my finding by testing another browser. I'll keep poking at it for bit longer, to see if there's a "good enough" CSS directive that will fix. |
0019871 to
2352138
Compare
|
@nineteendo I've padded the numbers a little, should display correctly in Safari now. I've also added an exclusion for non-relevant pages, so results should be better. Searching for |
This comment was marked as outdated.
This comment was marked as outdated.
I'll bump it up to 1.8rem in the next commits. Note this this will have the same problem at 1000 results, but I'm also okay, since anyone having to navigate down that far is probably using the wrong search term anyways (and we only have ~708 pages today). |
Instead of using sphinx's built-in search which has some challenges in its implementation, as well as the integration with the customized builder and theme, use a different approach. Pagefind will generate fragments that can be loaded client-side efficiently. Refs: https://pagefind.app/ Refs: https://pypi.org/project/pagefind/ Signed-off-by: Mike Fiedler <miketheman@gmail.com>
Since `title` variable is already available in the HTML, use that and hide it from view instead of trying to muck with the nodes during transformation. Signed-off-by: Mike Fiedler <miketheman@gmail.com>
The default inherited styles are good enough to get started, and styling can happen at any point with customization in `style.css` instead of trying to mash the default style to work here. Refs: https://pagefind.app/docs/ui-usage/#customising-the-styles Signed-off-by: Mike Fiedler <miketheman@gmail.com>
Signed-off-by: Mike Fiedler <miketheman@gmail.com>
Signed-off-by: Mike Fiedler <miketheman@gmail.com>
These pages are effectively indexes themselves, no need to include them. Signed-off-by: Mike Fiedler <miketheman@gmail.com>
Signed-off-by: Mike Fiedler <miketheman@gmail.com>
For the 100-999 range of results. Signed-off-by: Mike Fiedler <miketheman@gmail.com>
This reverts commit eedb14b.
This reverts commit ed6cea3.
Signed-off-by: Mike Fiedler <miketheman@gmail.com>
Signed-off-by: Mike Fiedler <miketheman@gmail.com>
a865e71 to
cae73c6
Compare
|
It's taken some time, but I believe I have a reasonable changeset now - let me know if you'd prefer me to rearrange the commits in any way. |
Thanks, I thought I had fixed that. 🤔 What browser is this? |
|
Safari. |
|
Pressing "/" pops open the Read the Docs search box, which doesn't show any results when typing in the box. I was expecting it instead to focus the cursor in the new search box at the top left, similar to https://docs.python.org and https://pypi.org. |
With relative spacing, if the form grows (as it does when results come in), the button would move along with it. With absolute values and no transform, it stays where it needs to stay. Signed-off-by: Mike Fiedler <miketheman@gmail.com>
|
@nineteendo Thanks - super helpful! I've added a change to make the clear button placement static. @hugovk My changes did not add a slash shortcut yet - I wanted to get the basics running before adding more JavaScript. |



Instead of using sphinx's built-in search which has some challenges in its implementation, as well as the integration with the customized builder and theme, use a different approach.
Pagefind will generate fragments that can be loaded client-side efficiently.
Refs: https://pagefind.app/
Refs: https://pypi.org/project/pagefind/
Resolves #4083
📚 Documentation preview 📚: https://pep-previews--4247.org.readthedocs.build/