Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
81 changes: 81 additions & 0 deletions .fork/customizations.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -271,6 +271,18 @@
Working is emerald, deliberately diverging from the sky the mobile Live
Activity still uses; mobile has not been migrated.

A card is three lines only when it has a PR or a diff for the third, or does
not yet know whether it has a PR; with a known-empty third line the model and
runtime move up beside the branch and the card closes at the design's
two-line 64px. The unknown case matters as much as the empty one: whether a
thread has a PR is the answer to a per-row VCS query, and collapsing before
it lands would grow every PR card 15px mid-scroll as the queries resolve.
Nothing else varies the height: an absent branch or model leaves its half of
a row empty rather than closing the row up. The li's contain-intrinsic-size
is chosen by the same predicate the component renders from, so it can never
claim a height the row does not have, and it measures the li — the drawn card
plus its own py-0.5, so 90 and 68 rather than 86 and 64.

Diff counts bind to --success-foreground / --destructive-foreground rather
than emerald/red literals, so they hold up in light mode. They render only
once latestTurnDiff() returns data: it is an upstream stub returning null
Expand All @@ -295,6 +307,75 @@
- apps/web/src/__fork_guards__/sidebarV2Rain.test.ts
- apps/web/src/__fork_guards__/sidebarV2CardRows.test.ts

- id: sidebar-v2-project-grouping
intent: >
Sidebar V2's active cards can be grouped under project headers, from a
switch at the top of the fork's project scope row. Upstream's flat list
stays the default, and the preference is device-local (localStorage) rather
than a field in packages/contracts.

Both modes are one list shape. buildActiveThreadSections returns sections
for either, flat being the single headerless section, and the render and the
keyboard-order flattening both read that one sequence. They must never
become two derivations that agree by convention.

orderedThreads is the flattened section order. Three consumers are
positional and address the wrong row if it disagrees with paint order:
resolveAdjacentThreadId (arrow navigation), rangeSelectTo (shift-select) and
planForwardNavigation, which picks where you land after settling or snoozing
the thread you are viewing — under grouping that is the next card in the
grouped order, which may be the next project's first card. The number-key
jump labels are keyed by thread rather than by index, so they cannot
misaddress; they are numbered from this list, so a stale order only shows
them out of sequence down the screen.

Only the active cards group, and only where a header says something the
screen does not already: the snoozed and settled shelves stay flat, and
grouping is skipped whenever one project is on screen, whether by scope or
by the sidebar having a single project. The switch is disabled with a stated
reason in those states rather than accepting a click that does nothing.

Section order is projectGroups' own order and thread order inside a section
is the order it arrives in, so grouping introduces no second notion of
sorting and cannot disagree with the user's project sort order.

A thread whose project resolves to no group collects in a trailing section
whose header carries a null display name. It is never dropped, and never
trails headerless under the previous project. The label for that section is
the render site's, so the bucketing stays free of user-facing copy.

A grouped card hides its project name rather than dropping it: the header
carries it on screen, and the card keeps it for assistive tech, which has no
"two rows up". Grouped mode must never convey less than flat mode.

The header is a heading (role/aria-level) inside a presentational li, not a
collapsible button and not a bare span, and carries a 16px folder mark with
no trailing hairline.

The ref index is memoized on the project list alone; the thread list churns
on the clock, capability descriptors and per-row PR states, none of which
change which project a ref belongs to.

Four Tier-4 fences carry this in SidebarV2.tsx: the import, the row prop,
the section/order memos, and the render hunk. The render hunk is the one
that sits inside upstream's own list body and so the one most likely to be
lost in a sync; the guard test asserts it emits headers from the same
sections it renders threads from, because a silent loss leaves a flat
sidebar over a grouped ordered list.
tier: 4
files:
- apps/web/src/custom/sidebarV2ProjectGrouping.ts
- apps/web/src/custom/SidebarV2ProjectGroupHeader.tsx
# Shared with fork-sidebar-chrome, which owns the two rows themselves and
# their metrics; this entry owns only the grouping switch inside the scope
# row's menu.
- apps/web/src/custom/SidebarV2ChromeRows.tsx
shadows: []
watch:
- apps/web/src/components/SidebarV2.tsx
verify:
- apps/web/src/__fork_guards__/sidebarV2ProjectGrouping.test.ts

