fix(ui): four paper cuts — filter hints, status baseline, drawer width - #193
Merged
Conversation
- payouts: drop the Name filter hint. The label already answers it, and the one hint-bearing cell made the filter row read two lines deep on the left and one on the right. - admin/audit: reword the Action hint to "what happened, like tier.changed" so all three hints read in the same voice as Actor and Target. - globals.css: hold .st's marker box at a constant 0.5em square in every tone and paint neutral's thin bar with a gradient instead of shrinking the box. An inline-flex box synthesizes its baseline from its first flex item (Flexbox 8.5), so the shorter neutral marker sat the whole token 1.4px below its neighbours in the same row. - admin/sync: give each job's run-history drawer a flush right edge with a sibling filler div inside a flex .scroller, rather than a table column. All three drawers now measure 1164px inside a 1198px panel; previously 721 / 803 / 704.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 52 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 selected for processing (5)
Comment |
guarzo
enabled auto-merge (squash)
August 9, 2026 20:07
guarzo
added a commit
that referenced
this pull request
Aug 9, 2026
…t status line Follow-up to #193, from the same critique. - admin/accounts: the token badge said "0/8 ok" in the exact case the badge turns red. A red marker and the word OK argued with each other in the one cell built to be glanced at; "healthy" is a word the numerator can be zero of, where "ok" is a verdict and reads as one no matter what precedes it. - admin/accounts: REVOKE drops to .btn--danger-quiet at rest and keeps full .btn--danger on arm, matching FREEZE and UNLINK. globals.css already records why: full --danger on a per-row control "made it the most saturated thing on the account page, permanently, which reads as a warning against an ordinary choice". Every row of an admin table can be an admin, and resting red as furniture spends the alarm channel the token badge two cells left needs when an account actually goes dark. - account: the per-row status summary renders only where a character deviates from nominal. For a managed, contacts-ok, on-ACL character the sentence can only be "token ok, standings ok, map on" — the same string on every row, under a heading that has already said all of them are healthy. The R4 parity fix it was added for is kept exactly where it was earned: a row whose map or standings state differs still says so, and the gate is on the element, so both channels drop it together. The <caption> states the rule so an unannotated row is readable as nominal rather than uncomputed. Two new e2e cases cover the gate, which no existing test reached: seedNominalCrew never seeds wandererAclObservation, so all ten of its characters are `map off` and none is nominal. The existing counts are unchanged for that reason, and their regexes are tightened from `map (on|off)` to `map off` to pin the precondition they now depend on. Not done, and why: putting the object in the Discord unlink's visible label ("unlink Discord") was tried and reverted. It widens the button ~64px, which at ~700px pushes the arming live region off the line; the line box grows on arm, align-items: center re-centres the button out from under a stationary pointer, and the pointerLeave disarms it. That is the #112 mechanism, caught by "arming the Discord unlink does not move it out from under the pointer". The twin-UNLINK finding was also weaker than it read: the two controls already differ in grade, size and accessible name, and only share the visible word. Reasoning left in a comment at the call site. Verification: typecheck, lint, format:check clean; 83 files / 1324 unit tests pass; e2e/account.spec.ts 71 passed.
guarzo
added a commit
that referenced
this pull request
Aug 9, 2026
…t status line (#194) Follow-up to #193, from the same critique. - admin/accounts: the token badge said "0/8 ok" in the exact case the badge turns red. A red marker and the word OK argued with each other in the one cell built to be glanced at; "healthy" is a word the numerator can be zero of, where "ok" is a verdict and reads as one no matter what precedes it. - admin/accounts: REVOKE drops to .btn--danger-quiet at rest and keeps full .btn--danger on arm, matching FREEZE and UNLINK. globals.css already records why: full --danger on a per-row control "made it the most saturated thing on the account page, permanently, which reads as a warning against an ordinary choice". Every row of an admin table can be an admin, and resting red as furniture spends the alarm channel the token badge two cells left needs when an account actually goes dark. - account: the per-row status summary renders only where a character deviates from nominal. For a managed, contacts-ok, on-ACL character the sentence can only be "token ok, standings ok, map on" — the same string on every row, under a heading that has already said all of them are healthy. The R4 parity fix it was added for is kept exactly where it was earned: a row whose map or standings state differs still says so, and the gate is on the element, so both channels drop it together. The <caption> states the rule so an unannotated row is readable as nominal rather than uncomputed. Two new e2e cases cover the gate, which no existing test reached: seedNominalCrew never seeds wandererAclObservation, so all ten of its characters are `map off` and none is nominal. The existing counts are unchanged for that reason, and their regexes are tightened from `map (on|off)` to `map off` to pin the precondition they now depend on. Not done, and why: putting the object in the Discord unlink's visible label ("unlink Discord") was tried and reverted. It widens the button ~64px, which at ~700px pushes the arming live region off the line; the line box grows on arm, align-items: center re-centres the button out from under a stationary pointer, and the pointerLeave disarms it. That is the #112 mechanism, caught by "arming the Discord unlink does not move it out from under the pointer". The twin-UNLINK finding was also weaker than it read: the two controls already differ in grade, size and accessible name, and only share the visible word. Reasoning left in a comment at the call site. Verification: typecheck, lint, format:check clean; 83 files / 1324 unit tests pass; e2e/account.spec.ts 71 passed.
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.
Four paper cuts reported from a pass over the live app.
Operations — the Name filter hint was redundant
The label already answers what the field does. Audit's three fields each keep a hint because Actor / Action / Target are three ways of narrowing the same row; here there is only one field and the label is the answer. Removing it also fixes the ragged filter row, which read two lines deep on the left and one on the right.
Audit — the Action hint now matches its neighbours
matches the start of an action, like tier.changedbecomeswhat happened, like tier.changed, so the three read in one voice: who did it / what happened / who it happened to. The shorter string also stops wrapping to two lines.Trade-off: the old copy explicitly taught prefix matching and the new copy does not. Discoverability now rests on the field's
<datalist>, which offers bare namespaces (tier.,payout.). Easy to put the instruction back if that reads as a loss.Admin accounts — the neutral status token sat low
.st's marker box is now a constant0.5emsquare in every tone, with neutral painting its thin bar via a gradient inside that box instead of shrinking the box itself. Aninline-flexbox with no baseline of its own synthesizes one from its first flex item's bottom margin edge (CSS Flexbox §8.5), so the shorter neutral marker was dragging the whole token down.Measured on the first row of
/admin/accounts:active/none/0/1admin(neutral)top: 477.015625, marker5.5pxtop: 478.421875, marker1.64pxtop: 477.015625, marker5.5pxtop: 477.015625, marker5.5pxSync — the run-history drawers were ragged
Each job's drawer stopped at its own content width. It now reaches the panel's right edge via a sibling filler div inside a flex
.scroller.The remaining 34px is the strip's own padding. Confirmed flush at 1920px, 1280px, 320px and 390×844.
Why not a trailing spacer column. That was tried first and reverted: under
table-layout: auto, a percentage column width resolves against the table itself and re-resolves on every reflow, so the spacer kept re-fighting the other columns. Opening a run's Raw disclosure got squeezed and the row blew out to 785px against an e2e ceiling of 140px, confirmed as caused by the change via an A/B against unmodified code. The sibling div leaves the table's own column sizing completely untouched, andflex-shrink: 0keeps the44remfloor and the narrow-viewport horizontal scroll intact.Verification
Left alone deliberately
Out of scope for a paper-cut pass, flagged rather than swept:
docs/design-walkthrough.md:271recommends the old audit hint string. It is a dated review record, not live guidance.src/app/globals.css~3040 and ~3091 (the latter cites a "~58px wider" measurement for a string that no longer exists), ande2e/audit.spec.ts:311.Reviewers
The CSS is where the risk is.
.st::beforeis shared by every status token in the app, and.scroller:has(.log--runs) { display: flex }changes the formatting context of the sync drawer's scroll container.