-
-
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. |
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 <[email protected]>
3d3d32c
to
6ff3ea9
Compare
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 <[email protected]>
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 <[email protected]>
Signed-off-by: Mike Fiedler <[email protected]>
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. |
Signed-off-by: Mike Fiedler <[email protected]>
0019871
to
2352138
Compare
These pages are effectively indexes themselves, no need to include them. Signed-off-by: Mike Fiedler <[email protected]>
@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 |
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/