- id: fork-surface-palette
intent: >
The dark shell is not black. The workspace stage is #212121 and the sidebar
Expand Down
46 changes: 46 additions & 0 deletions .fork/notes/FORK-CUSTOMIZATION-DECISIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,52 @@ Related deep-dives that predate this file and stay where they are:
- The status-mark vocabulary (form carries the state, hue reinforces it) is taken from the
phanttom Ghostty sidebar design. Working takes emerald from that design rather than the sky the
mobile Live Activity uses; the divergence is deliberate and mobile has not been migrated.
- The two-line card is the design's own variant, not a space saving invented here. The third line
exists to carry the PR badge and the diff counts; with neither it is a blank 15px strip under
every card. Worth knowing while reading it: `latestTurnDiff()` is an upstream stub returning
null today, so until shells carry checkpoint summaries the three-line form only appears on
threads with a PR.

## sidebar-v2-project-grouping

- The first cut modelled the two modes as `groups | null`, with `null` meaning flat, and re-tested
that sentinel at three sites: the ordered-thread memo, the `projectTitle` prop, and the render.
Review called it correctly — three coordinated special cases that have to agree by convention,
in a file already past 2.5k lines. The section model replaces it: flat is one headerless
section, so there is one sequence, the render maps it and the keyboard order flattens it, and
"these two derivations disagree" stops being representable. Worth remembering when the next
variant lands: the fix for a mode flag is usually a shape both modes fit.
- The same first cut collapsed the card on `prBadge !== null`, which is false both for "no PR" and
for "the VCS query has not answered yet". Every PR-carrying card would have rendered at two
lines and grown to three as its query landed, reflowing the list under the pointer — worse than
the blank strip the collapse removes, and it would have got worse still once `latestTurnDiff()`
starts returning data, since that is async too. Hence the explicit `prUnknown` input: collapse
only where the answer is known, or where no query was ever issued.
- Grouping and the project scope filter answer different questions — "everything, arranged by
repo" versus "only this repo" — which is why grouping is a switch inside the scope menu rather
than another entry in its radio list. It is also why grouping is skipped while a scope is set:
the list is already one project, so the header would only repeat the scope row's own label one
line lower.
- The shelves stay flat because both are time-ordered tails whose value is being short. Slicing a
tail by project turns one header into a dozen sub-headers over one or two rows each.
- The preference is device-local because a client setting would mean a fork-shaped key in
`packages/contracts` — a schema upstream owns and every sync has to carry — for a view
preference that is per-window anyway.
- The header is not collapsible. Upstream's two shelf headers collapse because each hides a tail;
a project header sits over the inbox, where nothing wants hiding, and the scope menu one row
above already does what collapsing would. It drops the shelf headers' trailing hairline for the
same reason: a shelf rule is one divider closing off the list above it, while project headers
recur every few rows and a rule at that cadence stripes the panel against the card edges. The
folder mark and the space above it carry the separation instead, and the mark is the scope
menu's own so a header and its menu entry read as the same object.
- Grouped cards hide their project name rather than dropping it. Dropping it was the first
implementation and it made grouped mode strictly worse than flat mode for a screen reader: the
header is a visual adjacency, so a non-visual user lost the project association entirely. The
header is now a heading in a presentational `li` and the name stays on the card as `sr-only`.
- Post-settle landing changes under grouping, and it is intended. `planForwardNavigation` reads
the ordered list positionally, so settling the last active card in one project now lands on the
next project's first card rather than on the next most-recently-active thread. "Forward" should
mean the next visible row; that is what it now means in both modes.

## fork-sidebar-chrome

Expand Down
38 changes: 34 additions & 4 deletions apps/web/src/__fork_guards__/sidebarV2CardRows.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import { describe, expect, it } from "vite-plus/test";

