-
Notifications
You must be signed in to change notification settings - Fork 61
Hugo: add searchTerm option to search shortcode #489
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: master
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for cue ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
3eb1865
to
a8713db
Compare
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.
Overall looks good! Just 1 testcase where it doesnt work: searchTerm set but not tags pre-selected
a8713db
to
2441f96
Compare
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.
The only edge case which I found while testing: Remove all preselected terms & tags: the search will reset to the preselected filters & term. There is no way to empty it. But I don't know how to fix that because on page load there is also no search term and than the preselected term kicks in: that's what we want. There's no way to know now if the user emptied it or if we started fresh. It's also preselected for a reason :)
2441f96
to
64a4e58
Compare
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! Please just fix the DCO and my last comment, and I'll submit this :-)
* Add searchTerm to example page * Pass searchTerm from search shortcode, through search partial, to filter partial * Set initial searchItem with search filter To test: * Navigate to /examples/shortcodes/search and verify that the search results are accurate. Fixes cue-lang/cue#3566 Signed-off-by: juanstelling <[email protected]>
64a4e58
to
9bc6197
Compare
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 - I'll import into Gerrit after we've resolved the unrelated CI test failures.
To test:
Fixes cue-lang/cue#3566