-
Notifications
You must be signed in to change notification settings - Fork 61
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
Optionally use material search #202
Conversation
the `search/search_index.json` template behaves differently based on which search plugin is being used. with this change, we allow users to use material theme's search for generating the index.
(validated this works as expected in https://github.com/alexef/mkdocs-techdocs-core/blob/dev/src/core.py#L22, but I'm yet to validate that using custom settings like the proposed way in this PR will work) |
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 for taking this on @alexef, left one comment.
I'm also wondering, would this impact the core Backstage Search feature? Would you be able to test that to confirm?
We are using it locally and search seems to work just like before (except that now we don't get duplicate results). Updated README added a tiny unit test. If you can please have another look. |
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.
Small typo, other changes look good though 👍
Co-authored-by: Andre Wanlin <[email protected]>
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 again @alexef 🚀
I'm the initial review but we need the TechDocs team to look as well so I'll pull them in. It might take a bit longer as it's a holiday tomorrow in the US.
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.
Looks great, thank you for your contribution!
Hello 👋 First of all thanks for the PR! We also have the need to boost individual pages in a TechDocs but we didn't implement this yet. We tried using the The text contains html tags (like We've seen also that when using Elasticsearch with Highlighting, the html tags are not taken into account (html tags can be truncated in the search result highlight and highlighting segments conflicts with the html tags). How are you handling the display of search results in Backstage ? |
@tcardonne atm I'm using a custom document collator in backstage, which I put in between the default one (that downloads search-index.json from s3) and I strip the tags before they are sent to Opensearch. So my index doesn't contain html tags, and the search results (with highlighting) work just like before. In backstage/backstage#25497 I'm preparing a less intrusive method that would allow minimal changes to the code, and allow us to continue to use the |
up |
fixes: #192
see also: backstage/backstage#25497
the
search/search_index.json
template behaves differently based on which search plugin is being used.with this change, we allow users to use material theme's search for generating the index.
Differences: