diff --git a/.oxlintrc.json b/.oxlintrc.json index 69477d0..c8020d1 100644 --- a/.oxlintrc.json +++ b/.oxlintrc.json @@ -11,7 +11,10 @@ "import/no-unassigned-import": "off", // A component's function is mostly markup, so a line limit meant for logic // just asks for components to be split where they do not want to be. - "eslint/max-lines-per-function": "off" + "eslint/max-lines-per-function": "off", + // App.tsx is the composition root: importing every part of the app is what + // it is for, and splitting it to satisfy a count would only hide that. + "import/max-dependencies": "off" }, "settings": { "jsx-a11y": { diff --git a/src/App.tsx b/src/App.tsx index c8df58c..0790afd 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -9,6 +9,7 @@ import { openRepository } from "./lib/api"; import { isGigitError, type OpenedRepository } from "./lib/bindings"; import { createGraphView } from "./lib/graph"; import { createRecents } from "./lib/recents"; +import { createRefs } from "./lib/refs"; /** What to call a repository: its working tree's folder, or the git directory. */ function nameOf(repository: OpenedRepository): string { @@ -17,6 +18,13 @@ function nameOf(repository: OpenedRepository): string { return path.replace(/\/+$/u, "").split("/").pop() ?? path; } +/** The branch that is checked out, if HEAD is on one at all. */ +function headBranch(repository: OpenedRepository | undefined): string | undefined { + const head = repository?.summary.head; + + return head?.state === "branch" ? head.shorthand : undefined; +} + /** How the current HEAD reads in the title bar. */ function headOf(repository: OpenedRepository): string { const head = repository.summary.head; @@ -37,6 +45,7 @@ export default function App() { const [error, setError] = createSignal(); const graph = createGraphView(); const recents = createRecents(); + const refs = createRefs(); onSettled(() => void recents.refresh()); @@ -50,9 +59,13 @@ export default function App() { // Opening promotes the repository to the front of the list, so the // welcome pane is right when it is next seen. void recents.refresh(); + // The sidebar and the graph are independent, so the refs do not have to + // land before the first commits can be drawn. + void refs.show(opened.id); await graph.show(opened.id); } catch (failure) { setRepository(undefined); + refs.clear(); setError(isGigitError(failure) ? failure.message : String(failure)); } finally { setOpening(false); @@ -91,7 +104,11 @@ export default function App() { - + {/* tabindex="-1" so the skip button actually moves focus here */}
@@ -105,7 +122,7 @@ export default function App() {
diff --git a/src/components/RepositoryNav.tsx b/src/components/RepositoryNav.tsx index 2989269..2907cdf 100644 --- a/src/components/RepositoryNav.tsx +++ b/src/components/RepositoryNav.tsx @@ -1,23 +1,82 @@ -const SECTIONS = ["Branches", "Remotes", "Tags", "Stashes"] as const; +import { For, Show } from "solid-js"; + +import type { RefEntry } from "../lib/bindings"; +import type { Refs } from "../lib/refs"; /** - * Sidebar listing the repository's refs. The sections are empty until a - * repository is open; each one becomes a list of refs in a later change. + * The repository's references. + * + * Entries are text rather than buttons: nothing happens when you click a + * branch yet, and a control that looks interactive but is not is worse than + * plain text. They become controls when checking out exists. */ -export default function RepositoryNav() { +export default function RepositoryNav(props: { + refs: Refs; + /** The checked-out branch, so it can be marked among the others. */ + head?: string; + /** Whether a repository is open at all. */ + isOpen: boolean; +}) { return ( ); } + +function Section(props: { title: string; entries: RefEntry[]; head?: string; isOpen: boolean }) { + const headingId = () => `refs-${props.title.toLowerCase()}`; + + return ( +
  • +

    + {props.title} + 0}> + {/* A count is worth having when a repository has hundreds of remote + branches and the list below is mostly off screen. */} + + {props.entries.length} + + +

    + + 0} + fallback={ +

    {props.isOpen ? "None" : "—"}

    + } + > +
      + + {(entry) => ( +
    • + + current branch: + + {entry.shorthand} +
      • + )} +       +
    +     +
    • + ); +} diff --git a/src/lib/refs.ts b/src/lib/refs.ts new file mode 100644 index 0000000..74ab19e --- /dev/null +++ b/src/lib/refs.ts @@ -0,0 +1,71 @@ +/** + * The repository's references, as the sidebar sees them. + */ + +import { createSignal } from "solid-js"; + +import { listReferences } from "./api"; +import { isGigitError, type RefEntry, type RepositoryId } from "./bindings"; + +/** Refs split into the groups the sidebar shows. */ +export type GroupedRefs = { + branches: RefEntry[]; + remotes: RefEntry[]; + tags: RefEntry[]; +}; + +/** + * Sorts names the way a person reads them, so `v9` comes before `v10` rather + * than after it. + */ +const BY_NAME = new Intl.Collator(undefined, { numeric: true, sensitivity: "base" }); + +/** + * Split refs by kind and sort each group. + * + * `other` refs — notes, `refs/stash`, replacements — are deliberately dropped. + * They are machinery rather than things a person navigates by, and showing + * them would bury the branches. + */ +export function groupRefs(entries: RefEntry[]): GroupedRefs { + const sorted = (kind: RefEntry["kind"]) => + entries + .filter((entry) => entry.kind === kind) + // Sorting in place is safe here: filter already returned a fresh array, + // so there is nothing of the caller's to mutate. + // oxlint-disable-next-line unicorn/no-array-sort + .sort((left, right) => BY_NAME.compare(left.shorthand, right.shorthand)); + + return { + branches: sorted("localBranch"), + remotes: sorted("remoteBranch"), + tags: sorted("tag"), + }; +} + +export function createRefs() { + const [entries, setEntries] = createSignal([]); + const [message, setMessage] = createSignal(); + + /** Load a repository's refs, replacing whatever was shown before. */ + async function show(id: RepositoryId) { + setMessage(undefined); + + try { + setEntries(await listReferences(id)); + } catch (error) { + // The sidebar failing is not a reason to lose the history beside it. + setEntries([]); + setMessage(isGigitError(error) ? error.message : String(error)); + } + } + + function clear() { + setEntries([]); + setMessage(undefined); + } + + return { entries, grouped: () => groupRefs(entries()), message, show, clear }; +} + +export type Refs = ReturnType; diff --git a/tests/e2e/sidebar.spec.ts b/tests/e2e/sidebar.spec.ts new file mode 100644 index 0000000..34b3ad4 --- /dev/null +++ b/tests/e2e/sidebar.spec.ts @@ -0,0 +1,128 @@ +import { expect, test } from "@playwright/test"; + +import { linearHistory, reference } from "../fixtures/rows"; +import { mockTauri, openRepository, sidebarSection } from "./tauri"; + +const REFERENCES = [ + reference("main"), + reference("feature/graph"), + reference("origin/main", "remoteBranch"), + reference("origin/feature/graph", "remoteBranch"), + reference("v1.0.0", "tag"), + reference("v2.0.0", "tag"), + // Machinery, which the sidebar should leave out. + reference("stash", "other"), +]; + +test.describe("sidebar", () => { + test("says nothing definite before a repository is open", async ({ page }) => { + await mockTauri(page, { rows: [], references: REFERENCES, chooseFolder: "/demo" }); + await page.goto("/"); + + const sidebar = page.getByRole("navigation", { name: "Repository" }); + + await expect(sidebar).toContainText("Branches"); + // "None" would be a claim about a repository nothing has read yet. + await expect(sidebar).not.toContainText("None"); + }); + + test("fills in once a repository is open", async ({ page }) => { + await mockTauri(page, { + rows: linearHistory(10), + references: REFERENCES, + chooseFolder: "/demo", + }); + await page.goto("/"); + + await openRepository(page); + + // The mocked repository reports HEAD on `main`, which carries its marker. + await expect(sidebarSection(page, "Branches")).toHaveText([ + "feature/graph", + "current branch: main", + ]); + await expect(sidebarSection(page, "Remotes")).toHaveText([ + "origin/feature/graph", + "origin/main", + ]); + await expect(sidebarSection(page, "Tags")).toHaveText(["v1.0.0", "v2.0.0"]); + }); + + test("leaves out refs that are machinery rather than navigation", async ({ page }) => { + await mockTauri(page, { + rows: linearHistory(10), + references: REFERENCES, + chooseFolder: "/demo", + }); + await page.goto("/"); + + await openRepository(page); + await expect(sidebarSection(page, "Branches").first()).toBeVisible(); + + await expect(page.getByRole("navigation", { name: "Repository" })).not.toContainText("stash"); + }); + + test("counts each section", async ({ page }) => { + await mockTauri(page, { + rows: linearHistory(10), + references: REFERENCES, + chooseFolder: "/demo", + }); + await page.goto("/"); + + await openRepository(page); + + await expect(page.getByRole("heading", { name: "Branches 2" })).toBeVisible(); + await expect(page.getByRole("heading", { name: "Remotes 2" })).toBeVisible(); + }); + + test("marks the checked out branch in words", async ({ page }) => { + // Colour alone says nothing to a screen reader. + await mockTauri(page, { + rows: linearHistory(10), + references: REFERENCES, + chooseFolder: "/demo", + }); + await page.goto("/"); + + await openRepository(page); + + // The mocked repository reports HEAD on `main`. + await expect(sidebarSection(page, "Branches").filter({ hasText: "current branch" })).toHaveText( + ["current branch: main"], + ); + }); + + test("says a section is empty when the repository really has nothing in it", async ({ page }) => { + await mockTauri(page, { + rows: linearHistory(10), + references: [reference("main")], + chooseFolder: "/demo", + }); + await page.goto("/"); + + await openRepository(page); + await expect(sidebarSection(page, "Branches")).toHaveText(["current branch: main"]); + + await expect(page.getByRole("navigation", { name: "Repository" })).toContainText("None"); + }); + + test("copes with a repository that has hundreds of refs", async ({ page }) => { + // A busy repository should fill the sidebar rather than break it. + const many = Array.from({ length: 400 }, (_, index) => + reference(`origin/branch-${index}`, "remoteBranch"), + ); + + await mockTauri(page, { + rows: linearHistory(10), + references: [reference("main"), ...many], + chooseFolder: "/demo", + }); + await page.goto("/"); + + await openRepository(page); + + await expect(page.getByRole("heading", { name: "Remotes 400" })).toBeVisible(); + await expect(sidebarSection(page, "Remotes")).toHaveCount(400); + }); +}); diff --git a/tests/e2e/tauri.ts b/tests/e2e/tauri.ts index 6cec81c..d09a186 100644 --- a/tests/e2e/tauri.ts +++ b/tests/e2e/tauri.ts @@ -14,7 +14,7 @@ import type { Page } from "@playwright/test"; -import type { GraphRow, RecentRepository } from "../../src/lib/bindings"; +import type { GraphRow, RecentRepository, RefEntry } from "../../src/lib/bindings"; export type MockOptions = { /** Every row the fake backend is willing to hand over. */ @@ -23,6 +23,8 @@ export type MockOptions = { failToOpen?: string; /** What the recents list starts out holding. */ recents?: RecentRepository[]; + /** The repository's references, for the sidebar. */ + references?: RefEntry[]; /** * What the folder dialog returns. `null` stands for the user dismissing it, * which the app must treat as an ordinary outcome rather than a failure. @@ -103,6 +105,9 @@ export async function mockTauri(page: Page, options: MockOptions): Promise case "plugin:dialog|open": { return serialised.chooseFolder ?? null; } + case "list_references": { + return serialised.references ?? []; + } case "list_recent_repositories": { return recents; } @@ -153,3 +158,15 @@ export function commitRows(page: Page) { export function recentRows(page: Page) { return page.getByRole("list", { name: "Recent" }).getByRole("listitem"); } + +/** + * The entries in one sidebar section. + * + * Addressed by the list's own name, because the history is a list too. + */ +export function sidebarSection(page: Page, title: "Branches" | "Remotes" | "Tags") { + return page + .getByRole("navigation", { name: "Repository" }) + .getByRole("list", { name: new RegExp(`^${title}`, "u") }) + .getByRole("listitem"); +} diff --git a/tests/fixtures/rows.ts b/tests/fixtures/rows.ts index 38ac3b2..8610be7 100644 --- a/tests/fixtures/rows.ts +++ b/tests/fixtures/rows.ts @@ -110,3 +110,23 @@ export function recent( ...partial, }; } + +/** A reference, as `list_references` returns one. */ +export function reference( + shorthand: string, + kind: import("../../src/lib/bindings").RefKind = "localBranch", +): import("../../src/lib/bindings").RefEntry { + const namespace = { + localBranch: "refs/heads/", + remoteBranch: "refs/remotes/", + tag: "refs/tags/", + other: "refs/", + }[kind]; + + return { + name: `${namespace}${shorthand}`, + shorthand, + kind, + target: "0".repeat(40), + }; +} diff --git a/tests/unit/refs.test.ts b/tests/unit/refs.test.ts new file mode 100644 index 0000000..1c8ab72 --- /dev/null +++ b/tests/unit/refs.test.ts @@ -0,0 +1,136 @@ +import { flush } from "solid-js"; +import { beforeEach, describe, expect, test, vi } from "vitest"; + +import { createRefs, groupRefs } from "../../src/lib/refs"; +import { reference } from "../fixtures/rows"; + +const listReferences = vi.fn(async () => [reference("main"), reference("v1.0.0", "tag")]); + +vi.mock("../../src/lib/api", () => ({ + listReferences: (...args: unknown[]) => listReferences(...(args as [])), +})); + +beforeEach(() => { + listReferences.mockClear(); + listReferences.mockImplementation(async () => [reference("main"), reference("v1.0.0", "tag")]); +}); + +describe("groupRefs", () => { + test("splits refs by what their namespace says they are", () => { + const grouped = groupRefs([ + reference("main"), + reference("origin/main", "remoteBranch"), + reference("v1.0.0", "tag"), + ]); + + expect(grouped.branches.map((entry) => entry.shorthand)).toEqual(["main"]); + expect(grouped.remotes.map((entry) => entry.shorthand)).toEqual(["origin/main"]); + expect(grouped.tags.map((entry) => entry.shorthand)).toEqual(["v1.0.0"]); + }); + + test("drops refs that are machinery rather than navigation", () => { + // Notes, refs/stash and replacements would bury the branches. + const grouped = groupRefs([ + reference("main"), + reference("stash", "other"), + reference("notes/commits", "other"), + ]); + + expect(grouped.branches).toHaveLength(1); + expect(grouped.remotes).toHaveLength(0); + expect(grouped.tags).toHaveLength(0); + }); + + test("sorts names the way a person reads them", () => { + // Sorting as plain strings would put v10 before v9. + const grouped = groupRefs([ + reference("v9.0.0", "tag"), + reference("v10.0.0", "tag"), + reference("v2.0.0", "tag"), + ]); + + expect(grouped.tags.map((entry) => entry.shorthand)).toEqual(["v2.0.0", "v9.0.0", "v10.0.0"]); + }); + + test("sorts branches regardless of the order they arrived in", () => { + const grouped = groupRefs([reference("zebra"), reference("alpha"), reference("feature/thing")]); + + expect(grouped.branches.map((entry) => entry.shorthand)).toEqual([ + "alpha", + "feature/thing", + "zebra", + ]); + }); + + test("does not mutate what it was given", () => { + const entries = [reference("zebra"), reference("alpha")]; + + groupRefs(entries); + + expect(entries.map((entry) => entry.shorthand)).toEqual(["zebra", "alpha"]); + }); + + test("copes with a repository that has no refs at all", () => { + const grouped = groupRefs([]); + + expect(grouped).toEqual({ branches: [], remotes: [], tags: [] }); + }); +}); + +describe("createRefs", () => { + test("starts empty and asks for nothing", () => { + const refs = createRefs(); + + expect(refs.entries()).toEqual([]); + expect(listReferences).not.toHaveBeenCalled(); + }); + + test("loads a repository's refs", async () => { + const refs = createRefs(); + + await refs.show("/demo/.git"); + await flush(); + + expect(listReferences).toHaveBeenCalledWith("/demo/.git"); + expect(refs.grouped().branches).toHaveLength(1); + expect(refs.grouped().tags).toHaveLength(1); + }); + + test("replaces what was shown when another repository opens", async () => { + const refs = createRefs(); + await refs.show("/first/.git"); + await flush(); + + listReferences.mockResolvedValueOnce([reference("develop")]); + await refs.show("/second/.git"); + await flush(); + + expect(refs.grouped().branches.map((entry) => entry.shorthand)).toEqual(["develop"]); + expect(refs.grouped().tags).toHaveLength(0); + }); + + test("empties itself when asked to clear", async () => { + const refs = createRefs(); + await refs.show("/demo/.git"); + await flush(); + + refs.clear(); + await flush(); + + expect(refs.entries()).toEqual([]); + }); + + test("reports a failure without leaving stale refs on screen", async () => { + // Showing another repository's branches would be worse than showing none. + const refs = createRefs(); + await refs.show("/first/.git"); + await flush(); + + listReferences.mockRejectedValueOnce({ kind: "git", message: "could not list references" }); + await refs.show("/second/.git"); + await flush(); + + expect(refs.entries()).toEqual([]); + expect(refs.message()).toBe("could not list references"); + }); +}); diff --git a/tests/unit/repository-nav.test.tsx b/tests/unit/repository-nav.test.tsx new file mode 100644 index 0000000..0058847 --- /dev/null +++ b/tests/unit/repository-nav.test.tsx @@ -0,0 +1,126 @@ +import { render } from "@solidjs/testing-library"; +import { createSignal, flush } from "solid-js"; +import { describe, expect, test, vi } from "vitest"; + +import RepositoryNav from "../../src/components/RepositoryNav"; +import type { RefEntry } from "../../src/lib/bindings"; +import { groupRefs, type Refs } from "../../src/lib/refs"; +import { reference } from "../fixtures/rows"; + +/** A stand-in for the store, so these tests never touch IPC. */ +function store(entries: RefEntry[]): Refs { + const [current] = createSignal(entries); + + return { + entries: current, + grouped: () => groupRefs(current()), + message: () => undefined, + show: vi.fn(), + clear: vi.fn(), + } as unknown as Refs; +} + +/** The entries under a given heading. */ +function section(container: HTMLElement, title: string): string[] { + const list = container.querySelector(`ul[aria-labelledby="refs-${title}"]`); + + return [...(list?.querySelectorAll("li") ?? [])].map((row) => row.textContent?.trim() ?? ""); +} + +const REFS = [ + reference("main"), + reference("feature/graph"), + reference("origin/main", "remoteBranch"), + reference("v1.0.0", "tag"), + reference("v2.0.0", "tag"), +]; + +describe("RepositoryNav", () => { + test("is a labelled landmark, so it can be jumped to", async () => { + const { getByRole } = render(() => ); + await flush(); + + expect(getByRole("navigation", { name: "Repository" })).toBeTruthy(); + }); + + test("lists branches, remotes and tags under their own headings", async () => { + const { container } = render(() => ); + await flush(); + + expect(section(container, "branches")).toEqual(["feature/graph", "main"]); + expect(section(container, "remotes")).toEqual(["origin/main"]); + expect(section(container, "tags")).toEqual(["v1.0.0", "v2.0.0"]); + }); + + test("counts what is in each section", async () => { + // Worth having when a repository has hundreds of remote branches. + const { getByRole } = render(() => ); + await flush(); + + expect(getByRole("heading", { name: "Branches 2" })).toBeTruthy(); + expect(getByRole("heading", { name: "Tags 2" })).toBeTruthy(); + }); + + test("shows no count for a section with nothing in it", async () => { + const { getByRole } = render(() => ); + await flush(); + + expect(getByRole("heading", { name: "Tags" })).toBeTruthy(); + }); + + test("marks the checked out branch in words, not just colour", async () => { + const { container } = render(() => ); + await flush(); + + const branches = section(container, "branches"); + + expect(branches).toContain("current branch: main"); + expect(branches).toContain("feature/graph"); + }); + + test("marks nothing when HEAD is detached", async () => { + // headBranch gives undefined for a detached or unborn HEAD. + const { container } = render(() => ); + await flush(); + + expect(section(container, "branches").join(" ")).not.toContain("current branch"); + }); + + test("does not confuse a remote branch that ends with the branch name", async () => { + const { container } = render(() => ( + + )); + await flush(); + + expect(section(container, "remotes")).toEqual(["origin/main"]); + }); + + test("says a section is empty when a repository is open", async () => { + const { container } = render(() => ); + await flush(); + + expect(container.textContent).toContain("None"); + }); + + test("says nothing definite before a repository is open", async () => { + // "None" would be a claim about a repository that has not been read. + const { container } = render(() => ); + await flush(); + + expect(container.textContent).not.toContain("None"); + }); + + test("keeps the full ref name available for a truncated entry", async () => { + const { container } = render(() => ( + + )); + await flush(); + + const entry = container.querySelector('ul[aria-labelledby="refs-branches"] li'); + expect(entry?.getAttribute("title")).toBe("refs/heads/feature/a-very-long-branch-name"); + }); +});