fix(account): answer the account page critique, and let a member wake themselves - #57
Merged
Merged
Conversation
… themselves The critique scored the page 25/40 on Nielsen's heuristics. Six of its findings plus one new control, in the order they matter: - unlink was one click with no undo. It now arms on the first click and only submits on a second, reverting on blur or after 4s. The controls stay in the table row: the row is where the character is, and the e2e spec already pins their rest colour and right edge across both rows. - A narrow viewport clipped the CONTACTS and MAP columns with nothing to say the table scrolled. `.scroller` gains edge fades that track real scroll position. They hang off a new non-scrolling `.scroller-frame`, because an absolutely-positioned child of an overflow box is part of that box's own scrollable content and would ride off with the table. - cryo read as a fault in --signal-warn amber. It is a pause the member asked for, so on their own page it is a neutral token with a sentence saying the tier is kept. The amber stays on the admin table, where cryo is a scanning target rather than a fact about yourself; DESIGN.md now records the split. - New: members can leave cryo themselves. `wakeSelf` is deliberately one-way — self-freeze would let a member dodge the inactivity policy — and keeps the row lock, statusChangedAt, the status.changed audit row with the member as actor, and the account sync enqueue. - The STANDINGS column (per-character contact writes) contradicted LAST PUSHED / Standings (the account's own standing) for blue and green members. The column is now Contacts. - The closing artwork outweighed a one-character account; it takes a smaller frame below that threshold. - Link Discord was the weakest affordance on the page despite being high value. Raised to btn--default, not gold: that is rationed to Add character. Cryo's copy and control fold into the existing Tier dd rather than taking a row of their own. `.facts` is a grid, and a `.visually-hidden` dt is taken out of flow by its own position: absolute, which shifts every dt/dd after it into the wrong track — invisible below 40rem where the grid collapses, and wrong at desktop for exactly the cryo members the feature is for.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 19 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: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (11)
Comment |
…ique-fixes # Conflicts: # src/app/account/page.tsx
This was referenced Aug 4, 2026
guarzo
added a commit
that referenced
this pull request
Aug 4, 2026
…o land Answers a design critique of the admin and member pages. Three priorities, in the order they were agreed, plus the cheap consistency wins. The crew drawer was the worst of it: the nested table fought the row it lived in, and the controls sat below the data they acted on. It is now two sibling rows joined by a colSpan, with the controls above the crew table. The drawer row uses `hidden` rather than unmounting, so an admin's half-typed note survives a toggle. Server actions that threw on recoverable conditions now redirect with an error code the destination page renders — a stale character, a mid-flight demotion, a last-admin demotion, an expired session. What still throws is a genuine bug, and `error.tsx` catches it: digest only, never the message, which can carry a raw DB string. REVOKE, UNLINK and FREEZE arm on the first click and act on the second. One `ConfirmArmScope` per table holds a single armed id, so arming one control disarms any other; Escape, blur, and a four-second timer all revert it. Button width is reserved for the longer label up front — the swap was jittering the row, caught by an e2e geometry assertion rather than by eye. This supersedes the UNLINK-only confirm added in #57. Its blur and revert behaviours are folded into ConfirmSubmit, which adds the shared arm scope, Escape, and per-target accessible names, so unlink-button.tsx is deleted rather than left as a second mechanism. Also: "Your account" on both navs, since "Account" and the admin roster's "Accounts" read as one destination; gold off the audit FILTER, which is not the primary action on that page; scope="col" on the sortable headers; pendingLabel on the side-effecting drawer controls, but deliberately not on the tier buttons, where the label is the value being set; a render stamp on both admin lists, since force-dynamic means the page is only as fresh as the last load; and the closing illustration aligned left at a smaller size, so the page ends on the vertical it started on.
guarzo
added a commit
that referenced
this pull request
Aug 4, 2026
…o land (#59) * fix(ui): rebuild the crew drawer, and give failed actions somewhere to land Answers a design critique of the admin and member pages. Three priorities, in the order they were agreed, plus the cheap consistency wins. The crew drawer was the worst of it: the nested table fought the row it lived in, and the controls sat below the data they acted on. It is now two sibling rows joined by a colSpan, with the controls above the crew table. The drawer row uses `hidden` rather than unmounting, so an admin's half-typed note survives a toggle. Server actions that threw on recoverable conditions now redirect with an error code the destination page renders — a stale character, a mid-flight demotion, a last-admin demotion, an expired session. What still throws is a genuine bug, and `error.tsx` catches it: digest only, never the message, which can carry a raw DB string. REVOKE, UNLINK and FREEZE arm on the first click and act on the second. One `ConfirmArmScope` per table holds a single armed id, so arming one control disarms any other; Escape, blur, and a four-second timer all revert it. Button width is reserved for the longer label up front — the swap was jittering the row, caught by an e2e geometry assertion rather than by eye. This supersedes the UNLINK-only confirm added in #57. Its blur and revert behaviours are folded into ConfirmSubmit, which adds the shared arm scope, Escape, and per-target accessible names, so unlink-button.tsx is deleted rather than left as a second mechanism. Also: "Your account" on both navs, since "Account" and the admin roster's "Accounts" read as one destination; gold off the audit FILTER, which is not the primary action on that page; scope="col" on the sortable headers; pendingLabel on the side-effecting drawer controls, but deliberately not on the tier buttons, where the label is the value being set; a render stamp on both admin lists, since force-dynamic means the page is only as fresh as the last load; and the closing illustration aligned left at a smaller size, so the page ends on the vertical it started on. * refactor(admin): derive the accounts table width from one list of columns Three places depended on the column count — the header row, the empty-state colSpan, and the colgroup span — and only two of them were derived. The third was a literal 9 that would have gone silently wrong the next time someone added a column. FIXED_COLUMNS now names the non-sortable columns in render order; the header maps over it and COLUMN_COUNT falls out of its length. Adding a column is one edit.
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.
The
$impeccable critique src/app/accountpass scored the page 25/40 on Nielsen's heuristics. This closes six of its findings plus one new control.What changed
unlinkwas one click, no undo.scrollerthat track real scroll positionwake meSTANDINGScolumn contradictedLAST PUSHED / StandingsContactsLink Discordwas the weakest affordancebtn--defaultWhere reviewers should focus
wakeSelfis one-way on purpose (src/services/accounts.ts). A member must never be able to freeze their own account — that would let them dodge the corp's inactivity policy. Freezing stays admin-only viasetAccountStatus. The function takes the same row lock as its admin siblings, setsstatusChangedAt, writes astatus.changedaudit row with the member as actor, and enqueues an account sync. It carries no authorization check because there is nothing to authorize: the caller's identity is the target, proven by session at the route layer, not by an actor/target split.unlinkstayed in the table row. An earlier read of the critique moved it behind the row disclosure. It's back in the row: that's where the character is, ande2e/account.spec.ts:95pins the control's rest colour and right edge across both rows. At rest the button is stilltype="submit"with the accessible nameunlink, so that test is untouched and passing.The fades hang off a new
.scroller-frame, not.scroller. An absolutely-positioned child of anoverflow-x: autobox is part of that box's own scrollable content and translates with it — the first attempt scrolled the fade off with the table. Verified by screenshot atscrollLeft: 200..scroller's ownposition: relativeis unchanged; it's load-bearing for containing.visually-hidden.Cryo's copy folds into the existing Tier
dd..factsis a grid, and a.visually-hiddendtis taken out of flow by its ownposition: absolute, so it occupies no track and shifts everydt/ddafter it by one cell. A first pass added a hidden-label row and silently swapped the Discord label and value at desktop — invisible below 40rem where the grid collapses to one column, and wrong for exactly the cryo members the feature exists for. Measured in the browser at 1280px both before and after.The fade sits at 2.55:1, deliberately under WCAG 1.4.11's 3:1. It reinforces an already-visible scrollbar and carries no information of its own. The CSS comment says so rather than claiming compliance. Raising the
color-mixto solid--rule-strongwould clear 3:1 if you'd rather have the heavier edge.Verification
npm test— 374 passed (48 files)npm run test:e2e— 25 passednpm run typecheck— cleanprettier --check,eslint— clean on changed files (pre-existing<img>warnings only)🤖 Generated with Claude Code