-
Notifications
You must be signed in to change notification settings - Fork 4
Search dialog tweaks #65
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
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
57e115d
to
3239b6b
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.
Nice! Works well when I tested.
Some design thoughts:
- I lean toward removing border-radius to make things look more Membrane-y
- Maybe also the highlighted results should be grayscale?
- The Starlight icons seem like a mismatch visually
} | ||
} | ||
|
||
@media (min-width: 50rem) { |
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.
Why the rem breakpoint?
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.
It's the break point that starlight uses. I copied it from their CSS. I'll add a comment!
src/components/Search.astro
Outdated
-webkit-mask: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' fill='none' stroke-width='0.5' stroke-dasharray='1 1' stroke='currentColor' stroke-linecap='round' viewBox='0 0 16 1000' preserveAspectRatio='xMinYMin slice'%3E%3Cpath d='M8 0v1000m6-988H8'/%3E%3C/svg%3E") | ||
0% 0% / 100% no-repeat; | ||
mask: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' fill='none' stroke-width='0.5' stroke-dasharray='1 1' stroke='currentColor' stroke-linecap='round' viewBox='0 0 16 1000' preserveAspectRatio='xMinYMin slice'%3E%3Cpath d='M8 0v1000m6-988H8'/%3E%3C/svg%3E") | ||
0% 0% / 100% no-repeat; | ||
} | ||
#starlight__search .pagefind-ui__result-nested:last-child::before { | ||
-webkit-mask-image: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' fill='none' stroke-width='0.5' stroke-dasharray='1 1' stroke='currentColor' stroke-linecap='round' stroke-linejoin='round' viewBox='0 0 16 16'%3E%3Cpath d='M8 0v12m6 0H8'/%3E%3C/svg%3E"); | ||
mask-image: url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' fill='none' stroke-width='0.5' stroke-dasharray='1 1' stroke='currentColor' stroke-linecap='round' stroke-linejoin='round' viewBox='0 0 16 16'%3E%3Cpath d='M8 0v12m6 0H8'/%3E%3C/svg%3E"); |
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 the dashed lines look a bit funky, especially near corners. Just my two cents 🤷♂️
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.
src/styles/base.css
Outdated
@media (min-width: 50rem) { | ||
dialog[aria-label="Search"] { | ||
margin: 4rem auto auto; | ||
border-radius: .5rem; |
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.
What do you think of 0 border-radius
? Feels more Membrane-y to me
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.
Fixed. This CSS was copied from the original, didn't notice it was there
I tried this initially but it made it look noisy and hard to parse visually |
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! Left a comment about a potential style change 🤘
width: 90%; | ||
max-width: 40rem; | ||
height: max-content; | ||
min-height: 15rem; |
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.
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 thought the same thing 👍
Co-authored-by: Thomas Seeley <104278845+iamseeley@users.noreply.github.com>
Thank you guys for the review. Unfortunately, pagefind's search results are not very good at the moment, weights don't do much and excerpts are poorly chosen and poorly formatted, especially around tables. The good news is that they are working on it. In the meantime I've applied to Algolia's DocSearch which is the other search provided integrated into starlight. If we get approved we might switch to that. It comes with its own UI so hopefully is customizable too. |
I had to copy the entire
Search.astro
component from starlight to configure pagefind. It still needs more tweaking but they are working on improving the ranking system so we should keep an eye on this milesone: https://github.com/CloudCannon/pagefind/milestone/6The excerpts generated by pagefind are not very good which is why the text appear nonsensical in some cases. I'll ask if there's any work planned on this front, if not, it could be something we contribute to the project: https://github.com/CloudCannon/pagefind