diff --git a/src/components/InboxList.svelte b/src/components/InboxList.svelte index b80eba94a..90fea298b 100644 --- a/src/components/InboxList.svelte +++ b/src/components/InboxList.svelte @@ -3,11 +3,9 @@ import { array, string } from "zod"; - import { - type FilteredRepo, - filterNotifications, - rowIdsOf, - } from "@app/lib/inboxFilter"; + import { filterNotifications, rowIdsOf } from "@app/lib/inboxFilter"; + import { inboxRepoOrder, togglePinned } from "@app/lib/repoOrdering"; + import useLocalStorage from "@app/lib/useLocalStorage.svelte"; import { modifierKey, preserveFocus } from "@app/lib/utils"; import Button from "@app/components/Button.svelte"; @@ -53,7 +51,7 @@ const searchableRepos = $derived( isFiltering - ? notificationsByRepo.filter(r => !hiddenRepos.includes(r.rid)) + ? notificationsByRepo.filter(r => !hiddenRepos.value.includes(r.rid)) : notificationsByRepo, ); @@ -89,82 +87,37 @@ : clearByRepo(rid); } - // Stored data can be anything, so an invalid list reads as empty instead of - // breaking the inbox. - function loadStoredRepos(key: string): string[] { - try { - const parsed = array(string()).safeParse( - JSON.parse(localStorage?.getItem(key) ?? "[]"), - ); - return parsed.success ? parsed.data : []; - } catch { - return []; - } - } - - let pinnedRepos: string[] = $state(loadPinnedRepos()); - let hiddenRepos: string[] = $state(loadHiddenRepos()); - - function loadPinnedRepos(): string[] { - return loadStoredRepos("pinnedInboxRepos"); - } - - function updatePinnedRepos(newRepos: string[]) { - pinnedRepos = newRepos; - localStorage.setItem("pinnedInboxRepos", JSON.stringify(newRepos)); - } + const pinnedRepos = useLocalStorage( + "pinnedInboxRepos", + array(string()), + [], + !window.localStorage, + ); + const hiddenRepos = useLocalStorage( + "hiddenInboxRepos", + array(string()), + [], + !window.localStorage, + ); function togglePin(rid: string) { - const repos = loadPinnedRepos(); - if (repos.includes(rid)) { - updatePinnedRepos(repos.filter(r => r !== rid)); - } else { - updatePinnedRepos([rid, ...repos]); - } - } - - function loadHiddenRepos(): string[] { - return loadStoredRepos("hiddenInboxRepos"); - } - - function updateHiddenRepos(newRepos: string[]) { - hiddenRepos = newRepos; - localStorage.setItem("hiddenInboxRepos", JSON.stringify(newRepos)); + pinnedRepos.value = togglePinned(pinnedRepos.value, rid); } function toggleHide(rid: string) { - const repos = loadHiddenRepos(); - if (repos.includes(rid)) { - updateHiddenRepos(repos.filter(r => r !== rid)); - } else { - updateHiddenRepos([rid, ...repos]); - } + hiddenRepos.value = togglePinned(hiddenRepos.value, rid); } const displayRepos = $derived( - sortedRepos(filteredRepos, pinnedRepos, hiddenRepos), + inboxRepoOrder( + filteredRepos, + pinnedRepos.value, + hiddenRepos.value, + r => r.repo.rid, + r => r.repo.name, + ), ); - function sortedRepos( - allRepos: FilteredRepo[], - pinned: string[], - hidden: string[], - ) { - // Preserve pinning order. - const pinnedRepos = pinned - .map(p => allRepos.find(r => r.repo.rid === p)) - .filter((r): r is FilteredRepo => r !== undefined); - - const sortedRepos = allRepos - .filter(r => !pinned.includes(r.repo.rid) && !hidden.includes(r.repo.rid)) - .sort((a, b) => a.repo.name.localeCompare(b.repo.name)); - const hiddenRepos = allRepos - .filter(r => hidden.includes(r.repo.rid)) - .sort((a, b) => a.repo.name.localeCompare(b.repo.name)); - - return [...pinnedRepos, ...sortedRepos, ...hiddenRepos]; - } - function loadedNotificationCount() { return notificationsByRepo.reduce((acc, repo) => { return acc + repo.count; @@ -275,9 +228,9 @@ excludeGroup={isFiltering ? excludeGroup : undefined} {isFiltering} groupedNotifications={r.groups} - hidden={hiddenRepos.includes(r.repo.rid)} + hidden={hiddenRepos.value.includes(r.repo.rid)} name={r.repo.name} - pinned={pinnedRepos.includes(r.repo.rid)} + pinned={pinnedRepos.value.includes(r.repo.rid)} rid={r.repo.rid} {clearByIds} clearByRepo={effectiveClearByRepo} diff --git a/src/components/SidebarRepoList.svelte b/src/components/SidebarRepoList.svelte index 1d73fb3db..aa7355c14 100644 --- a/src/components/SidebarRepoList.svelte +++ b/src/components/SidebarRepoList.svelte @@ -3,6 +3,7 @@ import { array, boolean, string } from "zod"; + import { pinnedFirst, pinnedIn, togglePinned } from "@app/lib/repoOrdering"; import useLocalStorage from "@app/lib/useLocalStorage.svelte"; let filterOpen = $state(false); @@ -23,10 +24,7 @@ ); function pinnedFrom(repos: RepoSummary[]): RepoSummary[] { - const byRid = new Map(repos.map(r => [r.rid, r])); - return pinnedRepoIds.value - .map(rid => byRid.get(rid)) - .filter((r): r is RepoSummary => r !== undefined); + return pinnedIn(repos, pinnedRepoIds.value, r => r.rid); } // The sidebar's top-to-bottom order: pinned repos first, in the order they @@ -34,9 +32,7 @@ // filter query, which is transient, so the numbering stays put while typing // in it. function orderRepos(repos: RepoSummary[]): RepoSummary[] { - return pinnedFrom(repos).concat( - repos.filter(r => !pinnedRepoIds.value.includes(r.rid)), - ); + return pinnedFirst(repos, pinnedRepoIds.value, r => r.rid); } export function revealRepoInSidebar(rid: string) { @@ -343,11 +339,7 @@ function togglePin(rid: string) { withPinAnimation(() => { - if (pinnedRepoIds.value.includes(rid)) { - pinnedRepoIds.value = pinnedRepoIds.value.filter(r => r !== rid); - } else { - pinnedRepoIds.value = [rid, ...pinnedRepoIds.value]; - } + pinnedRepoIds.value = togglePinned(pinnedRepoIds.value, rid); }); } diff --git a/src/lib/repoOrdering.ts b/src/lib/repoOrdering.ts new file mode 100644 index 000000000..23c32470b --- /dev/null +++ b/src/lib/repoOrdering.ts @@ -0,0 +1,49 @@ +/// Pins `rid` at the front of the list, or unpins it if it is already there. +export function togglePinned(list: string[], rid: string): string[] { + return list.includes(rid) ? list.filter(r => r !== rid) : [rid, ...list]; +} + +/// The pinned items first, in the order they were pinned, then the rest in +/// their original order. +export function pinnedFirst( + items: T[], + pinned: string[], + ridOf: (item: T) => string, +): T[] { + return [ + ...pinnedIn(items, pinned, ridOf), + ...items.filter(item => !pinned.includes(ridOf(item))), + ]; +} + +/// The pinned items that are present, in the order they were pinned. +export function pinnedIn( + items: T[], + pinned: string[], + ridOf: (item: T) => string, +): T[] { + const byRid = new Map(items.map(item => [ridOf(item), item])); + return pinned + .map(rid => byRid.get(rid)) + .filter((item): item is T => item !== undefined); +} + +/// The inbox's order: pinned repos in pin order, then the others by name, then +/// the hidden ones by name. +export function inboxRepoOrder( + items: T[], + pinned: string[], + hidden: string[], + ridOf: (item: T) => string, + nameOf: (item: T) => string, +): T[] { + const byName = (a: T, b: T) => nameOf(a).localeCompare(nameOf(b)); + return [ + ...pinnedIn(items, pinned, ridOf), + ...items + .filter(item => !pinned.includes(ridOf(item))) + .filter(item => !hidden.includes(ridOf(item))) + .sort(byName), + ...items.filter(item => hidden.includes(ridOf(item))).sort(byName), + ]; +} diff --git a/tests/unit/repoOrdering.test.ts b/tests/unit/repoOrdering.test.ts new file mode 100644 index 000000000..d1bb39223 --- /dev/null +++ b/tests/unit/repoOrdering.test.ts @@ -0,0 +1,65 @@ +import { describe, expect, test } from "vitest"; + +import { + inboxRepoOrder, + pinnedFirst, + pinnedIn, + togglePinned, +} from "@app/lib/repoOrdering"; + +type Repo = { rid: string; name: string }; +const repo = (rid: string, name = rid): Repo => ({ rid, name }); +const rids = (repos: Repo[]) => repos.map(r => r.rid); +const ridOf = (r: Repo) => r.rid; + +describe("togglePinned", () => { + test("pins at the front", () => { + expect(togglePinned(["a"], "b")).toEqual(["b", "a"]); + }); + + test("unpins", () => { + expect(togglePinned(["a", "b", "c"], "b")).toEqual(["a", "c"]); + }); + + test("doesn't change the given list", () => { + const list = ["a"]; + togglePinned(list, "b"); + + expect(list).toEqual(["a"]); + }); +}); + +describe("pinned ordering", () => { + const repos = [repo("a"), repo("b"), repo("c"), repo("d")]; + + test("pinnedIn lists present pins in pin order", () => { + expect(rids(pinnedIn(repos, ["c", "gone", "a"], ridOf))).toEqual([ + "c", + "a", + ]); + }); + + test("pinnedFirst puts pins first and keeps the rest in order", () => { + expect(rids(pinnedFirst(repos, ["c", "a"], ridOf))).toEqual([ + "c", + "a", + "b", + "d", + ]); + }); +}); + +test("inboxRepoOrder lists pinned, then the rest by name, then hidden by name", () => { + const repos = [ + repo("1", "zebra"), + repo("3", "mango"), + repo("2", "apple"), + repo("4", "banana"), + repo("6", "yak"), + repo("5", "cherry"), + ]; + + expect( + rids(inboxRepoOrder(repos, ["3"], ["6", "5"], ridOf, r => r.name)), + ).toEqual(["3", "2", "4", "1", "5", "6"]); +});