feat(design): retune the palette and compose /account - #202
Merged
Conversation
Feedback was that the app reads as machine-made, with the colour scheme named specifically. The old ground was the second reflex, not the first: this design correctly refused cyan-on-black sci-fi, then landed on navy-with-a-gold-accent, which is the most-generated "premium dark tool" palette in circulation. The ground is now a neutral near-black whose tint scales up with lightness, and the tint ramp is the rule rather than an oversight. "Tint every neutral toward the brand hue" only holds for mid and light neutrals; at near-black a warm hue reads as brown. Two warm grounds were built and rejected on exactly that before landing here: navy #080f1f R/B 0.26 warm #1 #0e0906 R/B 2.33 (chroma copied from the navy's 0.035) warm #2 #0c0a08 R/B 1.50 (halved; red-minus-blue only 4, still brown) neutral #0a0a0a R/B 1.00 Red-minus-blue is the wrong statistic at these luminances — a gap of 4 is invisible at rgb(200) and a 50% cast at rgb(12). Judge a ground by its hex, and at near-black by the ratio. Healthy states stop being green. DESIGN.md's status-token rule has always said colour is only for actionable state, and `.st--ok` was where the shipped UI contradicted it: every ok chip, every LINKED, every "all healthy" count rendered full-chroma green. A screenful of green dots reporting that nothing needs doing is the generic-dashboard signature. `--signal-bad` lifts to 0.66 because 0.64 measured 4.42:1 against `--hull-hi` — under the AA floor exactly when the pointer is on the row, the same class of bug as the disabled-opacity one already recorded. The account illustration is recut for a dark ground (lightness -40); at full brightness it was the lightest object on any screen in the app. The same treatment was tried on the 34px header mark and rejected — the gold rope goes out and the disc turns to mud. Artwork modification is cleared by Faoble; the master is in git history. Every text token measured against all three grounds: worst case --ink-faint at 4.63:1 on a hovered row, nothing under 4.5:1.
…differs Two problems on /account, both structural rather than cosmetic. The page had a dead half. `.page` is a 78rem column but nothing in it used more than the manifest's 48rem, so roughly 470px of the column sat empty at 1440px while the page ran long vertically. "Sync schedule" and the closing illustration are material a member reads but does not operate, so they now sit in a rail beside the manifest instead of stacked under it, and the page is substantially shorter. Below 64rem the rail collapses under the manifest, manifest first. `.page` and `--measure-page` are untouched, so the one-column origin — H1 left edge, rule origins, header seal on one vertical across every route — is unchanged. The manifest contradicted its own header. `isNominal` required `onMapAcl`, while this page's own comments and core/account-health.ts both hold that map membership cannot substantiate a fault. So a member on no map was disqualified from the collapsed treatment and every row recited "token ok, standings ok, map off" — underneath a head reading "10 characters — all healthy". `crewNorms` measures deviation against the crew rather than an absolute ideal: a fact every character shares is one fact about the account, stated once in the head, not ten times in the table. Parity holds — the head gained "no characters on the map", so nothing left only one channel — and the table's `<caption>` now says where the fact lives rather than promising it per row. The per-row disclosure drops its visible "actions" caption, which repeated identically on every row and was the loudest pattern in the table. KNOWN DEFECT, recorded in globals.css rather than fixed: that marker now sits alone at the table's right edge, ~490px from the name it expands. Two fixes were built and measured, and both cost more than the defect — leading the row with ACTIONS costs 52px of the 320px forced-scroll budget (134 -> 186 against a 170 tripwire), and making NAME fit-width so ACTIONS takes the slack starves the long-location case (352px wanted, 151px measured). The route that works is moving the control inside the NAME cell, which needs `leadCells` split across page.tsx and character-row.tsx. Specs: four rewritten where `crewNorms` changed what they measure, one rewritten and one added for the rail. The alignment test seeds a payout so two rule heads remain in the main column — without it the count guard had to drop to `> 0` and the "several headings share one edge" half of the claim stopped being tested. `crewNorms` gains unit coverage for the empty, single, uniform and mixed cases.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 21 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (6)
Comment |
Five conflicts, all in the crew manifest, all from #199 landing "let escape back out of the drawer, and drop the column with nothing in it" in the same two files this branch reshapes. Every one resolved as both-sides-keep. Nothing from either branch was dropped: - `character-row.tsx`: main's `onKeyDown={onEscape}` and its conditional ACTIONS `<td>`, plus this branch's icon-only toggle. The two are orthogonal — main changed when the cell renders and how it takes a keypress, this branch changed what the button says. - `<colgroup>` / `<thead>`: main's `showActionsColumn` gate on the trailing `<col>` and `<th>`, plus this branch's comment recording why ACTIONS stays fit-width and trailing. - The manifest `.map` body and both `colSpan` call sites: main's version taken wholesale, since its only code change was `manifestColumns()` gaining a second argument and its comment now explains the lone-survivor case (`applyNoMainRule` leaves a single character with `isMain === false`, so that row keeps a live `make main`). Re-indented one level for this branch's `.account-layout__main` wrapper. `crewNorms` threading survived the largest resolution — verified by grep rather than assumed, since that block was replaced wholesale. Verified on the merge result, not on either parent: npx playwright test 371 passed npm test 1372 passed npx tsc --noEmit clean npx eslint . clean npx prettier --check . clean Counts are up from 358/1329 because main brought its own specs, and they pass against this branch's changes unmodified.
guarzo
enabled auto-merge (squash)
August 10, 2026 03:34
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.
Feedback from a user was that the app "feels heavily AI generated", with the
colour scheme named specifically. This is the answer to that on
/account,which is the surface a member actually visits. Two commits, split so the colour
change is revertable on its own.
Screenshots (desktop + phone), and the measurements behind the palette:
https://claude.ai/code/artifact/f3c1c530-c41d-4eba-a61d-1c28c7a96891
Known defect, shipping deliberately
The per-row disclosure marker is stranded. Each manifest row ends with a
bare
+at the far right of the table, ~490px from the name it expands,because NAME is the elastic column. Ten of them float in an empty column.
It is cosmetic, not functional — a
+at the end of a row is a conventionalaffordance and the control keeps its
aria-label. I originally called this abreach of DESIGN.md ruling R4 and that was overstated: R4 governs facts
living in only one channel, and "there is more here" is stated visually by the
marker itself. The real complaint is that it looks unresolved.
Two fixes were built and reverted, both measurably worse than the defect:
+ [portrait] Name)The route that works is moving the toggle inside the NAME cell and dropping the
ACTIONS column, which needs the
leadCellsprop split acrosspage.tsxandcharacter-row.tsx. Recorded inglobals.cssunder.row-toggle--actionsandin the
<colgroup>comment inpage.tsx, with both measurements, so the deadends are not re-derived. Follow-up work, not a blocker.
1. Palette (
0965bd6)The old ground was the second reflex, not the first. This design correctly
refused cyan-on-black sci-fi, then landed on navy-with-a-gold-accent — the
most-generated "premium dark tool" palette in circulation. That is what the
feedback was naming.
The ground is now a neutral near-black whose tint scales up with lightness, and
the ramp is the rule rather than an oversight. "Tint every neutral toward the
brand hue" only holds for mid and light neutrals; at near-black a warm hue reads
as brown. Two warm grounds were built and rejected on exactly that:
--void#080f1f#0e0906#0c0a08#0a0a0aRed-minus-blue is the wrong statistic at these luminances — a gap of 4 is
invisible at
rgb(200)and a 50% cast atrgb(12). That mistake is why it tookthree passes, and the rule is now written down in DESIGN.md.
Also in this commit:
said colour is only for actionable state;
.st--okwas where the shipped UIcontradicted it. Every
okchip, everyLINKED, every "all healthy" countrendered full-chroma green. A screenful of green dots reporting that nothing
needs doing is the generic-dashboard signature.
--signal-badlifts to 0.66. At 0.64 it measured 4.42:1 against--hull-hi— under the AA floor exactly when the pointer is on the row, thesame class of bug as the disabled-opacity one already recorded in DESIGN.md.
public/brand/hero-account.webpis a modified asset. Recut for a darkground (lightness −40); at full brightness it was the lightest object on any
screen in the app. Modification is cleared by Faoble, and the master is in git
history. The same treatment was tried on the 34px header mark and rejected —
the gold rope goes out and the disc turns to mud.
Every text token was measured against all three grounds rather than asserted:
worst case
--ink-faintat 4.63:1 on a hovered row, nothing under 4.5:1.2. Composition (
aa95d13)The page had a dead half.
.pageis a 78rem column but nothing used morethan the manifest's 48rem, so ~470px sat empty at 1440px while the page ran long
vertically. "Sync schedule" and the closing illustration are material a member
reads but does not operate, so they now sit in a rail beside the manifest.
Collapses to one column below 64rem, manifest first.
.pageand--measure-pageare untouched, so the one-column origin (H1 left edge, ruleorigins, header seal on one vertical across every route) is unchanged.
The manifest contradicted its own header.
isNominalrequiredonMapAcl,while this page's own comments and
core/account-health.tsboth hold that mapmembership cannot substantiate a fault. So a member on no map was disqualified
from the collapsed treatment and every row recited
token ok, standings ok, map off— underneath a head reading "10 characters — all healthy".crewNormsmeasures deviation against the crew rather than an absolute ideal: a fact every
character shares is one fact about the account, said once in the head. Parity
holds — the head gained "no characters on the map", and the table's
<caption>now says where the fact lives rather than promising it per row.
Reviewer focus
crewNormsedge cases: empty crew, single character, mixed, uniform-off withsome rows faulted. Unit-covered in
tests/account-page.test.ts.crewNormschanged what they measure — eachshould still guard its original intent, not just be green. The alignment test
seeds a payout so two rule heads remain in the main column; without it the
count guard had to drop to
> 0and the "several headings share one edge"half of the claim stopped being tested.
.account-layoutescapes the:where()cap on class specificity. Worth alook that it cannot leak to other routes.
Verification
Run against the final state of the branch:
npx playwright test— 358 passednpm test— 1329 passed (83 files)npx tsc --noEmit— cleannpx eslint .— cleannpx prettier --check .— cleanOut of scope, but you will hit it
e2e/provision.ts:95pollspg_isreadyinside the container. Duringinitdbthe Postgres entrypoint runs a temporary server on the unix socketonly, so it reports ready while TCP is not serving; that server then restarts
and the migration connects into the shutdown window, failing with
Connection terminated unexpectedlyonCREATE SCHEMA IF NOT EXISTS "drizzle". It costfour consecutive failed runs here and will look like flake in CI on a loaded
machine. Not touched in this PR.