-
-
Notifications
You must be signed in to change notification settings - Fork 590
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
Fix some Pagefind UI issues #2848
Conversation
🦋 Changeset detectedLatest commit: cf5e110 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for astro-starlight ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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 this looks great @HiDeoo! I don’t have major feedback on the design — I think it looks pretty good (definitely better than before 😅) — and given this is a less used Pagefind feature, I’m happy to have it working nicely like this.
Re: the changesets, yeah I guess these could be condensed into a single one? Maybe even as simple as this?
Fixes styling of filter and metadata elements in Pagefind search UI
The “load more” button hasn’t actually changed, so probably doesn’t need mentioning IMO.
But no notes from me on the code changes!
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! LGTM ✅ 🚀
Co-authored-by: Chris Swithinbank <357379+delucis@users.noreply.github.com>
Description
--sl-color-accent-light
in Search component is not defined #2840This PR fixes a few UI issues related to the Pagefind UI.
Note that I'm not super happy with the changesets, I think I didn't manage to get the right balance between what is changed and the level of detail in the changeset, maybe even 1 changeset could be enough? Happy to iterate on this based on feedback.
Unknown CSS variable
This issue was the first one reported in #2840. This PR removes the unknown CSS variable (
--pagefind-ui-primary
) and now uses the desired one (--sl-color-text
) which was already the color used when the browser was falling back to the default color when the variable was not defined.The
--pagefind-ui-primary
variable is used in 2 places in Pagefind, the first one being the "Load more results" button:The second place is the checkbox background color when using filters where some extra CSS was added to fix the tick color:
Ultimately, some changes could be made upstream to avoid this issue and rely on accent colors for the checkbox and should be beneficial for all Pagefind users as it's also an issue with a default Pagefind setup.
Metadata
When using the Pagefind metadata feature, the UI was broken. I tried to use the same UI as inline code at the moment for the tags.
This PR improves the UI but as usual when it comes to UI, that's not my strong suit so happy to iterate on this based on feedback/suggestions.