Post-refactor UI: a scannable Issues list, sortable columns, and one repair - #98
Open
matthewpmunger wants to merge 8 commits into
Open
Post-refactor UI: a scannable Issues list, sortable columns, and one repair#98matthewpmunger wants to merge 8 commits into
matthewpmunger wants to merge 8 commits into
Conversation
`{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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
…e title
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 <noreply@anthropic.com>
`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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
…the columns 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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
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 `<button>`, so the keyboard gets this from the
platform rather than from a key handler that has to be kept in step
with it. Nothing here calls `preventDefault`, which is the one way to
break that.
Focus alone does not open it, which is the toggletip half of the pattern: a
keyboard reader tabbing a list of these would otherwise have a panel over the
row at every stop, and the panel is not where they are going. Space is the ask,
`aria-expanded` is the answer.
WCAG 1.4.13 wants three things of hover content, and each is a line here rather
than an accident. Dismissible: Escape closes and focus does not move.
Hoverable: the panel is `position: fixed`, so the pointer's trip from the icon
crosses elements belonging to neither and `pointerleave` fires on the way —
hence the close delay, without which you can see the text and cannot put the
pointer on it. Persistent: nothing closes it on a timer.
The text is mounted inside a `role="status"` that is always present, because
`aria-expanded` reports that something opened and never says what it said, and
a live region created at the same moment as its content is one nothing was
watching yet.
Fixed positioning and z-index 120 follow `.select-menu__popover`, for its
reason: an absolutely positioned panel is clipped by whichever ancestor
scrolls, and in a list that ancestor is the row.
WIDTH. There was no measure to inherit — this is the app's first tooltip, and
the stylesheet's only measure was `68ch` for running prose, three times this
and meant for a paragraph that owns its column. `--info-tip-max-width: 20rem`,
named so the next tip does not pick its own number.
The row wrapper introduced for the old expansion is gone with it, and the row's
background moved from an inline style into `.issue-row` — inline beat the
`:hover` rule in the stylesheet, so the row could never light up.
Tested from the source, in the shape the object-header tests already use: that
the trigger is a real button, that no key is swallowed, that Escape moves no
focus, that the panel sits inside the live region rather than beside it, and
that `aria-controls` is dropped when the panel is not there — a dangling IDREF
being a broken relationship rather than an empty one.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Base commit
98177fc85db31ddc38fa7b12b76bb5e99d51462a("R4/S10: registry v11, /tasks redirect, empty allowlist, drop task producer (R4/S10: registry v11, /tasks redirect, empty allowlist, drop task producer #97)")npm run lint,npm run typecheck,npm test -- --runandnpm run buildall exit 0.Reported honestly rather than tidily: these were run against the base after
the work, not before it. I started this session on the branch without checking
the base first, which is the one thing AGENTS.md asks for up front. The base
turned out to be green, so nothing here is built on top of a red commit — but
that was luck rather than process, and a reviewer should know which it was.
What & why
Post-refactor UI work on the Issues list, the case detail and the object
header. Everything here came out of driving the app after the S1–S10 series
landed; each item is either a defect the refactor left behind or a place the
list stopped serving the job it exists for.
The through-line on the Issues list: it is read by scanning, and it was not
scannable. The primary column held a full sentence, the columns did not line up
row to row, and the only way to reorder was a menu of four options, two of
which headed no column.
Changes
repair: the Ora note's dropped space (5e6da97) — claimed inREPAIRS.md, own commit. The recorded cause was wrong and is corrected inthe ledger: JSX does not drop a leading space on the first line of a text
node. What drops it is an HTML entity elsewhere in the same text node
(
Ora's, two lines below). Proven with probe routes against this app'stoolchain; 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.
e6d2bb9) — only pages thestore still knows; an unknown id stays plain text rather than linking to a
404.
c51f7fe) —both contradicted the file's stated reading order, so the doc-comment moved
with the code.
1635016).Scope→Pages(db615dc) — four new sorts.stateandconfidenceindexWORK_STATES/CONFIDENCESrather thanrestating their order. Labels come from
ISSUE_SORT_LABEL, so a column andthe Sort menu cannot call one ordering two things.
8073ab7) — see below.670be4d).21ea33c).Three of those need more than a line:
Columns that line up.
ISSUE_ROW_COLUMNSis shared by the header, thegroup headers and every row so that the columns line up — but each row is its
own grid, and
max-contentresolves per grid. Every row was therefore sized toITS content: effort measured 29px on a row reading "Days" and 71px on one
reading "No estimate", state 72px against 82px, and the two
frcolumnsinherited the difference. 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 its own content. Confidence is sized by its header,
because "CONFIDENCE" is wider than "Probable".
The row is no longer one big
<Link>. It could not stay one: the pages init are links elsewhere, and an anchor inside an anchor is invalid — the browser
closes the outer one and the row silently becomes two links with a gap. The
case link is now a normal link on the cause and stretches its hit area over the
row in CSS, so 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 onto a span inside it:
overflow: hiddenclips the link's own stretched::afterback to one column, which looks identical and is not.Direction and rule 18. Rule 18 had to come out of
byWorstMeasuredforimpact, into a
measuredFirstthat sits outside the sign. Reversing "largestsaving 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. Tested in both directions for impact and effort.
Verification
npm run buildpassesnpm run lintpassesnpm run typecheckpassesnpm test -- --runpasses — 107 files, 1393 tests (12 added)REPAIRS.mdand kept in itsown commit
Driven in the browser: the Ora note renders its space; an affected-page row
navigates to
/pages/hosting(HTTP 200,h1"Hosting"); header actions sit ina row with metadata beneath the title; every column sorts and reverses, with
the header and the Sort menu staying in step; page links navigate without the
stretched row link swallowing them; a click in the Confidence column still
opens the case; the tip opens on hover and on click, closes on Escape, and
reports
aria-expanded, a matchingaria-controls,position: fixedandmax-width: 320px.One gap, stated rather than papered over: I could not confirm Space and
Enter with real key presses — the browser harness's synthetic key events do not
reach the focused element (Enter did not fire either). Activation therefore
rests on the platform guarantee: a real
<button type="button">, with nothingin the component calling
preventDefault.src/lib/__tests__/info-tip.test.tspins that contract from the source. Worth a manual Space/Enter check during
review.
Notes
The
repairlabel is deliberately not applied.5e6da97is a claimedrepair in its own commit, as AGENTS.md requires, but labelling this PR
repairwould waive
base-branch-greenfor a large feature diff, and the label is "aclaim in
REPAIRS.md, not a way past the gate". If you would rather the repairwent in on its own, say so and I will split it out — it touches only
settings/page.tsxandREPAIRS.md.Judgement calls a reviewer may want to reverse:
Pagesstill under-describes its column. The cell rendersscopeLineOf,which is pages and devices ("Hosting · Mobile, Desktop").
pageScopeOfalready exists and returns pages only; switching the cell to it makes the
header exactly true at the cost of dropping the device clause from the list.
That is a product call, not a rename, so the cell was left alone.
culpritLabelhas 27 entries andanswers
otherfor the rest — labelled "Something else the nightly testfound", which says less than the sentence it would replace. Those rows keep
their diagnosis (and get no tip, since there is nothing further to show).
Four of seven demo rows get a short label, three keep a sentence. The payoff
only fully arrives when the catalogue covers the CrUX and agent audits, which
is copy to be written rather than code.
aria-expandedis the answer. Otherwise a keyboard reader tabbing the listgets a panel over the row at every stop.
68chfor running prose, ~3× too wide.--info-tip-max-width: 20rem, namedso the next tip does not pick its own number.
670be4dreverses a decision made indb615dcwithin this branch — theearlier commit argued against a direction toggle, and this one adds it at the
author's request. Left as two commits rather than squashed, so the reasoning
on both sides survives.
Deferred:
docs/page-watch-ux-audit-2026-08-21.md's contrast finding is stale(the colour migration fixed it; the tokens now carry measured ratios), and the
two other open rows in
REPAIRS.mdare untouched — neither is UI, and theledger is explicit that one commit fixes one defect.
🤖 Generated with Claude Code