a11y(admin/accounts): say what unlinking a member's Discord costs - #111
Conversation
The admin Discord cell's `unlink` sits beside an inert `linked` token and disconnects someone else's account, but announced only "unlink Discord for <member>" — nothing about what unlinking does. - `aria-describedby` on the unlink ConfirmSubmit, pointing at a `.visually-hidden` sentence: "Unlinking queues removal of the Discord roles authGD manages for this member." Carried as a description, not folded into the accessible name, which has to keep matching the visible label (WCAG 2.5.3) and is re-spoken on every press. - Hidden always rather than revealed on arm. The account page's `ConfirmCost` (#108) reads scope-level arm state; this table has one tbody-wide ConfirmArmScope over three confirms per row, so arming one control would reveal every row's cost. `.visually-hidden` is `position: absolute`, so the sentence costs no row height. - "queues removal", not "removes" — unlinkDiscord ends in enqueueSync and the roles come off in the worker. - e2e asserts the description at rest and armed: ConfirmSubmit swaps the accessible name on arm while leaving aria-describedby alone, and nothing else in the suite would notice if the id link broke. The plan's `.inline-pair` gap item is already covered by #108 (--s-1 to --s-3, both call sites). The restName/confirmName convention check found the row's revoke and freeze controls already conforming; no change needed.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 14 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 (2)
Comment |
…e scope ConfirmCost read `armedId !== null` — "something in this scope is armed". That is correct in a scope holding one control and silently wrong in every other one, and #111 hit it: the admin accounts table wraps every row in one ConfirmArmScope, so it had to ship its cost sentence hidden always and asked for a per-control id. This is that id. ArmContext now carries the armed control's `describedBy` beside its useId, in one state object so a render can never see the new id with the previous description. A scope may now hold any number of controls and any number of cost sentences; controls sharing a sentence reveal it together. No behaviour change today. Both current call sites are scopes of one, so nothing observable differs, and there is no test: the difference needs a click, and the repo has no jsdom/testing-library — renderToStaticMarkup cannot arm a control. This lands as the footgun removal #111 asked for, not as a fix for a bug anyone can currently reach. The doc also records what #112 found while trying to use it in the admin table, which is the more expensive half of this to rediscover: revealing on arm inside a td widens the cell, the widening moves the armed button out from under a stationary mouse, pointerLeave fires, and the control disarms itself. The admin sentence stays visually-hidden always. Measured, not reasoned about — the first attempt appeared to work only because flex-basis:100% resolved against an inline-flex parent and squeezed the note to 77px instead of its intended 34ch.
…e scope (#112) ConfirmCost read `armedId !== null` — "something in this scope is armed". That is correct in a scope holding one control and silently wrong in every other one, and #111 hit it: the admin accounts table wraps every row in one ConfirmArmScope, so it had to ship its cost sentence hidden always and asked for a per-control id. This is that id. ArmContext now carries the armed control's `describedBy` beside its useId, in one state object so a render can never see the new id with the previous description. A scope may now hold any number of controls and any number of cost sentences; controls sharing a sentence reveal it together. No behaviour change today. Both current call sites are scopes of one, so nothing observable differs, and there is no test: the difference needs a click, and the repo has no jsdom/testing-library — renderToStaticMarkup cannot arm a control. This lands as the footgun removal #111 asked for, not as a fix for a bug anyone can currently reach. The doc also records what #112 found while trying to use it in the admin table, which is the more expensive half of this to rediscover: revealing on arm inside a td widens the cell, the widening moves the armed button out from under a stationary mouse, pointerLeave fires, and the control disarms itself. The admin sentence stays visually-hidden always. Measured, not reasoned about — the first attempt appeared to work only because flex-basis:100% resolved against an inline-flex parent and squeezed the note to 77px instead of its intended 34ch.
1 participant
Executes the three scope items in
docs/superpowers/plans/2026-08-05-admin-discord-unlink-adjacency.md. Rebased on #108 — one of the three turned out to be already done there.What changed
The admin Discord cell's
unlinksits beside an inertlinkedtoken and disconnects someone else's account, but announced onlyunlink Discord for <member>. Nothing about what unlinking does.aria-describedbyon the unlinkConfirmSubmit, pointing at a.visually-hiddensentence: "Unlinking queues removal of the Discord roles authGD manages for this member." Carried as a description, not folded into the accessible name — the name is spoken ahead of every press and has to keep matching the visible label (WCAG 2.5.3).unlinkDiscordends inenqueueSync(services/discord-link.ts:135-138); the roles come off in the worker. Same verb a11y(account): separate the unlink control from the LINKED it sits beside #108 chose on the account page, for the same reason.ConfirmSubmitswapsaria-labelon arm (confirm-submit.tsx:139) while leavingaria-describedbyalone (:140) — the description has to survive that swap, and nothing else in the suite would notice if the id link broke.Plan items that produced no code
.inline-pairgap (--s-1→--s-3). Already landed in #108. It has exactly two call sites, soadmin/accounts/page.tsxwas covered for free. I had this change and dropped it rather than duplicate-and-conflict.restName/confirmNameconvention across the row's destructive controls. Checked; already conforming.revokeandfreezeboth name their row the same wayunlinkdoes. No change.Why hidden always, and not
ConfirmCost#108's
ConfirmCostreveals the consequence when the action arms. It reads scope-level arm state, and this table has one tbody-wideConfirmArmScope(page.tsx:282) over three confirms per row — unlink, revoke/promote, freeze/wake — across every row. Arming any one control would reveal every row's cost.ConfirmCost's own doc says it is correct only in a scope holding oneConfirmSubmit. When it takes a per-control id, this cell should switch to it; giving this one cell its ownConfirmArmScopeto get there early would break the single-armed-control-per-table invariant the shared scope exists to hold.Reveal-on-arm is also wrong for a table row on its own terms: the hint reflows the row, and
ConfirmSubmitdisarms onpointerleavefor mouse users (confirm-submit.tsx:162-170) — the button sliding out from under a stationary pointer would disarm the control the admin just armed. Absolute positioning is no escape, since.scrolleris a scroll container and the hint would extend the scrollable area rather than float clear of it..visually-hiddenis itselfposition: absolute, so the sentence costs no row height — which is the point, given this renders once per account and scanning is the primary act (PRODUCT.md principle 3).Verification
npm run format:check— All matched files use Prettier code style!npm run typecheck— cleannpm run lint— 0 errors, 1 pre-existing warning (ui.tsx:109,<img>, untouched)npm test— 74 files, 957 passednpm run test:e2e -- admin.spec.ts— 41 passedThe new e2e assertion was mutation-tested: renaming the span's
idproduces a genuine failure (Received: ""), not a silent pass.Out of scope per the plan: making
linkedclickable, any modal/tooltip, and anything insrc/app/account/.