From 5e6da9712e9ecc33c1960ef0c5b307e5f7c7ead8 Mon Sep 17 00:00:00 2001 From: Matthew P Munger Date: Fri, 28 Aug 2026 16:16:34 -0500 Subject: [PATCH 1/8] repair: give the Ora note back its dropped space MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `{SETTINGS_SYSTEM_CONTRIBUTES.ora}` was followed by ` Switching it on...` on the same line and the space did not survive, so the note read "...you have to switch on.Switching it on sends...". The recorded cause was wrong, and worth correcting in the ledger because it sends the next session after the wrong thing: JSX does NOT drop a leading space on the first line of a text node — probe routes against this app's own toolchain keep it. What drops it is an HTML entity elsewhere in the same text node, here `Ora's` two lines below. Spelled `Oras` the space survives; as `'` it does not, with or without a newline. Swept the tree for the real pattern — a text node carrying an entity that also begins with a mid-line space beside an expression — and this is the only site. Trailing spaces are unaffected, so `previous {rangeDays} days.` in the page detail is not a second instance. Verified in the rendered DOM rather than the diff: `switch on. Switching`. Co-Authored-By: Claude Opus 5 --- REPAIRS.md | 36 ++++++++++++++++++++++++--------- src/app/(app)/settings/page.tsx | 3 ++- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/REPAIRS.md b/REPAIRS.md index 5a199ca..ea5e416 100644 --- a/REPAIRS.md +++ b/REPAIRS.md @@ -94,20 +94,36 @@ one commit. ### The Ora note runs two sentences together on screen - **Found by:** C3, on `chunk-c3`, while merging `origin/main` (`07834fa`). -- **Not claimed, and deliberately not fixed here.** It is a one-character change - in a line C3 also edits, which is exactly the shape R3 warns about: a shared - defect buried in a feature diff cannot be reviewed or reverted on its own. +- **Left open by C3, deliberately.** It is a one-character change in a line C3 + also edits, which is exactly the shape R3 warns about: a shared defect buried + in a feature diff cannot be reviewed or reverted on its own. +- **Claimed by:** `ui-improvements-post-refactor`, at base `98177fc`. - **Symptom:** in `settings/page.tsx`, `{SETTINGS_SYSTEM_CONTRIBUTES.ora}` is - followed by ` Switching it on sends...` on the same line, and JSX drops that - leading space. The rendered note reads + followed by ` Switching it on sends...` on the same line, and the leading + space is dropped. The rendered note reads "...you have to switch on.Switching it on sends...", with the DOM showing `switch on.Switching`. Introduced by S9 (#93); no check reads rendered copy, so CI is green on it. -- **Scope:** the only `{expr} Text` pair in that file, and the file uses `{" "}` - nowhere, so this is a one-off rather than a pattern. -- **Fix:** `{SETTINGS_SYSTEM_CONTRIBUTES.ora}{" "}` — or move the following word - onto its own line, which is what makes JSX keep the gap. Worth a look at S9's - other screens for the same pair before closing it. +- **Cause — not what the symptom looks like.** "JSX drops a leading space" is + not true, and a session that believes it will go looking for the wrong thing. + JSX keeps the leading space on the first line of a text node; four probe + routes against this app's own toolchain confirmed it. What drops the space is + an **HTML entity elsewhere in the same text node** — here `Ora's`, two + lines further down. Same paragraph with the entity spelled out as `Oras` + keeps its space; with `'` it loses it. It is an SWC behaviour, and it + needs no newline: a single-line `{X} Ora's` loses the space too. +- **Only leading whitespace is affected.** A trailing space before an + expression survives the entity — `previous {rangeDays} days.` in + `pages/[id]/page.tsx` renders correctly and is NOT a defect. Recorded because + it is the first thing a sweep turns up. +- **Scope — swept, one site.** Parsing every `.tsx` for a text node that + carries an entity AND begins with a mid-line space next to an expression + returns exactly this one. So a one-off in fact, though not for the reason + first recorded: the file does use `{" "}`, three lines below the defect, which + is why the retention sentence beside it has always rendered correctly. +- **Fix:** `{SETTINGS_SYSTEM_CONTRIBUTES.ora}{" "}` with the sentence moved to + the next line. Verified in the rendered DOM, not just the diff: + `switch on. Switching`. ## Landed diff --git a/src/app/(app)/settings/page.tsx b/src/app/(app)/settings/page.tsx index ec7462a..8203fe5 100644 --- a/src/app/(app)/settings/page.tsx +++ b/src/app/(app)/settings/page.tsx @@ -572,7 +572,8 @@ function ConnectedSystemsGroup({ disabled }: { disabled: boolean }) {

{EVIDENCE_SOURCE_LABEL.ora}

- {SETTINGS_SYSTEM_CONTRIBUTES.ora} Switching it on sends the live web address of each watched page to + {SETTINGS_SYSTEM_CONTRIBUTES.ora}{" "} + Switching it on sends the live web address of each watched page to Ora, whose scans are public: the result enters Ora's directory and anyone can read it. Webflow staging addresses are never sent.{" "} {/* Draft, pending legal review. Rendered rather than withheld: a From e6d2bb92cf08c9a836b5516b06e06a13c0a719cf Mon Sep 17 00:00:00 2001 From: Matthew P Munger Date: Fri, 28 Aug 2026 16:16:46 -0500 Subject: [PATCH 2/8] feat(case): link each affected page to its own detail MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Affected pages rows named a page and went nowhere, so the reader who wanted to see the page behind a case had to go back out to Pages and find it again. The name is now a link to that page's detail. Only a page the store still knows is linked. `issue.pageIds` is the case's own record of what it covers and a page can leave the watchlist while the case that named it stays, so the id fallback stays plain text — a row that does not navigate beats a row that navigates to a 404. The struck-through reading on an excluded row is unchanged: excluded means not counted, not gone, so those rows link too. Co-Authored-By: Claude Opus 5 --- src/components/case-pages.tsx | 48 ++++++++++++++++++++++++----------- 1 file changed, 33 insertions(+), 15 deletions(-) diff --git a/src/components/case-pages.tsx b/src/components/case-pages.tsx index 23b97d7..d5aaf8d 100644 --- a/src/components/case-pages.tsx +++ b/src/components/case-pages.tsx @@ -1,6 +1,7 @@ "use client"; -import { useState } from "react"; +import { useState, type CSSProperties } from "react"; +import Link from "next/link"; import { applicabilityOf, exclusionReasonOf, @@ -12,6 +13,7 @@ import { applicabilityActionLabel, type ExclusionReason } from "@/lib/vocabulary import { excludedNote, pagesCount } from "@/lib/case-copy"; import { formatImpact } from "@/lib/impact-format"; import { ExclusionReasonPicker } from "@/components/exclusion-reason-picker"; +import { useStore } from "@/components/store"; /** * The pages this case covers, and which of them it counts (4b). @@ -64,6 +66,7 @@ export function CasePages({ onInclude, impactByPage, }: CasePagesProps) { + const { pathFor } = useStore(); const [choosingFor, setChoosingFor] = useState(null); const included = includedPages(issue); const excluded = excludedPageIds(issue); @@ -92,6 +95,28 @@ export function CasePages({ const reason = exclusionReasonOf(issue, pageId); const impact = formatImpact(impactByPage?.[pageId] ?? 0); const path = pagePaths?.[pageId]; + const label = pageTitles[pageId] ?? path ?? pageId; + /** + * Only a page the store still knows gets a link. + * + * `issue.pageIds` is the case's own record of what it covers, and a + * page can leave the watchlist while the case that named it stays. + * Falling back to the raw id and linking it anyway would send the + * reader to a 404 — a row that does not navigate is the better of + * the two failures, so the name still renders, just as text. + */ + const known = pageTitles[pageId] !== undefined || path !== undefined; + const nameStyle: CSSProperties = { + display: "block", + fontSize: 13, + color: isExcluded ? "var(--text-muted)" : "var(--text-body)", + // The reading stays. Struck through says "not counted"; + // removing it would say "never measured", which is a lie. + textDecoration: isExcluded ? "line-through" : "none", + whiteSpace: "nowrap", + overflow: "hidden", + textOverflow: "ellipsis", + }; return (

-
- {pageTitles[pageId] ?? path ?? pageId} -
+ {known ? ( + + {label} + + ) : ( +
{label}
+ )} {isExcluded && reason ? (
{excludedNote(reason)} From c51f7fe07b3444639f886d46899b2ffff048b9a3 Mon Sep 17 00:00:00 2001 From: Matthew P Munger Date: Fri, 28 Aug 2026 16:16:46 -0500 Subject: [PATCH 3/8] feat(header): lay the actions out in a row, and keep metadata with the title MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two changes to `ObjectDetailHeader`, both of them to the stated reading order rather than around it, so the doc-comment moves with the code: actions were stacked, now a row. A pair reads as a pair side by side; stacked, Dismiss looked like a consequence of Accept rather than its alternative. Wraps rather than squeezing, so two actions and a long title on a narrow viewport cannot clip one. metadata was below the whole header, now in the title's own column directly under the explanation. Spanning the header meant the stacked actions pushed it away from the sentence it qualifies. Metadata still sits after the title and the prose, which is the part the order exists to protect — a chip strip above the title asks the reader to classify a problem nobody has described yet. `case-applicability.test.ts` asserts that ordering from the source and still passes. Co-Authored-By: Claude Opus 5 --- src/components/object-detail-header.tsx | 29 ++++++++++++++++--------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/src/components/object-detail-header.tsx b/src/components/object-detail-header.tsx index 8cb2b6c..bef47ff 100644 --- a/src/components/object-detail-header.tsx +++ b/src/components/object-detail-header.tsx @@ -22,12 +22,15 @@ import type { ReactNode } from "react"; * state + date what it is now, and since when * title the object in its own words, at most two lines * explanation one paragraph — why this is here, in prose - * actions stacked right, so they never separate the title from its text - * metadata BELOW everything, because it is reference and not the point + * actions right of the title, in a row, so they never separate the + * title from its text + * metadata directly under the explanation, in the title's own column * - * Metadata sits last on purpose. A strip of chips above the title makes the - * reader parse a taxonomy before they have been told what the problem is, and - * the taxonomy only means anything once they have. + * Metadata sits after the prose on purpose. A strip of chips above the title + * makes the reader parse a taxonomy before they have been told what the + * problem is, and the taxonomy only means anything once they have. It shares + * the title's column rather than spanning the header, so it stays beneath the + * sentence it qualifies instead of being pushed below the actions. */ export interface ObjectDetailHeaderProps { @@ -41,9 +44,11 @@ export interface ObjectDetailHeaderProps { /** One paragraph. If it needs two, one of them belongs in the body. */ explanation?: string; /** - * Stacked at the right. Unlike `PageHeader` this takes a node rather than + * In a row at the right. Unlike `PageHeader` this takes a node rather than * one action, because an object legitimately offers a decision and its * opposite — Accept and Dismiss are a pair, not a primary and a runner-up. + * A pair reads as a pair side by side; stacked, the second looked like a + * consequence of the first. */ actions?: ReactNode; /** Reference detail, rendered below the paragraph. */ @@ -116,14 +121,20 @@ export function ObjectDetailHeader({ {explanation}

) : null} + + {metadata ?
{metadata}
: null}
{actions ? (
) : null}
- - {metadata ?
{metadata}
: null} ); } From 16350163f6b841752b9c35340cea8d8c3d3a69be Mon Sep 17 00:00:00 2001 From: Matthew P Munger Date: Fri, 28 Aug 2026 16:26:07 -0500 Subject: [PATCH 4/8] refactor: one spelling of the row's diagnosis line MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `issue.diagnosis || issue.title` was computed in `issue-row.tsx`, which was fine while the row was its only reader. Sorting by diagnosis needs to order rows by the text they actually show, and a comparator carrying its own copy of the fallback is the drift rule 20 exists to stop — the same move `scopeLineOf` and `formatImpact` already made. Structurally typed rather than taking `IssueCase`, because `issue-case.ts` imports `case-copy.ts` and naming the type here would close a cycle for a two-field read. Co-Authored-By: Claude Opus 5 --- src/components/issue-row.tsx | 9 +++++---- src/lib/case-copy.ts | 20 ++++++++++++++++++++ 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/src/components/issue-row.tsx b/src/components/issue-row.tsx index 3b2f201..0c07164 100644 --- a/src/components/issue-row.tsx +++ b/src/components/issue-row.tsx @@ -3,6 +3,7 @@ import type { CSSProperties } from "react"; import type { IssueCase } from "@/lib/issue-case"; import { EFFORT_LABEL, formatGroupImpact, formatImpact } from "@/lib/impact-format"; import { scopeLineOf } from "@/lib/scope-line"; +import { diagnosisLineOf } from "@/lib/case-copy"; import { CONFIDENCE_LABEL } from "@/lib/vocabulary"; import { caseHref } from "@/lib/paths"; import { StatusChip } from "@/components/status-chip"; @@ -91,10 +92,10 @@ export interface IssueRowProps { export function IssueRow({ issue, basePath, pageTitles, nested = false }: IssueRowProps) { const impact = formatImpact(issue.impactMs); - // The case's own plain sentence where it has one. `fromRec` leaves this empty - // rather than authoring copy, and the stored title is what the source called - // it — the honest fallback, not a second diagnosis. - const diagnosis = issue.diagnosis || issue.title; + // The case's own plain sentence where it has one, falling back to the stored + // title. `case-copy` owns the choice, because the diagnosis sort orders rows + // by the same text. + const diagnosis = diagnosisLineOf(issue); return ( Date: Fri, 28 Aug 2026 16:26:07 -0500 Subject: [PATCH 5/8] feat(issues): make every column a sort, and rename Scope to Pages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The header row was six unfocusable spans above a list that could only be reordered from a menu holding four options, two of which headed no column. Every column now sorts, and the header IS the control. Four sorts added, each with a direction chosen for a reason rather than inherited from the type: state down the registry's lifecycle. Derived by indexing WORK_STATES, not by copying the order into a constant — alphabetically this is dismissed, in_progress, new, which is a triage list upside down, and R1's F10 is what copying a stated rule into a second constant costs. confidence strongest first, indexed off CONFIDENCES the same way. pages broadest first. One fix covering six pages is the case this sort exists to surface. diagnosis alphabetical, on the sentence the row shows — including the title fallback, so the order is one a reader could work out from the screen. Labels now come from ISSUE_SORT_LABEL, so a column and the Sort menu read one map and cannot end up calling the same ordering two things. That is also where the Scope column becomes Pages. Caveat on that rename, left as-is deliberately: the cell renders `scopeLineOf`, which is pages AND devices — "Hosting · Mobile, Desktop" — so the header now under-describes its column. `pageScopeOf` already exists and returns pages only; switching the cell to it would make the header exact at the cost of dropping the device clause from the list, which is a product call rather than a rename. No direction toggle. Each sort has one canonical direction with a reason attached, and reversing them produces orders nobody asked for, so no arrow is drawn — an arrow promises a second click that does something. The menu stays: Newest and What changed head no column, because there is no date column to head. Co-Authored-By: Claude Opus 5 --- src/app/(app)/issues/page.tsx | 68 +++++++++++++++++++++++---- src/components/store.tsx | 55 +++++++++++++++++++++- src/lib/__tests__/issues-list.test.ts | 49 +++++++++++++++++++ 3 files changed, 161 insertions(+), 11 deletions(-) diff --git a/src/app/(app)/issues/page.tsx b/src/app/(app)/issues/page.tsx index 2651b38..0da9358 100644 --- a/src/app/(app)/issues/page.tsx +++ b/src/app/(app)/issues/page.tsx @@ -2,6 +2,7 @@ import { useMemo, useState } from "react"; import { useRouter, useSearchParams } from "next/navigation"; +import Link from "next/link"; import { DEFAULT_ISSUE_SORT, ISSUE_SORTS, @@ -54,10 +55,35 @@ import { WATCH_EMPTY } from "@/lib/watch-copy"; * another, and neither is a preference worth storing. */ -/** Column headers, drawn on the same six tracks as the rows below them. */ -const COLUMN_HEADERS = ["State", "Diagnosis", "Scope", "Confidence", "Impact", "Effort"] as const; +/** + * Column headers, drawn on the same six tracks as the rows below them — and the + * sort control for the column each one heads. + * + * The header IS the sort rather than a caption above one. Two consequences worth + * stating, because both were decisions: + * + * - The label comes from `ISSUE_SORT_LABEL`, so a column and the Sort menu + * read one map and cannot end up calling the same ordering two things. + * - There is no direction to toggle. Each sort has one canonical direction + * with a reason attached — least effort first surfaces what can be cleared + * today, broadest first surfaces the fix that covers six pages — and + * reversing them produces orders nobody asked for ("hardest first"). So no + * arrow is drawn, because an arrow would promise a second click that does + * something. + * + * Newest and What changed head no column: there is no date column to head. The + * menu is what keeps them reachable, which is why it stays. + */ +const COLUMN_SORTS = [ + "state", + "diagnosis", + "pages", + "confidence", + "impact", + "effort", +] as const satisfies readonly IssueSort[]; -function ColumnHeaders() { +function ColumnHeaders({ sort, hrefFor }: { sort: IssueSort; hrefFor: (next: IssueSort) => string }) { return (
- {COLUMN_HEADERS.map((label, index) => ( - = 4 ? NUMERIC_CELL : TRUNCATE_CELL}> - {label} - - ))} + {COLUMN_SORTS.map((key, index) => { + const active = key === sort; + const numeric = index >= 4; + return ( + + {ISSUE_SORT_LABEL[key]} + + ); + })}
); } @@ -204,7 +254,7 @@ export default function IssuesPage() { />
- + linkTo({ sort: next })} />
{view.groups.map((group) => ( diff --git a/src/components/store.tsx b/src/components/store.tsx index e48f147..02832ad 100644 --- a/src/components/store.tsx +++ b/src/components/store.tsx @@ -23,7 +23,18 @@ import { import { issueCasesFrom, lastRunAtOf } from "@/lib/issue-cases"; import type { CaseDecision, CaseDecisionRequest } from "@/lib/case-decisions"; import { partitionByImpact } from "@/lib/impact-format"; -import { APPLICABILITY_LABEL, COUNTED_QUEUES, ISSUE_ACTION_LABEL, type ExclusionReason, type Queue } from "@/lib/vocabulary"; +import { + APPLICABILITY_LABEL, + CONFIDENCES, + COUNTED_QUEUES, + ISSUE_ACTION_LABEL, + WORK_STATES, + type Confidence, + type ExclusionReason, + type Queue, + type WorkState, +} from "@/lib/vocabulary"; +import { diagnosisLineOf } from "@/lib/case-copy"; import { normalizeNativeElementControls } from "@/lib/nativeElements"; import { localISODate } from "@/lib/ui"; import { withBasePath } from "@/lib/paths"; @@ -1250,7 +1261,16 @@ export { partitionByImpact }; /* ── Sorting ────────────────────────────────────────────────────────────── */ -export const ISSUE_SORTS = ["impact", "newest", "changed", "effort"] as const; +export const ISSUE_SORTS = [ + "impact", + "newest", + "changed", + "state", + "diagnosis", + "pages", + "confidence", + "effort", +] as const; export type IssueSort = (typeof ISSUE_SORTS)[number]; /** Impact is the default, because it is the only one that ranks by consequence. */ @@ -1260,6 +1280,10 @@ export const ISSUE_SORT_LABEL: Record = { impact: "Impact", newest: "Newest", changed: "What changed", + state: "State", + diagnosis: "Diagnosis", + pages: "Pages", + confidence: "Confidence", effort: "Effort", }; @@ -1270,6 +1294,23 @@ export function parseIssueSort(value: string | null | undefined): IssueSort { /** Least work first, so a sort by effort surfaces what can be cleared today. */ const EFFORT_ORDER: Record = { minutes: 0, hours: 1, days: 2, unknown: 3 }; +/** + * State and confidence rank in the registry's own declared order, derived from + * its arrays rather than restated here. + * + * Both arrays are already ordered the way a reader wants to read them — the + * lifecycle from `new` to `dismissed`, and confidence from `confirmed` down to + * `unclear` — so indexing them IS the comparator. Copying the numbers out would + * be R1's F10 again: a rule stated in one place and duplicated in a constant + * that no longer changes with it. A state added to the registry lands in the + * right position here without this file being touched. + */ +const indexOrder = (values: readonly T[]): Record => + Object.fromEntries(values.map((value, index) => [value, index])) as Record; + +const STATE_ORDER: Record = indexOrder(WORK_STATES); +const CONFIDENCE_ORDER: Record = indexOrder(CONFIDENCES); + /** The calendar day of an ISO stamp, for comparing a detection to a run. */ const dayOf = (iso: string): string => iso.slice(0, 10); @@ -1304,6 +1345,16 @@ export function sortRemediationGroups( impact: (a, b) => byWorstMeasured(a, b) || byNewest(a, b) || byId(a, b), newest: (a, b) => byNewest(a, b) || byId(a, b), changed: (a, b) => inLastRun(a) - inLastRun(b) || byNewest(a, b) || byId(a, b), + state: (a, b) => STATE_ORDER[a.state] - STATE_ORDER[b.state] || byWorstMeasured(a, b) || byId(a, b), + // The text the row actually shows, so the order a reader sees is the order + // they could have worked out from the screen. + diagnosis: (a, b) => + diagnosisLineOf(a.primary).localeCompare(diagnosisLineOf(b.primary)) || byId(a, b), + // Broadest first: the count is the reason to look, and one fix covering six + // pages is the case this sort exists to surface. + pages: (a, b) => b.pageIds.length - a.pageIds.length || byWorstMeasured(a, b) || byId(a, b), + confidence: (a, b) => + CONFIDENCE_ORDER[a.confidence] - CONFIDENCE_ORDER[b.confidence] || byWorstMeasured(a, b) || byId(a, b), effort: (a, b) => EFFORT_ORDER[a.effort] - EFFORT_ORDER[b.effort] || byWorstMeasured(a, b) || byId(a, b), }; diff --git a/src/lib/__tests__/issues-list.test.ts b/src/lib/__tests__/issues-list.test.ts index fed201f..4a7dd32 100644 --- a/src/lib/__tests__/issues-list.test.ts +++ b/src/lib/__tests__/issues-list.test.ts @@ -277,6 +277,55 @@ describe("sortRemediationGroups", () => { expect(changed.slice(0, 2).sort()).toEqual(["quick", "vague"]); }); + /* ── The four column sorts ──────────────────────────────────────────── */ + + it("ranks state down the registry's lifecycle, not alphabetically", () => { + // The two orders disagree, which is the point: alphabetically this is + // dismissed, in_progress, new — exactly backwards for a triage list. + const groups = groupByRemediation([ + makeCase({ id: "gone", cause: "gone", state: "dismissed" }), + makeCase({ id: "fresh", cause: "fresh", state: "new" }), + makeCase({ id: "doing", cause: "doing", state: "in_progress" }), + ], { at: "2026-08-25T06:00:00.000Z" }); + expect(sortRemediationGroups(groups, "state", lastRun).map((group) => group.primary.id)) + .toEqual(["fresh", "doing", "gone"]); + }); + + it("ranks confidence strongest first", () => { + // The registry's order happens to coincide with alphabetical here, so this + // pins the intent — confirmed at the top — rather than the mechanism. + const groups = groupByRemediation([ + makeCase({ id: "vague", cause: "vague", confidence: "unclear" }), + makeCase({ id: "sure", cause: "sure", confidence: "confirmed" }), + makeCase({ id: "likely", cause: "likely", confidence: "probable" }), + ], { at: "2026-08-25T06:00:00.000Z" }); + expect(sortRemediationGroups(groups, "confidence", lastRun).map((group) => group.primary.id)) + .toEqual(["sure", "likely", "vague"]); + }); + + it("ranks pages broadest first, because breadth is the reason to look", () => { + const groups = groupByRemediation([ + makeCase({ id: "one", cause: "one", pageIds: ["home"] }), + makeCase({ id: "six", cause: "six", scope: "pages", pageIds: ["home", "pricing", "docs", "blog", "about", "help"] }), + makeCase({ id: "two", cause: "two", scope: "pages", pageIds: ["home", "pricing"] }), + ], { at: "2026-08-25T06:00:00.000Z" }); + expect(sortRemediationGroups(groups, "pages", lastRun).map((group) => group.primary.id)) + .toEqual(["six", "two", "one"]); + }); + + it("ranks diagnosis by the sentence the row shows, falling back to the title", () => { + // "alpha" carries no diagnosis, so the row shows its stored title and this + // sort has to order on the same string — otherwise the list is alphabetical + // by text nobody can see. + const groups = groupByRemediation([ + makeCase({ id: "zeta", cause: "zeta", diagnosis: "Zeta blocks rendering." }), + makeCase({ id: "alpha", cause: "alpha", diagnosis: "", title: "Alpha blocks rendering." }), + makeCase({ id: "mid", cause: "mid", diagnosis: "Mid blocks rendering." }), + ], { at: "2026-08-25T06:00:00.000Z" }); + expect(sortRemediationGroups(groups, "diagnosis", lastRun).map((group) => group.primary.id)) + .toEqual(["alpha", "mid", "zeta"]); + }); + it("does not mutate its input", () => { const before = groups.map((group) => group.key); sortRemediationGroups(groups, "effort", lastRun); From 8073ab75c87c184e95436ba2d98f06572028d2cf Mon Sep 17 00:00:00 2001 From: Matthew P Munger Date: Fri, 28 Aug 2026 16:50:10 -0500 Subject: [PATCH 6/8] feat(issues): show the cause, keep the diagnosis a click away, align the columns MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three things about the list row, and one of them is why the other two were possible. COLUMNS THAT LINE UP. `ISSUE_ROW_COLUMNS` is shared by the header, the group headers and every row so that the six columns line up — but each row is its own grid, and `max-content` resolves per grid. Every row was therefore sized to ITS content: effort measured 29px on a row reading "Days" and 71px on one reading "No estimate", state 72px against 82px, and the two `fr` columns inherited the difference. Shared template, six columns lining up nowhere. The four deterministic tracks are now fixed widths — the longest label each can hold, measured from the rendered list — so the template no longer reads the content. `minmax(floor, fixed)` rather than bare pixels, so a narrow viewport takes the space back in the same order for every row. Confidence is sized by its HEADER, because "CONFIDENCE" is wider than "Probable" and a header that ellipses is a column nobody can identify. THE CAUSE, NOT THE SENTENCE. The column showed the diagnosis, which is a sentence, in a list read by scanning. It now shows what KIND of problem it is — "Code the site never runs", "Images bigger than they are shown" — from the classifier that already authors these and already shows them as a chip on the page detail. Not a new vocabulary; the existing one reaching the list. Where the classifier does not recognise the audit it answers `other`, labelled "Something else the nightly test found", which tells a reader strictly less than the sentence it would replace — the visitor and agent findings all land there. Those keep their diagnosis. The column is shorter where there is something shorter to say and never emptier than it was, and a row whose cause IS its diagnosis gets no disclosure, because a control that reveals the text you are already reading is worse than no control. THE ROW IS NO LONGER ONE BIG LINK. It could not stay one: the pages in it are now links to their own detail, and an anchor inside an anchor is invalid — the browser closes the outer one and the row quietly becomes two links with a gap. The disclosure has the same problem as a button. So the case link is a normal link on the cause and stretches its hit area over the row in CSS: the row-sized click target survives, without the row-sized announcement the UX audit called out. Note the truncation had to move off that link and onto a span inside it — `overflow: hidden` on the link clips its own stretched `::after` back to one column, which looks identical and is not. `title` on that link is now the VISIBLE text. It becomes the accessible name, so a diagnosis there announced and voice-targeted the link as something other than what it reads as. The page-naming rule stays `scope-line`'s — two named, more than two counted — exported as `pageScopeNames` rather than re-derived, and a count is not a page so that branch links nothing. `pageHref` joins `caseHref` in `paths.ts` for the same reason it exists: three callers, one spelling. Co-Authored-By: Claude Opus 5 --- src/app/globals.css | 89 ++++++++++++ src/components/issue-group.tsx | 14 +- src/components/issue-row.tsx | 252 ++++++++++++++++++++++++++------- src/lib/case-copy.ts | 28 ++++ src/lib/paths.ts | 19 +++ src/lib/scope-line.ts | 50 +++++-- 6 files changed, 387 insertions(+), 65 deletions(-) diff --git a/src/app/globals.css b/src/app/globals.css index 3780386..3ed334b 100644 --- a/src/app/globals.css +++ b/src/app/globals.css @@ -2730,3 +2730,92 @@ textarea:focus-visible, border: 0; } } + +/* ── The issues list row ────────────────────────────────────────────────── */ + +/* + * The row is a grid of cells, not one big link, because two of its cells are + * links to somewhere else and a nested anchor is invalid. The case link instead + * stretches its own hit area over the whole row, which keeps the row-sized + * click target without a row-sized announcement. + */ +.issue-row { + position: relative; +} + +.issue-row__open { + color: inherit; + text-decoration: none; +} + +/* The stretched hit area. Not a second link — the same one, made row-sized. */ +.issue-row__open::after { + content: ""; + position: absolute; + inset: 0; +} + +.issue-row:hover { + background: var(--surface-raised); +} + +.issue-row__open:focus-visible { + outline: none; +} + +/* The focus ring belongs on the row, because the row is what the link covers. */ +.issue-row:has(.issue-row__open:focus-visible) { + outline: 2px solid var(--focus-ring); + outline-offset: -2px; +} + +/* + * Above the stretched link, so they keep their own targets. Without the + * stacking context these sit under it and every click opens the case. + */ +.issue-row__page, +.issue-row__disclosure { + position: relative; + z-index: 1; +} + +.issue-row__page { + color: inherit; + text-decoration: none; +} + +.issue-row__page:hover, +.issue-row__page:focus-visible { + color: var(--action-primary-ink); + text-decoration: underline; +} + +.issue-row__disclosure { + flex: 0 0 auto; + display: inline-flex; + align-items: center; + justify-content: center; + width: 18px; + height: 18px; + padding: 0; + border: none; + border-radius: 4px; + background: transparent; + color: var(--text-muted); + font-size: 11px; + line-height: 1; + cursor: pointer; +} + +.issue-row__disclosure:hover { + background: var(--surface-input); + color: var(--text-body); +} + +/* Keeps the cause labels on one line down the list where a row has no second + layer to disclose. Same width as the control it stands in for. */ +.issue-row__disclosure-spacer { + flex: 0 0 auto; + width: 18px; + height: 18px; +} diff --git a/src/components/issue-group.tsx b/src/components/issue-group.tsx index c928a87..3f3860b 100644 --- a/src/components/issue-group.tsx +++ b/src/components/issue-group.tsx @@ -7,9 +7,9 @@ import { ISSUE_ROW_GAP, IssueRow, NUMERIC_CELL, + PageScope, TRUNCATE_CELL, formatGroupImpact, - scopeLineOf, } from "@/components/issue-row"; /** @@ -49,7 +49,6 @@ export function IssueGroup({ group, basePath, pageTitles, nested = false }: Issu // The remediation's first step, which is what the shared fix starts with. It // is data, from the case; nothing is authored here. const fix = group.remediation.steps.find((step) => step.trim() !== "") ?? group.primary.title; - const scope = scopeLineOf(group.pageIds, group.primary.strategies, pageTitles); return (
@@ -74,8 +73,17 @@ export function IssueGroup({ group, basePath, pageTitles, nested = false }: Issu {fix} + {/* The pages link here for the same reason they do on a row: this is + the same column, and a header that names a page the reader cannot + open teaches them the column is not clickable. */} - {`${group.cases.length} cases · ${scope}`} + {`${group.cases.length} cases · `} + {/* The weakest member confidence, as a word. See the note in issue-row. */} diff --git a/src/components/issue-row.tsx b/src/components/issue-row.tsx index 0c07164..ca06878 100644 --- a/src/components/issue-row.tsx +++ b/src/components/issue-row.tsx @@ -1,11 +1,14 @@ +"use client"; + import Link from "next/link"; -import type { CSSProperties } from "react"; +import { useId, useState, type CSSProperties } from "react"; import type { IssueCase } from "@/lib/issue-case"; +import type { Strategy } from "@/lib/types"; import { EFFORT_LABEL, formatGroupImpact, formatImpact } from "@/lib/impact-format"; -import { scopeLineOf } from "@/lib/scope-line"; -import { diagnosisLineOf } from "@/lib/case-copy"; +import { PAGE_SCOPE_NAME_LIMIT, SCOPE_SEPARATOR, deviceScopeOf, pageScopeNames, scopeLineOf } from "@/lib/scope-line"; +import { causeLineOf, diagnosisLineOf } from "@/lib/case-copy"; import { CONFIDENCE_LABEL } from "@/lib/vocabulary"; -import { caseHref } from "@/lib/paths"; +import { caseHref, pageHref } from "@/lib/paths"; import { StatusChip } from "@/components/status-chip"; /** @@ -27,19 +30,37 @@ import { StatusChip } from "@/components/status-chip"; * the six columns line up down the whole list. Indentation is padding inside * the first cell, never a change to this template — a nested row that shifts * the tracks stops lining up with everything else. + * + * NO `max-content` HERE, and that is the point rather than an oversight. + * + * Every row is its own grid. `max-content` is resolved per grid, so a template + * that says "as wide as the content" makes each row as wide as ITS content: the + * effort column measured 29px on a row reading "Days" and 71px on one reading + * "No estimate", the state column 72px against 82px, and the two `fr` columns + * inherited the difference. Sharing the template gave six columns that lined up + * nowhere except by accident. The fix is tracks whose size does not depend on + * what is in them. + * + * The four fixed widths are the longest label each column can hold, measured + * from the rendered list and rounded up: nothing is clipped today, and anything + * longer ellipses through `TRUNCATE_CELL` rather than pushing the row wider. + * They are `minmax(floor, fixed)` rather than bare pixels so a narrow viewport + * takes the space back in the same order for every row — deterministic, because + * the template no longer reads the content. */ export const ISSUE_ROW_COLUMNS = [ - // state — a chip carries meaning, so it keeps a floor and is never clipped - "minmax(72px, max-content)", - // diagnosis — takes the space the others do not need, and ellipses + // state — a chip carries meaning, so it keeps its floor and is never clipped + "minmax(72px, 104px)", + // cause — takes the space the others do not need, and ellipses "minmax(0, 1fr)", - // scope — shrinks alongside the diagnosis, at a smaller share of it + // pages — shrinks alongside the cause, at a smaller share of it "minmax(0, 0.62fr)", - // confidence, impact, effort — content-sized while there is room, and able to - // give it back rather than push the row past the viewport - "minmax(0, max-content)", - "minmax(88px, max-content)", - "minmax(0, max-content)", + // confidence, impact, effort. Confidence is sized by its HEADER rather than + // its values: "Probable" is 52px and "CONFIDENCE" is not, and a column header + // that ellipses is a column nobody can identify. + "minmax(0, 100px)", + "minmax(88px, 96px)", + "minmax(0, 88px)", ].join(" "); export const ISSUE_ROW_GAP = 14; @@ -59,6 +80,7 @@ export const ISSUE_ROW_NEST_INDENT = 18; */ export { EFFORT_LABEL, formatGroupImpact, formatImpact }; export { scopeLineOf }; +export { causeLineOf, diagnosisLineOf }; /** One line, ellipsed. Applied to every free-text cell in the list. */ export const TRUNCATE_CELL: CSSProperties = { @@ -90,56 +112,178 @@ export interface IssueRowProps { nested?: boolean; } +/** + * The pages a case covers, each one a link to its own detail. + * + * The naming rule is `scope-line`'s, not this file's: two pages are named, more + * than two become a count. A count is not a page, so there is nothing to link + * on that branch — which is the honest outcome rather than a link to whichever + * page happened to be first. + */ +export function PageScope({ + pageIds, + strategies, + basePath, + pageTitles, +}: { + pageIds: readonly string[]; + strategies: readonly Strategy[]; + basePath: string; + pageTitles: Record; +}) { + const names = pageScopeNames(pageIds, pageTitles); + const devices = deviceScopeOf(strategies); + const named = names.length > 0 && names.length <= PAGE_SCOPE_NAME_LIMIT; + + return ( + <> + {named + ? names.map((page, index) => ( + + {index > 0 ? ", " : null} + + {page.title} + + + )) + : names.length > 0 + ? `${names.length} pages` + : null} + {names.length > 0 && devices ? SCOPE_SEPARATOR : null} + {devices} + + ); +} + export function IssueRow({ issue, basePath, pageTitles, nested = false }: IssueRowProps) { + const [open, setOpen] = useState(false); + const panelId = useId(); const impact = formatImpact(issue.impactMs); - // The case's own plain sentence where it has one, falling back to the stored - // title. `case-copy` owns the choice, because the diagnosis sort orders rows - // by the same text. + // What KIND of problem this is, in three or four words — the column a list of + // four dozen rows is actually scanned on. + const cause = causeLineOf(issue); + // The sentence itself, one disclosure away rather than truncated at the + // column edge. `case-copy` owns the title fallback, because the cause sort + // and this both read it. const diagnosis = diagnosisLineOf(issue); + // Where the classifier did not recognise the audit, `causeLineOf` already + // fell back to this same sentence — so there is no second layer to open, and + // a control that reveals the text you are looking at is worse than no + // control. The 18px is still reserved, so the labels start on one line down + // the whole list whether or not a row has something to show. + const hasSecondLayer = diagnosis !== "" && diagnosis !== cause; return ( - - - - - - - {diagnosis} - - - - {scopeLineOf(issue.pageIds, issue.strategies, pageTitles)} - - - {/* The word, in the row's secondary text token — never a strength hue. - `--confidence-weak` under the word "Confirmed" is a token painting - the opposite of what it says (registry rule 13), and hue here would - double-encode a value the word already carries. Strength as colour - belongs where there is no word to read it from. */} - - {CONFIDENCE_LABEL[issue.confidence]} - - - - {impact.text} - - - - {EFFORT_LABEL[issue.effort]} - - + {/* + The row is no longer one big ``. + + It could not stay one: the pages in it are links to somewhere else, and + an anchor inside an anchor is invalid — the browser closes the outer one + and the row silently becomes two links with a gap between them. The + disclosure has the same problem as a button. + + So the case link is a normal link on the cause, and `.issue-row__open` + stretches its hit area over the whole row in CSS. One focusable link per + row rather than a row-sized target that reads its six cells aloud, which + is the announcement the UX audit called out; the page links and the + disclosure sit above it and keep their own targets. + */} +
+ + + + + {/* NOT `TRUNCATE_CELL` on this cell or on the link inside it. The + stretched hit area is a positioned `::after` on the link, and + `overflow: hidden` anywhere above it clips that box back to the + cell — which is a whole-row target that only covers one column. + The truncation moves to the span around the text instead, where it + still ellipses and no longer clips anything. */} + + {hasSecondLayer ? ( + + ) : ( + + + + + + + {/* The word, in the row's secondary text token — never a strength hue. + `--confidence-weak` under the word "Confirmed" is a token painting + the opposite of what it says (registry rule 13), and hue here would + double-encode a value the word already carries. Strength as colour + belongs where there is no word to read it from. */} + + {CONFIDENCE_LABEL[issue.confidence]} + + + + {impact.text} + + + + {EFFORT_LABEL[issue.effort]} + +
+ + {/* The second layer. Nothing is hidden that was not already truncated — + the sentence used to end in an ellipsis at the column edge, and now it + ends where it ends. */} + {open && hasSecondLayer ? ( +
+ {diagnosis} +
+ ) : null} +
); } diff --git a/src/lib/case-copy.ts b/src/lib/case-copy.ts index 76797a8..0b58145 100644 --- a/src/lib/case-copy.ts +++ b/src/lib/case-copy.ts @@ -1,4 +1,5 @@ import { CONFIDENCE_LABEL, type Confidence, type ExclusionReason } from "./vocabulary"; +import { webflowClassificationFor } from "./webflowPerformance"; /** * The words the case says, in one place. @@ -47,6 +48,33 @@ export function diagnosisLineOf(issue: { diagnosis: string; title: string }): st return issue.diagnosis || issue.title; } +/** + * What KIND of problem this is, in three or four words. + * + * "Code running at startup", "Images bigger than they are shown". The classifier + * already authors these against the audit id, and the page detail already shows + * them as a chip — this is not a new vocabulary, it is the existing one reaching + * the list. + * + * It exists because a diagnosis is a sentence and a list of four dozen rows is + * read by scanning, not by reading. The sentence is still the answer; it is one + * disclosure away rather than truncated at the column edge. + * + * `cause` is the audit id and `title` is what the source called it — the two + * inputs the classifier takes, and both are fields the case already carries. + * + * Where the classifier does NOT recognise the audit it answers `other`, and its + * label for that is "Something else the nightly test found" — which tells a + * reader strictly less than the sentence it would be replacing. The visitor + * findings and the agent ones all land there. So the fallback is the diagnosis + * itself: the column is shorter where there is something shorter to say, and + * never emptier than it was. + */ +export function causeLineOf(issue: { cause: string; title: string; diagnosis: string }): string { + const classification = webflowClassificationFor({ id: issue.cause, title: issue.title }); + return classification.culprit === "other" ? diagnosisLineOf(issue) : classification.culpritLabel; +} + /* ── The pages table ────────────────────────────────────────────────────── */ export function pagesCount(included: number, excluded: number): string { diff --git a/src/lib/paths.ts b/src/lib/paths.ts index 4a8c181..ffb4a72 100644 --- a/src/lib/paths.ts +++ b/src/lib/paths.ts @@ -43,6 +43,25 @@ export function caseHref(basePath: string, caseId: string): string { return withBasePath(basePath, casePath(caseId)); } +/* ── The page address ─────────────────────────────────────────── */ + +/** + * Where one watched page lives, from the app root. + * + * The same reasoning as `casePath`, for the other object this app has detail + * screens for. Written down once because the list row, the case's Affected + * pages table and the watchlist all link to it, and three spellings is three + * places for a route change to be missed. + */ +export function pagePath(pageId: string): string { + return `${DESTINATION_PATH.pages}/${encodeURIComponent(pageId)}`; +} + +/** The same address, inside the app, with the deployment's base path on it. */ +export function pageHref(basePath: string, pageId: string): string { + return withBasePath(basePath, pagePath(pageId)); +} + /** * The same address from outside the app, where a relative link is no use. * diff --git a/src/lib/scope-line.ts b/src/lib/scope-line.ts index e05868b..04ef147 100644 --- a/src/lib/scope-line.ts +++ b/src/lib/scope-line.ts @@ -17,22 +17,57 @@ import type { Strategy } from "./types"; */ const STRATEGY_LABEL: Record = { mobile: "Mobile", desktop: "Desktop" }; +/** + * How many pages a scope line names before it gives up and shows a count. + * + * Two titles are still a list a reader can hold; past that the count is the more + * useful fact, and a sentence naming six pages is a sentence nobody finishes. + * + * Exported because the list row renders this same decision as links rather than + * as a string, and a second copy of "two" is how the string and the links start + * disagreeing about when a page stops being named (rule 20). + */ +export const PAGE_SCOPE_NAME_LIMIT = 2; + +/** The pages a scope covers, titled, in the order the case lists them. */ +export function pageScopeNames( + pageIds: readonly string[], + pageTitles: Record, +): { id: string; title: string }[] { + return pageIds.map((pageId) => ({ id: pageId, title: pageTitles[pageId] ?? pageId })); +} + /** * Just the pages, with no device clause: "Pricing" · "Pricing, Home" · "4 pages". * - * Two titles are named because two is still a list a reader can hold; past that - * the count is the more useful fact, and a sentence naming six pages is a - * sentence nobody finishes. + * The string form. `pageScopeNames` is the same decision with the ids kept, for + * the row that needs to link each one. */ export function pageScopeOf( pageIds: readonly string[], pageTitles: Record, ): string { - const titles = pageIds.map((pageId) => pageTitles[pageId] ?? pageId); - if (titles.length === 0) return ""; - return titles.length <= 2 ? titles.join(", ") : `${titles.length} pages`; + const names = pageScopeNames(pageIds, pageTitles); + if (names.length === 0) return ""; + return names.length <= PAGE_SCOPE_NAME_LIMIT + ? names.map((page) => page.title).join(", ") + : `${names.length} pages`; } +/** + * The device clause on its own: "Mobile" · "Mobile, Desktop". + * + * Split out for the same reason as `pageScopeNames`: the list row renders the + * page half as links and the device half as text, so it needs the two pieces + * rather than the finished sentence, and it must not spell either itself. + */ +export function deviceScopeOf(strategies: readonly Strategy[]): string { + return strategies.map((strategy) => STRATEGY_LABEL[strategy]).join(", "); +} + +/** The separator between the two halves, and between two named pages. */ +export const SCOPE_SEPARATOR = " \u00b7 "; + /** "Pricing" · "Pricing, Home" · "4 pages", then the devices it was seen on. */ export function scopeLineOf( pageIds: readonly string[], @@ -40,6 +75,5 @@ export function scopeLineOf( pageTitles: Record, ): string { const where = pageScopeOf(pageIds, pageTitles); - const devices = strategies.map((strategy) => STRATEGY_LABEL[strategy]).join(", "); - return [where, devices].filter(Boolean).join(" · "); + return [where, deviceScopeOf(strategies)].filter(Boolean).join(SCOPE_SEPARATOR); } From 670be4d0239315f6a9a3dbd4f0b7dfa724571d87 Mon Sep 17 00:00:00 2001 From: Matthew P Munger Date: Fri, 28 Aug 2026 16:50:22 -0500 Subject: [PATCH 7/8] feat(issues): a second click on a column reverses it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reverses the call made two commits ago. Each sort still has a canonical direction with a reason attached, so that is what the FIRST click gives you — the lifecycle from `new`, effort from the cheapest, impact from the largest, rather than a uniform descending that is wrong for half of them. The second click reverses, and the arrow on the header says which way it currently reads. Direction lives in `?dir=`, and only when it is not the sort's own default, so the common link stays short and a reversed one is explicit and still sendable. The sign applies to the PRIMARY key only. Everything after the first `||` is a tie-break, and a tie-break that flipped with the header would reshuffle rows nobody asked to reorder. Rule 18 had to come OUT of `byWorstMeasured` for impact, into a `measuredFirst` that sits outside the sign. Reversing "largest saving first" has to give "smallest MEASURED first", never "no reading first" — an absent measurement is not a small one, and a direction toggle is not permission to rank it as zero, which is exactly what negating the combined comparator did. Effort keeps `byWorstMeasured` as its tie-break, so "hardest first" does not float an unmeasured finding to the top of its band either. Both are tested in both directions. The `diagnosis` sort becomes `cause`, following the column: it ranks on the label the row now shows rather than on a sentence that is no longer on screen, which is the same rule the sort was written under. Its test pins that the order comes from the labels, not the audit ids behind them — bootup-time, dom-size, unused-javascript sort one way and their labels another. Co-Authored-By: Claude Opus 5 --- src/app/(app)/issues/page.tsx | 71 ++++++++++++++------ src/components/store.tsx | 95 ++++++++++++++++++++++----- src/lib/__tests__/issues-list.test.ts | 74 ++++++++++++++++++--- 3 files changed, 196 insertions(+), 44 deletions(-) diff --git a/src/app/(app)/issues/page.tsx b/src/app/(app)/issues/page.tsx index 0da9358..2cff8dc 100644 --- a/src/app/(app)/issues/page.tsx +++ b/src/app/(app)/issues/page.tsx @@ -5,12 +5,16 @@ import { useRouter, useSearchParams } from "next/navigation"; import Link from "next/link"; import { DEFAULT_ISSUE_SORT, + DEFAULT_SORT_DIRECTION, ISSUE_SORTS, ISSUE_SORT_LABEL, parseIssueSort, + parseSortDirection, + reverseDirection, useIssuesView, useStore, type IssueSort, + type SortDirection, } from "@/components/store"; import { COUNTED_QUEUES, DESTINATION_LABEL, DESTINATION_PATH, QUEUE_LABEL, parseQueue, type Queue } from "@/lib/vocabulary"; import { PageHeader } from "@/components/page-header"; @@ -55,35 +59,44 @@ import { WATCH_EMPTY } from "@/lib/watch-copy"; * another, and neither is a preference worth storing. */ +/** Which way round the active column currently reads. */ +const DIRECTION_GLYPH: Record = { asc: "\u2191", desc: "\u2193" }; +const DIRECTION_WORD: Record = { asc: "ascending", desc: "descending" }; + /** * Column headers, drawn on the same six tracks as the rows below them — and the * sort control for the column each one heads. * - * The header IS the sort rather than a caption above one. Two consequences worth - * stating, because both were decisions: + * The header IS the sort rather than a caption above one: * * - The label comes from `ISSUE_SORT_LABEL`, so a column and the Sort menu * read one map and cannot end up calling the same ordering two things. - * - There is no direction to toggle. Each sort has one canonical direction - * with a reason attached — least effort first surfaces what can be cleared - * today, broadest first surfaces the fix that covers six pages — and - * reversing them produces orders nobody asked for ("hardest first"). So no - * arrow is drawn, because an arrow would promise a second click that does - * something. + * - The first click sorts the column in ITS OWN default direction rather than + * a uniform descending — a triage list wants the lifecycle from `new` and + * effort from the cheapest. The second click reverses it, and the arrow + * says which way it currently reads. * * Newest and What changed head no column: there is no date column to head. The * menu is what keeps them reachable, which is why it stays. */ const COLUMN_SORTS = [ "state", - "diagnosis", + "cause", "pages", "confidence", "impact", "effort", ] as const satisfies readonly IssueSort[]; -function ColumnHeaders({ sort, hrefFor }: { sort: IssueSort; hrefFor: (next: IssueSort) => string }) { +function ColumnHeaders({ + sort, + direction, + hrefFor, +}: { + sort: IssueSort; + direction: SortDirection; + hrefFor: (next: { sort: IssueSort; dir?: SortDirection }) => string; +}) { return (
{ const active = key === sort; const numeric = index >= 4; + // An inactive column opens in its own default; the active one reverses. + const next = active ? reverseDirection(direction) : DEFAULT_SORT_DIRECTION[key]; return ( + {/* Numeric columns are right-aligned, so their arrow leads rather + than trails; the label still ends at the column edge. */} + {numeric && active ? : null} {ISSUE_SORT_LABEL[key]} + {!numeric && active ? : null} ); })} @@ -141,21 +165,30 @@ export default function IssuesPage() { const queue = parseQueue(searchParams.get("queue")); const sort = parseIssueSort(searchParams.get("sort")); - const view = useIssuesView(queue, sort); + const direction = parseSortDirection(searchParams.get("dir"), sort); + const view = useIssuesView(queue, sort, direction); // The tail starts folded. Opening it is the one action this list offers, and // it is not a commitment to anything — see the note on the fold below. const [tailOpen, setTailOpen] = useState(false); const linkTo = useMemo( - () => (next: { queue?: Queue; sort?: IssueSort }) => { + () => (next: { queue?: Queue; sort?: IssueSort; dir?: SortDirection }) => { const params = new URLSearchParams(); params.set("queue", next.queue ?? queue); const nextSort = next.sort ?? sort; + // Naming a sort without a direction means "start it the way it reads by + // default" — which is how the menu behaves, and how the first click on a + // column behaves. Staying on the current sort keeps the current + // direction, so changing queue does not silently un-reverse the list. + const nextDir = next.dir ?? (nextSort === sort ? direction : DEFAULT_SORT_DIRECTION[nextSort]); if (nextSort !== DEFAULT_ISSUE_SORT) params.set("sort", nextSort); + // Only a direction that is not the sort's own default reaches the URL, so + // the common link stays short and a reversed one is explicit. + if (nextDir !== DEFAULT_SORT_DIRECTION[nextSort]) params.set("dir", nextDir); return pathFor(`${DESTINATION_PATH.issues}?${params.toString()}`); }, - [pathFor, queue, sort], + [pathFor, queue, sort, direction], ); // The same counts the tabs badge, stated in a sentence. One selector behind @@ -254,7 +287,7 @@ export default function IssuesPage() { />
- linkTo({ sort: next })} /> +
{view.groups.map((group) => ( diff --git a/src/components/store.tsx b/src/components/store.tsx index 02832ad..460ebe5 100644 --- a/src/components/store.tsx +++ b/src/components/store.tsx @@ -16,6 +16,7 @@ import { byWorstMeasured, casesInQueue, groupByRemediation, + hasMeasuredImpact, type Effort, type IssueCase, type RemediationGroup, @@ -34,7 +35,7 @@ import { type Queue, type WorkState, } from "@/lib/vocabulary"; -import { diagnosisLineOf } from "@/lib/case-copy"; +import { causeLineOf } from "@/lib/case-copy"; import { normalizeNativeElementControls } from "@/lib/nativeElements"; import { localISODate } from "@/lib/ui"; import { withBasePath } from "@/lib/paths"; @@ -1266,7 +1267,7 @@ export const ISSUE_SORTS = [ "newest", "changed", "state", - "diagnosis", + "cause", "pages", "confidence", "effort", @@ -1281,7 +1282,7 @@ export const ISSUE_SORT_LABEL: Record = { newest: "Newest", changed: "What changed", state: "State", - diagnosis: "Diagnosis", + cause: "Cause", pages: "Pages", confidence: "Confidence", effort: "Effort", @@ -1291,6 +1292,45 @@ export function parseIssueSort(value: string | null | undefined): IssueSort { return (ISSUE_SORTS as readonly string[]).includes(value ?? "") ? (value as IssueSort) : DEFAULT_ISSUE_SORT; } +/* ── Direction ──────────────────────────────────────────────────────────── */ + +export const SORT_DIRECTIONS = ["asc", "desc"] as const; +export type SortDirection = (typeof SORT_DIRECTIONS)[number]; + +/** + * The direction each sort reads in when you first ask for it. + * + * Not a uniform default, because "descending" is not a useful opening move for + * half of these: a triage list wants the lifecycle from `new`, effort from the + * cheapest, confidence from the strongest, and impact from the largest. These + * are the directions the sorts already had before they could be reversed, kept + * as the first click so nothing about the list changed under anyone. + */ +export const DEFAULT_SORT_DIRECTION: Record = { + impact: "desc", + newest: "desc", + changed: "desc", + state: "asc", + cause: "asc", + pages: "desc", + confidence: "asc", + effort: "asc", +}; + +export function parseSortDirection( + value: string | null | undefined, + sort: IssueSort, +): SortDirection { + return (SORT_DIRECTIONS as readonly string[]).includes(value ?? "") + ? (value as SortDirection) + : DEFAULT_SORT_DIRECTION[sort]; +} + +/** The direction a second click on an already-sorted column asks for. */ +export function reverseDirection(direction: SortDirection): SortDirection { + return direction === "asc" ? "desc" : "asc"; +} + /** Least work first, so a sort by effort surfaces what can be cleared today. */ const EFFORT_ORDER: Record = { minutes: 0, hours: 1, days: 2, unknown: 3 }; @@ -1327,12 +1367,29 @@ export function sortRemediationGroups( groups: readonly RemediationGroup[], sort: IssueSort, lastRunAt?: string, + direction: SortDirection = DEFAULT_SORT_DIRECTION[sort], ): RemediationGroup[] { + // Applied to the PRIMARY key only. Everything after the first `||` is a + // tie-break, and a tie-break that flipped with the header would reshuffle + // rows the reader did not ask to reorder. + const sign = direction === DEFAULT_SORT_DIRECTION[sort] ? 1 : -1; const lastRunDay = lastRunAt ? dayOf(lastRunAt) : undefined; const inLastRun = (group: RemediationGroup): number => lastRunDay && group.detectedAt && dayOf(group.detectedAt) >= lastRunDay ? 0 : 1; const byNewest = (a: RemediationGroup, b: RemediationGroup) => b.detectedAt.localeCompare(a.detectedAt); + + /** + * Rule 18, split out of `byWorstMeasured` so it can sit OUTSIDE the sign. + * + * Reversing "largest saving first" has to give "smallest MEASURED first", not + * "no reading first". An absent measurement is not a small one, and a + * direction toggle is not permission to rank it as zero — which is exactly + * what negating the combined comparator would have done. + */ + const measuredFirst = (a: RemediationGroup, b: RemediationGroup) => + Number(hasMeasuredImpact(b.impactMs)) - Number(hasMeasuredImpact(a.impactMs)); + const byImpactSize = (a: RemediationGroup, b: RemediationGroup) => b.impactMs - a.impactMs; // The id tie-break keeps the order stable when the sort key matches, so a // re-render never reshuffles equal rows. const byId = (a: RemediationGroup, b: RemediationGroup) => a.primary.id.localeCompare(b.primary.id); @@ -1342,20 +1399,22 @@ export function sortRemediationGroups( // ordered by a zero they never measured. Newest and What changed rank on a // date every case carries, so no measurement stands in for a missing one. const compare: Record number> = { - impact: (a, b) => byWorstMeasured(a, b) || byNewest(a, b) || byId(a, b), - newest: (a, b) => byNewest(a, b) || byId(a, b), - changed: (a, b) => inLastRun(a) - inLastRun(b) || byNewest(a, b) || byId(a, b), - state: (a, b) => STATE_ORDER[a.state] - STATE_ORDER[b.state] || byWorstMeasured(a, b) || byId(a, b), + impact: (a, b) => measuredFirst(a, b) || sign * byImpactSize(a, b) || byNewest(a, b) || byId(a, b), + newest: (a, b) => sign * byNewest(a, b) || byId(a, b), + changed: (a, b) => sign * (inLastRun(a) - inLastRun(b)) || byNewest(a, b) || byId(a, b), + state: (a, b) => sign * (STATE_ORDER[a.state] - STATE_ORDER[b.state]) || byWorstMeasured(a, b) || byId(a, b), // The text the row actually shows, so the order a reader sees is the order // they could have worked out from the screen. - diagnosis: (a, b) => - diagnosisLineOf(a.primary).localeCompare(diagnosisLineOf(b.primary)) || byId(a, b), + cause: (a, b) => sign * causeLineOf(a.primary).localeCompare(causeLineOf(b.primary)) || byId(a, b), // Broadest first: the count is the reason to look, and one fix covering six // pages is the case this sort exists to surface. - pages: (a, b) => b.pageIds.length - a.pageIds.length || byWorstMeasured(a, b) || byId(a, b), + pages: (a, b) => sign * (b.pageIds.length - a.pageIds.length) || byWorstMeasured(a, b) || byId(a, b), confidence: (a, b) => - CONFIDENCE_ORDER[a.confidence] - CONFIDENCE_ORDER[b.confidence] || byWorstMeasured(a, b) || byId(a, b), - effort: (a, b) => EFFORT_ORDER[a.effort] - EFFORT_ORDER[b.effort] || byWorstMeasured(a, b) || byId(a, b), + sign * (CONFIDENCE_ORDER[a.confidence] - CONFIDENCE_ORDER[b.confidence]) || byWorstMeasured(a, b) || byId(a, b), + // The effort tie-break keeps rule 18 inside each band in both directions: + // reversing to "hardest first" must not float an unmeasured finding to the + // top of its band. + effort: (a, b) => sign * (EFFORT_ORDER[a.effort] - EFFORT_ORDER[b.effort]) || byWorstMeasured(a, b) || byId(a, b), }; return [...groups].sort(compare[sort]); @@ -1390,7 +1449,11 @@ export interface IssuesView { * things a person should be able to link someone to, and neither is a * preference worth persisting. */ -export function useIssuesView(queue: Queue, sort: IssueSort): IssuesView { +export function useIssuesView( + queue: Queue, + sort: IssueSort, + direction: SortDirection = DEFAULT_SORT_DIRECTION[sort], +): IssuesView { const { recs, pages, performanceThresholds, caseDecisions } = useStore(); return useMemo(() => { // The decisions log is part of the derivation's input, not a filter applied @@ -1407,12 +1470,12 @@ export function useIssuesView(queue: Queue, sort: IssueSort): IssuesView { cases, counts: queueCountsOf(cases), inQueue, - groups: sortRemediationGroups(groupByRemediation(inline, at), sort, lastRunAt), - tail: sortRemediationGroups(groupByRemediation(tail, at), sort, lastRunAt), + groups: sortRemediationGroups(groupByRemediation(inline, at), sort, lastRunAt, direction), + tail: sortRemediationGroups(groupByRemediation(tail, at), sort, lastRunAt, direction), tailCases: tail, minimumSavingsMs, pageTitles: Object.fromEntries(pages.map((page) => [page.id, page.title])), lastRunAt, }; - }, [recs, pages, caseDecisions, performanceThresholds, queue, sort]); + }, [recs, pages, caseDecisions, performanceThresholds, queue, sort, direction]); } diff --git a/src/lib/__tests__/issues-list.test.ts b/src/lib/__tests__/issues-list.test.ts index 4a7dd32..ac6b783 100644 --- a/src/lib/__tests__/issues-list.test.ts +++ b/src/lib/__tests__/issues-list.test.ts @@ -1,10 +1,12 @@ import { describe, expect, it } from "vitest"; import { DEFAULT_ISSUE_SORT, + DEFAULT_SORT_DIRECTION, ISSUE_SORTS, issueCasesFrom, lastRunAtOf, parseIssueSort, + parseSortDirection, partitionByImpact, queueCountsOf, sortRemediationGroups, @@ -313,17 +315,71 @@ describe("sortRemediationGroups", () => { .toEqual(["six", "two", "one"]); }); - it("ranks diagnosis by the sentence the row shows, falling back to the title", () => { - // "alpha" carries no diagnosis, so the row shows its stored title and this - // sort has to order on the same string — otherwise the list is alphabetical - // by text nobody can see. + it("ranks cause by the label the row shows, not by the audit id behind it", () => { + // The ids sort as bootup-time, dom-size, unused-javascript. The labels they + // classify to sort differently, and the labels are what is on screen. const groups = groupByRemediation([ - makeCase({ id: "zeta", cause: "zeta", diagnosis: "Zeta blocks rendering." }), - makeCase({ id: "alpha", cause: "alpha", diagnosis: "", title: "Alpha blocks rendering." }), - makeCase({ id: "mid", cause: "mid", diagnosis: "Mid blocks rendering." }), + makeCase({ id: "nested", cause: "dom-size" }), // Deeply nested elements + makeCase({ id: "startup", cause: "bootup-time" }), // Code running at startup + makeCase({ id: "dead", cause: "unused-javascript" }), // Code the site never runs ], { at: "2026-08-25T06:00:00.000Z" }); - expect(sortRemediationGroups(groups, "diagnosis", lastRun).map((group) => group.primary.id)) - .toEqual(["alpha", "mid", "zeta"]); + expect(sortRemediationGroups(groups, "cause", lastRun).map((group) => group.primary.id)) + .toEqual(["startup", "dead", "nested"]); + }); + + /* ── Direction ────────────────────────────────────────────────────────── */ + + it("opens each sort in its own direction rather than a uniform descending", () => { + expect(parseSortDirection(undefined, "effort")).toBe("asc"); + expect(parseSortDirection(undefined, "impact")).toBe("desc"); + expect(parseSortDirection("nonsense", "state")).toBe("asc"); + expect(parseSortDirection("desc", "state")).toBe("desc"); + }); + + it("reverses when the same column is asked a second time", () => { + const stateGroups = groupByRemediation([ + makeCase({ id: "gone", cause: "gone", state: "dismissed" }), + makeCase({ id: "fresh", cause: "fresh", state: "new" }), + makeCase({ id: "doing", cause: "doing", state: "in_progress" }), + ], { at: "2026-08-25T06:00:00.000Z" }); + expect(sortRemediationGroups(stateGroups, "state", lastRun, "asc").map((group) => group.primary.id)) + .toEqual(["fresh", "doing", "gone"]); + expect(sortRemediationGroups(stateGroups, "state", lastRun, "desc").map((group) => group.primary.id)) + .toEqual(["gone", "doing", "fresh"]); + }); + + it("keeps rule 18 when impact is reversed: smallest MEASURED first, never the unmeasured", () => { + // The whole point of splitting rule 18 out of the sign. Reversing "largest + // saving first" asks for the smallest reading, not for the row that has no + // reading at all — an absent number is not a small one. + const order = sortRemediationGroups(groups, "impact", lastRun, "asc").map((group) => group.primary.id); + expect(order).toEqual(["quick", "mid", "slow", "vague"]); + expect(order.at(-1)).toBe("vague"); + }); + + it("keeps rule 18 when effort is reversed, inside each band", () => { + const sameBand = groupByRemediation([ + makeCase({ id: "blank", cause: "blank", impactMs: 0, effort: "hours" }), + makeCase({ id: "big", cause: "big", impactMs: 1900, effort: "hours" }), + ], { at: "2026-08-25T06:00:00.000Z" }); + // Hardest first still does not float the unmeasured finding above the + // measured one it shares a band with. + expect(sortRemediationGroups(sameBand, "effort", lastRun, "desc").map((group) => group.primary.id)) + .toEqual(["big", "blank"]); + }); + + it("shows the same groups in either direction", () => { + const baseline = groups.map((group) => group.key).sort(); + for (const sort of ISSUE_SORTS) { + for (const direction of ["asc", "desc"] as const) { + const sorted = sortRemediationGroups(groups, sort, lastRun, direction); + expect(sorted.map((group) => group.key).sort(), `${sort} ${direction}`).toEqual(baseline); + } + } + }); + + it("declares a direction for every sort", () => { + for (const sort of ISSUE_SORTS) expect(DEFAULT_SORT_DIRECTION[sort]).toMatch(/^(asc|desc)$/); }); it("does not mutate its input", () => { From 21ea33c2475972a18b7234d41de6e51aa3729407 Mon Sep 17 00:00:00 2001 From: Matthew P Munger Date: Fri, 28 Aug 2026 17:06:42 -0500 Subject: [PATCH 8/8] feat(issues): replace the row disclosure with an information tip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The disclosure was correct in the abstract and wrong in this row. The row is itself a link, so the reader had to hit an 18px control inside a target that navigates away if they miss — and the reward for hitting it was the list reflowing under the pointer. A tip that draws over the row costs no layout and no accuracy. A TOGGLETIP rather than a tooltip, because it answers to a click as well as a hover: hover opens while the pointer is on the icon or the panel. click pins it open; clicking again puts it away. Space and Enter are the same event on a real ` + + {/* + The live region is always mounted and the text is put INTO it when the + tip opens, which is what makes a screen reader read it out. A panel that + only appears in the DOM is a panel nothing announces: `aria-expanded` + reports that something opened, and never says what it said. + */} + + {open ? ( + + {text} + + ) : null} + + + ); +} diff --git a/src/components/issue-row.tsx b/src/components/issue-row.tsx index ca06878..183fa5c 100644 --- a/src/components/issue-row.tsx +++ b/src/components/issue-row.tsx @@ -1,7 +1,7 @@ "use client"; import Link from "next/link"; -import { useId, useState, type CSSProperties } from "react"; +import type { CSSProperties } from "react"; import type { IssueCase } from "@/lib/issue-case"; import type { Strategy } from "@/lib/types"; import { EFFORT_LABEL, formatGroupImpact, formatImpact } from "@/lib/impact-format"; @@ -10,6 +10,7 @@ import { causeLineOf, diagnosisLineOf } from "@/lib/case-copy"; import { CONFIDENCE_LABEL } from "@/lib/vocabulary"; import { caseHref, pageHref } from "@/lib/paths"; import { StatusChip } from "@/components/status-chip"; +import { InfoTip } from "@/components/info-tip"; /** * One case, at the depth the list needs: state, diagnosis, scope, confidence, @@ -156,134 +157,101 @@ export function PageScope({ } export function IssueRow({ issue, basePath, pageTitles, nested = false }: IssueRowProps) { - const [open, setOpen] = useState(false); - const panelId = useId(); const impact = formatImpact(issue.impactMs); // What KIND of problem this is, in three or four words — the column a list of // four dozen rows is actually scanned on. const cause = causeLineOf(issue); - // The sentence itself, one disclosure away rather than truncated at the - // column edge. `case-copy` owns the title fallback, because the cause sort - // and this both read it. + // The sentence itself, behind the tip rather than truncated at the column + // edge. `case-copy` owns the title fallback, because the cause sort and this + // both read it. const diagnosis = diagnosisLineOf(issue); // Where the classifier did not recognise the audit, `causeLineOf` already - // fell back to this same sentence — so there is no second layer to open, and - // a control that reveals the text you are looking at is worse than no + // fell back to this same sentence — so there is nothing further to say, and a + // control that shows you the text you are looking at is worse than no // control. The 18px is still reserved, so the labels start on one line down - // the whole list whether or not a row has something to show. + // the whole list whether or not a row has more to give. const hasSecondLayer = diagnosis !== "" && diagnosis !== cause; return ( + /* + The row is no longer one big ``. + + It could not stay one: the pages in it are links to somewhere else, and an + anchor inside an anchor is invalid — the browser closes the outer one and + the row silently becomes two links with a gap between them. The tip's + trigger is a button, which has the same problem. + + So the case link is a normal link on the cause, and `.issue-row__open` + stretches its hit area over the whole row in CSS. One focusable link per + row rather than a row-sized target that reads its six cells aloud, which + is the announcement the UX audit called out; the page links and the tip + sit above it and keep their own targets. + + The background and the rule above it are CSS rather than inline, because + an inline background beats a `:hover` rule in the stylesheet and the row + would never light up. + */
- {/* - The row is no longer one big ``. - - It could not stay one: the pages in it are links to somewhere else, and - an anchor inside an anchor is invalid — the browser closes the outer one - and the row silently becomes two links with a gap between them. The - disclosure has the same problem as a button. + + + - So the case link is a normal link on the cause, and `.issue-row__open` - stretches its hit area over the whole row in CSS. One focusable link per - row rather than a row-sized target that reads its six cells aloud, which - is the announcement the UX audit called out; the page links and the - disclosure sit above it and keep their own targets. - */} -
- - - - - {/* NOT `TRUNCATE_CELL` on this cell or on the link inside it. The - stretched hit area is a positioned `::after` on the link, and - `overflow: hidden` anywhere above it clips that box back to the - cell — which is a whole-row target that only covers one column. - The truncation moves to the span around the text instead, where it - still ellipses and no longer clips anything. */} - - {hasSecondLayer ? ( - - ) : ( - - - - - + {/* NOT `TRUNCATE_CELL` on this cell or on the link inside it. The + stretched hit area is a positioned `::after` on the link, and + `overflow: hidden` anywhere above it clips that box back to the + cell — which is a whole-row target that only covers one column. + The truncation moves to the span around the text instead, where it + still ellipses and no longer clips anything. */} + + {hasSecondLayer ? ( + + ) : ( + - {/* The word, in the row's secondary text token — never a strength hue. - `--confidence-weak` under the word "Confirmed" is a token painting - the opposite of what it says (registry rule 13), and hue here would - double-encode a value the word already carries. Strength as colour - belongs where there is no word to read it from. */} - - {CONFIDENCE_LABEL[issue.confidence]} - + + + - - {impact.text} - + {/* The word, in the row's secondary text token — never a strength hue. + `--confidence-weak` under the word "Confirmed" is a token painting + the opposite of what it says (registry rule 13), and hue here would + double-encode a value the word already carries. Strength as colour + belongs where there is no word to read it from. */} + + {CONFIDENCE_LABEL[issue.confidence]} + - - {EFFORT_LABEL[issue.effort]} - -
+ + {impact.text} + - {/* The second layer. Nothing is hidden that was not already truncated — - the sentence used to end in an ellipsis at the column edge, and now it - ends where it ends. */} - {open && hasSecondLayer ? ( -
- {diagnosis} -
- ) : null} + + {EFFORT_LABEL[issue.effort]} +
); } diff --git a/src/lib/__tests__/info-tip.test.ts b/src/lib/__tests__/info-tip.test.ts new file mode 100644 index 0000000..2d4b554 --- /dev/null +++ b/src/lib/__tests__/info-tip.test.ts @@ -0,0 +1,92 @@ +import { readFileSync } from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; +import { describe, expect, it } from "vitest"; + +const moduleDir = path.dirname(fileURLToPath(import.meta.url)); +const source = readFileSync(path.resolve(moduleDir, "../../components/info-tip.tsx"), "utf8"); +const styles = readFileSync(path.resolve(moduleDir, "../../app/globals.css"), "utf8"); + +/** + * The tip's accessibility contract, asserted from its source. + * + * These are properties of the markup rather than of a render, which is the same + * shape as the object-header tests in `case-applicability.test.ts` and the same + * reason: there is no DOM in this suite, and the things worth protecting here + * are structural. Each one is a way the tip has already been built wrong once, + * or a way the next edit could quietly break it. + */ +describe("the information tip", () => { + it("activates from the keyboard because it is a real button, not because of a key handler", () => { + /** + * Space and Enter on a `