Svelte: add repo popovers to repo name in header and in dynamic filters#62865
Svelte: add repo popovers to repo name in header and in dynamic filters#62865camdencheek merged 6 commits intomainfrom
Conversation
camdencheek
force-pushed
the
cc/repo-popover-search-results
branch
from
5ed07ce to
a22641e
Compare
There was a problem hiding this comment.
Extracted this into a new component to make it easier to create a slot with a default, but overridable, value.
There was a problem hiding this comment.
Renamed because I wanted a component named SectionItem now, so it makes sense to call this SectionItemData instead.
There was a problem hiding this comment.
We want to wrap the whole item in the popover, not just the repo name, because otherwise the popover placement varies depending on the length of the repo name.
|
The one on the repo header on the repo pages needs to be offset a little on Y-axis (down a little). |
There was a problem hiding this comment.
By the way, do we need to specify a key properly in this loop here?
There was a problem hiding this comment.
I don't think so since the array is immutable. But we should probably be explicit about it
There was a problem hiding this comment.
I would add a hover state here, it's been a case that people don't understand that this H1 element is clickable, since we are going to have a popover element to this hover state also makes sense here IMO
There was a problem hiding this comment.
Agree, but I think we should save this for the planned header redesign
client/web-sveltekit/src/lib/search/dynamicFilters/Section.svelte
Outdated
Show resolved
Hide resolved
camdencheek
force-pushed
the
cc/repo-popover-search-results
branch
from
00ef510 to
02696ac
Compare
| <SectionItem {item} {onFilterSelect}> | ||
| <slot name="label" slot="label" let:label let:value {label} {value} /> | ||
| </SectionItem> | ||
| {#if $$slots.label} |
There was a problem hiding this comment.
Can this if statement live within SectionItems tags here? Or this would break the default slot as well?
There was a problem hiding this comment.
Unfortunately, it seems there's an edge case where slot rendering cannot be conditional within a component. So I had to copy the fully component in the conditional. Another thing that gets much nicer in svelte 5
3 participants
This adds the
RepoPopoverhover info to the dynamic filters in the search sidebar (replacing the full repo name tooltip) and to the global header.Test plan
screenshot-2024-05-22_14-30-16.mp4