fix(ui): everything the cross-discipline interface review found - #56
Draft
samishal1998 wants to merge 2 commits into
Draft
fix(ui): everything the cross-discipline interface review found#56samishal1998 wants to merge 2 commits into
samishal1998 wants to merge 2 commits into
Conversation
15 findings, 13 of them WCAG escalation triggers. Each was verified in a real
browser before and after — headless Chrome over CDP against a live pstack.
Contrast (measured, not eyeballed):
- --fg-mute failed 4.5:1 on every surface it renders on (light 3.22-3.64, dark
4.09-4.47). It carries .mute, .hint, table headers, placeholders and log
timestamps. Now 4.70 light / 4.81 dark at the worst surface.
- White on --accent was 2.78:1 in dark, the DEFAULT theme, on the primary
button, the checkbox tick and the radio dot. The fill's gradient was the real
culprit: it mixed white into the ink's own background, so no accent value
survived it. Ink is now --accent-fg, and each theme anchors the gradient away
from its ink — dark lifts the top stop, light drops the bottom — so both stay
"lit from above" at 6.81 and 6.50.
- Six hard-coded log service hues were a dark-only set measuring 1.72-2.65:1 on
a light panel, under a comment claiming both themes. Tokenised per theme.
Keyboard:
- `button.ghost { box-shadow: none }` outranked `:focus-visible` and deleted the
focus ring on every ghost button — which is what dense rows and tables use.
Same root cause on .clist-item and .palette-row. Zero focusable elements now
match :focus-visible without a ring, measured with real Tab events.
- Three hand-rolled dialogs declared aria-modal="true" with none of its
behaviour: opening the shortcut sheet left focus on <body>, and three Tabs put
you on a nav link behind it. They now use reka-ui, already this app's idiom.
Focus enters, is trapped, and returns to the trigger.
- In the command palette, Tab moved DOM focus while the highlight stayed put, so
Enter activated a row the user could not see was selected.
320px:
- The mobile nav put 11 of its 15 destinations outside a fixed 304px bar with no
way to scroll to them: space-around centres negative free space, so both ends
fell off, and the rail is fixed so body scroll could not reach them either.
- .grid-2's minmax(320px, 1fr) floor exceeded its own container below a 368px
viewport, so panels ran 24px past the edge and stat tiles clipped.
- Card-mode row actions could not wrap and pushed off the leading edge.
Copy:
- One error banner served four operations while asserting a single cause, so a
network failure read as a Traefik rejection and an ACCEPTED action read as
refused.
- Five wordings for a clipboard failure, four of them dead ends, while the value
sat on screen selectable the whole time.
- The same "add a row" operation had five button verbs and four toast verbs.
- Two destructive row actions had no confirmation, against ui-rules.md:90.
- A container name truncated with no way to reach the full swarm task id.
Control heights:
- `min-height: 32px` could never bind (border-box: 20.25 line + 16 padding + 2
border = 38.25), so every input sat 4px taller than the button beside it and
two separate patches existed to hide it. Pinning single-line controls to
--control-h surfaced the cause: ten <input> elements carry no type attribute,
which `input[type='text']` never matches, so they had no border, no radius, no
focus treatment and a 22.5px browser default. The selector lists now include
`input:not([type])`.
The first commit took the 13 escalation triggers. This is everything else the
six domain reviews found, ~35 items, plus the convention doc that was teaching
two of them.
Accessibility:
- ActionButton put the reason a control is blocked in a `title`, on a natively
disabled button. `disabled` leaves the tab order, so "The only admin — promote
another account first." was mouse-only. A blocked button is now aria-disabled
(focusable), blocks its own click, and carries the reason via
aria-describedby. Verified: focusable, not natively disabled, reason exposed.
- Five sites inserted error text with no live region, so a failed sign-in was
silent. role="alert" for untied failures; id + aria-describedby + aria-invalid
for field-tied validation.
- `display: block` in card mode strips table semantics, and CSS cannot restore
what display removed — 18 tables now assert their roles in markup.
- role="tab" promised an APG keyboard contract nothing implemented (they are two
toggles: aria-pressed). An unreferenced role="tooltip" did nothing. Both gone.
- The log viewer got tabindex="0" — Safari does not make scrollers focusable —
and an accessible name it never had.
- Every target now clears 24x24; the InfoHint keeps its 18px look via an
::after overlay.
Contrast and colour:
- A form control had no 3:1 edge in either theme: --line was 1.14-1.43:1 against
its own fill. tokens.css's "the shadow carries the edge" note covers panels; an
input gets an INSET shadow, which draws no outer edge. New --line-control.
- Six hard-coded Tokyo-Night log hues, tokenised per theme.
- `.stat[data-zero]` and `.stat.busy .v` have equal specificity, so the later
rule won and a zero BUSY count rendered in the accent — the "look here" colour,
pointed at nothing.
Typography:
- A second, UNSCOPED `kbd` rule sat under .palette-foot and beat the real keycap
rule on every <kbd> in the app, at a raw 11px — below the file's stated 12px
floor, and it made the earlier rule dead. Scoped.
- --font-mono is referenced twice and defined NOWHERE (the token is --mono), so
both sites fell through to a hand-copied stack missing three families.
- --measure was defined, documented as enforced, and used zero times; prose ran
the full window. `code`/`.mono`'s 0.92em compounded to 11.04px on a --t-xs host.
- The `font:` shorthand resets font-variant-numeric, so a polled restart count
lost tabular figures. Tracking and line-height literals now use their tokens.
- 16px inputs under 560px: iOS Safari zooms the page below that.
Layout and polish:
- The .banner drew status twice — a 3px coloured left rule AND a tinted
background. The file's own comment on .toast calls that rule "a 2010 alert box"
and removed it there.
- ~440px of tabs sat off-screen at 320px with the scrollbar hidden, so there was
no cue at all.
- Six height-mismatch patches (align-self/align-items/raw px) deleted: the
control-height fix in the previous commit made them obsolete.
- RefreshButton rendered `sm` beside full-size controls, which ui-rules.md
forbids verbatim.
- Skeletons reserved 58px for a ~143px table, so the layout jumped on load.
- At 320px three stat tiles gave ~50px of content and every label elided —
the exact failure the .stats comment rejects for the 6-column case. 2x3 there.
- Three byte-identical hand-rolled magnifier SVGs replaced by the lucide icon
already in the dependency tree.
Copy:
- Four ErrorNote titles were bare nouns or "Something went wrong."
- 14 hand-written badges were lowercase while sentence() renders every API enum
in sentence case — drift visible inside a single table row.
- Three filtered-to-empty tables were dead ends; they now clear their filters.
- Three buttons dismissing one shown-once panel had three labels, one of them a
first-person assertion the button cannot verify ("I have stored it").
- A 36-word explanation rendered once PER unresolved row.
docs/ui-rules.md gains the accessibility section it never had — the drift above
had no rule to violate — and loses the one rule that mandated an anti-pattern
("A disabled control carries `title` saying why").
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.
Stacked on #55. Two commits: the 13 WCAG escalation triggers, then the ~35 MEDIUM/LOW items and the convention doc that was teaching two of them.
Every finding was verified in a real browser before and after — headless Chrome over CDP against a live
pstack servewith seeded data. Numbers below are measured, not estimated.:focus-visiblewith no ringscrollWidth344 vs 320bun run checkgreen 14/14 on a forced uncached run.Commit 1 — the blockers
Contrast.
--fg-mutefailed 4.5:1 on every surface it renders on (light 3.22–3.64, dark 4.09–4.47) while carrying.mute,.hint, table headers, placeholders and log timestamps. White on--accentwas 2.78:1 in dark, the default theme, on the primary button, the checkbox tick and the radio dot — and the fill's gradient was the real culprit, mixing white into the ink's own background so no accent value could survive it. Ink is now--accent-fg, and each theme anchors the gradient away from its ink (dark lifts the top stop, light drops the bottom), so both keep "lit from above" at 6.81 and 6.50.Keyboard.
button.ghost { box-shadow: none }outranked:focus-visibleand deleted the focus ring on every ghost button — the styleui-rules.mdprescribes for dense rows and tables. Same root cause on.clist-itemand.palette-row. Three hand-rolled dialogs declaredaria-modal="true"with none of its behaviour: opening the shortcut sheet left focus on<body>, and three Tabs put you on a nav link behind it. They now usereka-ui, already this app's idiom. In the command palette, Tab moved DOM focus while the highlight stayed put, so Enter activated a row the user could not see was selected.320px. The mobile nav put 11 of its 15 destinations outside a fixed 304px bar with no way to scroll to them —
space-aroundcentres negative free space, and the rail isposition: fixed, so body scroll could not reach them either..grid-2'sminmax(320px, 1fr)floor exceeded its own container below a 368px viewport.Control heights.
min-height: 32pxcould never bind (border-box: 20.25 line + 16 padding + 2 border = 38.25), so every input sat 4px taller than the button beside it, with two separate patches hiding it. Pinning single-line controls to--control-hsurfaced the cause: ten<input>elements carry notypeattribute, whichinput[type='text']never matches, so they had no border, no radius, no focus treatment and a 22.5px browser default.Commit 2 — the rest
ActionButtonput the reason a control is blocked in atitleon a nativelydisabledbutton;disabledleaves the tab order, so "The only admin — promote another account first." was mouse-only. Nowaria-disabled, focusable, reason exposed viaaria-describedby.Also: five error sites with no live region (a failed sign-in was silent to assistive tech); 18 tables asserting the roles
display: blockstrips in card mode; a form control with no 3:1 edge in either theme; a second unscopedkbdrule that beat the real keycap style on every<kbd>at a raw 11px;--font-monoreferenced twice and defined nowhere;--measuredefined, documented as enforced, and used zero times;.bannerdrawing status twice with a rule the file's own comment calls "a 2010 alert box"; and 14 lowercase badges againstsentence()'s casing.docs/ui-rules.mdgains the accessibility section it never had — several findings existed because there was no rule to violate — and loses the one rule that mandated an anti-pattern.Reviewer notes
--accentdarkened in light theme (#1f6feb→#1257c9), which also fixed accent-as-text failing at 4.09:1.apps/uiplus the convention doc, so no conformance or golden impact.