-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Svelte: add repo popovers to repo name in header and in dynamic filters #62865
Changes from 5 commits
7f9f0c2
7e31690
f7a8bcb
382eeef
02696ac
d6ff20a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,14 +1,10 @@ | ||
<script lang="ts"> | ||
import { mdiClose } from '@mdi/js' | ||
import { page } from '$app/stores' | ||
import Icon from '$lib/Icon.svelte' | ||
import { Button } from '$lib/wildcard' | ||
import CountBadge from './CountBadge.svelte' | ||
import { updateFilterInURL, type SectionItem } from './index' | ||
import type { SectionItemData } from './index.ts' | ||
import SectionItem from './SectionItem.svelte' | ||
export let items: SectionItem[] | ||
export let items: SectionItemData[] | ||
export let title: string | ||
export let filterPlaceholder: string = '' | ||
export let showAll: boolean = false | ||
|
@@ -35,23 +31,17 @@ | |
<ul> | ||
{#each limitedItems as item} | ||
<li> | ||
<a | ||
href={updateFilterInURL($page.url, item, item.selected).toString()} | ||
class:selected={item.selected} | ||
on:click={() => onFilterSelect(item.kind)} | ||
> | ||
<span class="label"> | ||
<slot name="label" label={item.label} value={item.value}> | ||
{item.label} | ||
</slot> | ||
</span> | ||
<CountBadge count={item.count} exhaustive={item.exhaustive} /> | ||
{#if item.selected} | ||
<span class="close"> | ||
<Icon svgPath={mdiClose} inline /> | ||
</span> | ||
<slot name="item" {item}> | ||
{#if $$slots.label} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 |
||
<SectionItem {item} {onFilterSelect}> | ||
<svelte:fragment slot="label" let:label let:value> | ||
<slot name="label" {label} {value} /> | ||
</svelte:fragment> | ||
</SectionItem> | ||
{:else} | ||
<SectionItem {item} {onFilterSelect} /> | ||
{/if} | ||
</a> | ||
</slot> | ||
</li> | ||
{/each} | ||
</ul> | ||
|
@@ -135,50 +125,4 @@ | |
.show-more { | ||
text-align: center; | ||
} | ||
a { | ||
display: flex; | ||
width: 100%; | ||
align-items: center; | ||
border: none; | ||
text-align: left; | ||
text-decoration: none; | ||
border-radius: var(--border-radius); | ||
color: inherit; | ||
white-space: nowrap; | ||
gap: 0.25rem; | ||
padding: 0.25rem 0.5rem; | ||
margin: 0; | ||
font-weight: 400; | ||
.label { | ||
flex: 1; | ||
text-overflow: ellipsis; | ||
overflow: hidden; | ||
color: var(--text-body); | ||
} | ||
&:hover { | ||
background-color: var(--color-bg-3); | ||
.label { | ||
color: var(--text-title); | ||
} | ||
} | ||
&.selected { | ||
background-color: var(--primary); | ||
color: var(--light-text); | ||
--color: var(--light-text); | ||
.label { | ||
color: var(--light-text); | ||
} | ||
} | ||
.close { | ||
flex-shrink: 0; | ||
} | ||
} | ||
</style> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
<script lang="ts"> | ||
import { mdiClose } from '@mdi/js' | ||
import { page } from '$app/stores' | ||
import Icon from '$lib/Icon.svelte' | ||
import CountBadge from './CountBadge.svelte' | ||
import { updateFilterInURL, type SectionItemData } from './index' | ||
export let item: SectionItemData | ||
export let onFilterSelect: (kind: SectionItemData['kind']) => void = () => {} | ||
</script> | ||
|
||
<a | ||
href={updateFilterInURL($page.url, item, item.selected).toString()} | ||
class:selected={item.selected} | ||
on:click={() => onFilterSelect(item.kind)} | ||
> | ||
<span class="label"> | ||
<slot name="label" label={item.label} value={item.value}> | ||
{item.label} | ||
</slot> | ||
</span> | ||
<CountBadge count={item.count} exhaustive={item.exhaustive} /> | ||
{#if item.selected} | ||
<span class="close"> | ||
<Icon svgPath={mdiClose} inline /> | ||
</span> | ||
{/if} | ||
</a> | ||
|
||
<style lang="scss"> | ||
a { | ||
display: flex; | ||
width: 100%; | ||
align-items: center; | ||
border: none; | ||
text-align: left; | ||
text-decoration: none; | ||
border-radius: var(--border-radius); | ||
color: inherit; | ||
white-space: nowrap; | ||
gap: 0.25rem; | ||
padding: 0.25rem 0.5rem; | ||
margin: 0; | ||
font-weight: 400; | ||
.label { | ||
flex: 1; | ||
text-overflow: ellipsis; | ||
overflow: hidden; | ||
color: var(--text-body); | ||
} | ||
&:hover { | ||
background-color: var(--color-bg-3); | ||
.label { | ||
color: var(--text-title); | ||
} | ||
} | ||
&.selected { | ||
background-color: var(--primary); | ||
color: var(--light-text); | ||
--color: var(--light-text); | ||
.label { | ||
color: var(--light-text); | ||
} | ||
} | ||
.close { | ||
flex-shrink: 0; | ||
} | ||
} | ||
</style> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,20 +32,24 @@ | |
import { goto } from '$app/navigation' | ||
import { page } from '$app/stores' | ||
import { getGraphQLClient } from '$lib/graphql' | ||
import Icon from '$lib/Icon.svelte' | ||
import ArrowBendIcon from '$lib/icons/ArrowBend.svelte' | ||
import LanguageIcon from '$lib/LanguageIcon.svelte' | ||
import Popover from '$lib/Popover.svelte' | ||
import RepoPopover, { fetchRepoPopoverData } from '$lib/repo/RepoPopover/RepoPopover.svelte' | ||
import CodeHostIcon from '$lib/search/CodeHostIcon.svelte' | ||
import SymbolKindIcon from '$lib/search/SymbolKindIcon.svelte' | ||
import { displayRepoName, scanSearchQuery, type Filter } from '$lib/shared' | ||
import { SVELTE_LOGGER, SVELTE_TELEMETRY_EVENTS } from '$lib/telemetry' | ||
import Tooltip from '$lib/Tooltip.svelte' | ||
import { delay } from '$lib/utils' | ||
import { Alert } from '$lib/wildcard' | ||
import Button from '$lib/wildcard/Button.svelte' | ||
import HelpFooter from './HelpFooter.svelte' | ||
import { | ||
type URLQueryFilter, | ||
type SectionItem, | ||
type SectionItemData, | ||
Comment on lines
-48
to
+52
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Renamed because I wanted a component named |
||
staticTypeFilters, | ||
typeFilterIcons, | ||
groupFilters, | ||
|
@@ -54,14 +58,15 @@ | |
} from './index' | ||
import LoadingSkeleton from './LoadingSkeleton.svelte' | ||
import Section from './Section.svelte' | ||
import SectionItem from './SectionItem.svelte' | ||
export let searchQuery: string | ||
export let streamFilters: Filter[] | ||
export let selectedFilters: URLQueryFilter[] | ||
export let state: 'complete' | 'error' | 'loading' | ||
$: groupedFilters = groupFilters(streamFilters, selectedFilters) | ||
$: typeFilters = staticTypeFilters.map((staticTypeFilter): SectionItem => { | ||
$: typeFilters = staticTypeFilters.map((staticTypeFilter): SectionItemData => { | ||
const selectedOrStreamFilter = groupedFilters.type.find( | ||
typeFilter => typeFilter.label === staticTypeFilter.label | ||
) | ||
|
@@ -83,7 +88,7 @@ | |
} | ||
} | ||
function handleFilterSelect(kind: SectionItem['kind']): void { | ||
function handleFilterSelect(kind: SectionItemData['kind']): void { | ||
SVELTE_LOGGER.log(SVELTE_TELEMETRY_EVENTS.SelectSearchFilter, { kind }, { kind }) | ||
} | ||
|
@@ -119,13 +124,24 @@ | |
filterPlaceholder="Filter repositories" | ||
onFilterSelect={handleFilterSelect} | ||
> | ||
<svelte:fragment slot="label" let:label> | ||
<Tooltip tooltip={label} placement="right"> | ||
<span> | ||
<CodeHostIcon disableTooltip repository={label} /> | ||
<span>{displayRepoName(label)}</span> | ||
</span> | ||
</Tooltip> | ||
<svelte:fragment slot="item" let:item> | ||
<Popover showOnHover let:registerTrigger placement="right-start"> | ||
<div use:registerTrigger> | ||
<SectionItem {item}> | ||
<svelte:fragment slot="label" let:label> | ||
<CodeHostIcon disableTooltip repository={label} /> | ||
<span>{displayRepoName(label)}</span> | ||
</svelte:fragment> | ||
</SectionItem> | ||
</div> | ||
<svelte:fragment slot="content"> | ||
{#await delay(fetchRepoPopoverData(getGraphQLClient(), item.label), 200) then data} | ||
<RepoPopover {data} withHeader /> | ||
{:catch error} | ||
<Alert size="slim" variant="danger">{error}</Alert> | ||
{/await} | ||
</svelte:fragment> | ||
</Popover> | ||
</svelte:fragment> | ||
Comment on lines
+127
to
145
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
</Section> | ||
<Section | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,9 +15,13 @@ | |
|
||
import { page } from '$app/stores' | ||
import { computeFit } from '$lib/dom' | ||
import { getGraphQLClient } from '$lib/graphql' | ||
import Icon from '$lib/Icon.svelte' | ||
import GlobalHeaderPortal from '$lib/navigation/GlobalHeaderPortal.svelte' | ||
import { DropdownMenu, MenuLink } from '$lib/wildcard' | ||
import Popover from '$lib/Popover.svelte' | ||
import RepoPopover, { fetchRepoPopoverData } from '$lib/repo/RepoPopover/RepoPopover.svelte' | ||
import { delay } from '$lib/utils' | ||
import { Alert, DropdownMenu, MenuLink } from '$lib/wildcard' | ||
|
||
import type { LayoutData } from './$types' | ||
import RepoSearchInput from './RepoSearchInput.svelte' | ||
|
@@ -80,7 +84,16 @@ | |
|
||
<GlobalHeaderPortal> | ||
<nav aria-label="repository"> | ||
<h1><a href="/{repoName}">{displayRepoName}</a></h1> | ||
<Popover showOnHover placement="bottom-start" let:registerTrigger> | ||
<h1 use:registerTrigger><a href="/{repoName}">{displayRepoName}</a></h1> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree, but I think we should save this for the planned header redesign |
||
<svelte:fragment slot="content"> | ||
{#await delay(fetchRepoPopoverData(getGraphQLClient(), repoName), 200) then data} | ||
<RepoPopover {data} withHeader /> | ||
{:catch error} | ||
<Alert size="slim" variant="danger">{error}</Alert> | ||
{/await} | ||
</svelte:fragment> | ||
</Popover> | ||
|
||
<ul use:computeFit on:fit={event => (visibleNavEntries = event.detail.itemCount)}> | ||
{#each navEntriesToShow as entry} | ||
|
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.
By the way, do we need to specify a key properly in this loop here?
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 don't think so since the array is immutable. But we should probably be explicit about it