import { SidebarV2IdleMark } from "../custom/SidebarV2StatusIndicator";
import { threadCardTitleRecedes } from "../custom/sidebarV2RowPolicy";
import { SidebarV2ThreadCardMeta } from "../custom/SidebarV2ThreadCardMeta";
import { SidebarV2ThreadCardMeta, threadCardShowsMetaRow } from "../custom/SidebarV2ThreadCardMeta";

function readSibling(relativePath: string): string {
return NodeFS.readFileSync(NodeURL.fileURLToPath(new URL(relativePath, import.meta.url)), "utf8");
Expand Down Expand Up @@ -92,11 +92,41 @@ describe("fork guard: sidebar-v2-card-rows", () => {
expect(sidebarV2).toContain("<SidebarV2IdleMark />");
});

it("reserves the card's drawn height for offscreen rows", () => {
it("collapses to two lines only when it knows there is no PR and no diff", () => {
// The third line exists to carry the PR badge and the diff counts. With
// neither, drawing it leaves a blank 15px strip under every card.
const show = threadCardShowsMetaRow;
const known = { hasPr: false, prUnknown: false, insertions: null, deletions: null };
expect(show(known)).toBe(false);
expect(show({ ...known, hasPr: true })).toBe(true);
expect(show({ ...known, insertions: 3 })).toBe(true);
expect(show({ ...known, deletions: 3 })).toBe(true);
// Zero is a real count — "+0 −0" is a turn that touched nothing, not a
// thread with no diff at all.
expect(show({ ...known, insertions: 0, deletions: 0 })).toBe(true);
// The one that is not about content: whether a thread has a PR is the
// answer to a per-row VCS query, and collapsing before it lands makes every
// PR card grow 15px mid-scroll as the queries resolve.
expect(show({ ...known, prUnknown: true })).toBe(true);
// Unknown means "has never answered", not "is polling": the query re-enters
// waiting on every refresh, and reading that alone flips the height on a
// loop rather than once.
expect(sidebarV2).toContain(
"const prUnknown = gitStatus.data === null && gitStatus.isPending;",
);
});

it("reserves each card's drawn height for offscreen rows", () => {
// content-visibility skips offscreen rows; the intrinsic size is what keeps
// the scrollbar honest while they are skipped. A stale value here makes the
// list jump as you scroll — three lines are 86px, not the old two-line 76.
expect(sidebarV2).toContain("[contain-intrinsic-size:auto_86px]");
// list jump as you scroll, so both heights are pinned. They measure the li,
// which is the drawn card plus its own py-0.5: three lines are 86 + 4, two
// are 64 + 4.
expect(sidebarV2).toContain("[contain-intrinsic-size:auto_90px]");
expect(sidebarV2).toContain("[contain-intrinsic-size:auto_68px]");
// And the choice is made from the same predicate the component renders
// from, so the hint cannot drift from the row count it describes.
expect(sidebarV2).toContain("threadCardShowsMetaRow({");
});

it("binds diff counts to semantic tokens rather than palette literals", () => {
Expand Down
106 changes: 106 additions & 0 deletions apps/web/src/__fork_guards__/sidebarV2ProjectGrouping.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
// @effect-diagnostics nodeBuiltinImport:off
/**
* Fork guard — see `.fork/README.md` §4b and
* `.fork/customizations.yaml#sidebar-v2-project-grouping`.
*
* The bucketing itself is behaviour, and it is tested as behaviour next to the
* module it lives in (`custom/sidebarV2ProjectGrouping.test.ts`). This file
* guards only the seam inside upstream's `SidebarV2.tsx`, which nothing else
* can observe without standing up the whole sidebar: that the switch is wired,
* that the rendered sequence and the keyboard-order sequence are the same one,
* and that grouping stays off the two shelves.
*
* The seam that matters most is the second. `orderedActiveThreads` backs arrow
* navigation, shift-range selection and post-settle landing, all positional, so
* a list that disagrees with the paint order addresses the wrong row — and
* nothing about the render *looks* wrong when it does. Both now derive from one
* `activeSections`, which is what makes the divergence unrepresentable rather
* than merely unlikely; these assertions catch a merge that splits them apart
* again.
*/

import * as NodeFS from "node:fs";
import * as NodeURL from "node:url";
import { describe, expect, it } from "vite-plus/test";

function readSibling(relativePath: string): string {
return NodeFS.readFileSync(NodeURL.fileURLToPath(new URL(relativePath, import.meta.url)), "utf8");
}

const sidebar = readSibling("../components/SidebarV2.tsx");
const chromeRows = readSibling("../custom/SidebarV2ChromeRows.tsx");

describe("fork guard: sidebar-v2-project-grouping", () => {
it("offers the switch from the project scope menu", () => {
expect(chromeRows).toContain("Group by project");
expect(chromeRows).toContain("onGroupByProjectChange");
// Toggling a view preference must not dismiss the menu it lives in.
expect(chromeRows).toContain("closeOnClick={false}");
expect(sidebar).toContain("groupByProject={groupByProject}");
// Where grouping can draw no header, the switch says so rather than
// accepting a click that does nothing.
expect(chromeRows).toContain("disabled={props.groupByProjectUnavailableReason !== null}");
expect(sidebar).toContain("groupByProjectUnavailableReason=");
});

it("renders and orders from one sequence", () => {
expect(sidebar).toContain("const activeSections = useMemo(");
// Keyboard order is the flattened render sequence, not a parallel
// derivation that has to agree with it by convention.
expect(sidebar).toContain("activeSections.flatMap((section) => section.threads)");
expect(sidebar).toContain("activeSections.flatMap((section, sectionIndex)");
expect(sidebar).toContain("[...orderedActiveThreads, ...visibleSnoozedThreads");
const definition = sidebar.indexOf("const orderedActiveThreads");
const use = sidebar.indexOf("[...orderedActiveThreads,");
expect(definition).toBeGreaterThanOrEqual(0);
expect(use).toBeGreaterThan(definition);
});

it("draws a header for every section that has one, in paint order", () => {
// The other half of the single-sequence claim: the render must emit the
// header from the same section whose threads follow it. Dropping this hunk
// is the likeliest outcome of a merge that rewrites upstream's list body,
// and it would leave a flat-looking sidebar over a grouped ordered list.
const start = sidebar.indexOf("const items: ReactNode[] = activeSections.flatMap(");
expect(start).toBeGreaterThanOrEqual(0);
const render = sidebar.slice(start, sidebar.indexOf("};", start));
expect(render).toContain("<SidebarV2ProjectGroupHeader");
expect(render).toContain("section.header");
expect(render).toContain('renderThreadRow(thread, "active", section.header !== null)');
});

it("groups the active cards only, and only where a header would say something new", () => {
expect(sidebar).toContain(
"groupByProject && projectScopeKey === null && projectGroups.length > 1",
);
// The shelves keep their flat, time-ordered rendering.
expect(sidebar).toContain("for (const thread of renderedSettledThreads)");
expect(sidebar).toContain("for (const thread of visibleSnoozedThreads)");
});

it("keeps the project on grouped cards for assistive tech", () => {
// Grouped cards stop drawing the project name because the header carries
// it — but a screen reader has no "two rows up", so hiding it visually is
// the whole of the change. Dropping the prop entirely would make grouped
// mode carry strictly less than flat mode.
expect(sidebar).toContain("projectTitleHidden={underProjectHeader}");
const meta = readSibling("../custom/SidebarV2ThreadCardMeta.tsx");
expect(meta).toContain("sr-only");
});

it("gives the header heading semantics inside upstream's thread list", () => {
const header = readSibling("../custom/SidebarV2ProjectGroupHeader.tsx");
expect(header).toContain('role="presentation"');
expect(header).toContain('role="heading"');
expect(header).toContain("aria-level={3}");
});

it("rebuilds the project index only when the project list changes", () => {
// The thread list churns on the clock, capability descriptors and PR states
// arriving per row; indexing every member ref on each of those is work per
// project for an answer that has not changed.
expect(sidebar).toContain(
"const projectRefIndex = useMemo(() => createProjectRefIndex(projectGroups), [projectGroups]);",
);
});
});
Loading
Loading