polish(console): a11y, finality lifecycle, loading, disabled-state, responsive#54
Conversation
…tatus, double-submit guards
📝 WalkthroughWalkthroughThe console updates interactive controls with visible keyboard focus, adds keyboard activation for clickable table rows, improves notification labels, and exposes loading states for auditor packet and attestation generation. ChangesConsole interaction polish
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/console/src/ui/controls.tsx`:
- Line 233: The copy control’s 24px hit area is below the required minimum. In
the copy button near the `className` using `h-6 w-6`, change the button
dimensions to at least `h-10 w-10` while preserving the icon’s smaller size.
- Around line 143-154: Update the Tr keyboard handling to preserve its row
behavior: destructure the caller-provided onKeyDown and tabIndex from rest,
invoke the caller handler while retaining the row handler, spread remaining
props before controlled values, and set the computed tabIndex afterward. In the
row handler, only process Enter or Space when event.target ===
event.currentTarget so bubbled events from nested controls are ignored.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2440f0c3-5374-4794-b2fb-98030122e9a2
📒 Files selected for processing (10)
apps/console/src/app/NetworkMenu.tsxapps/console/src/app/Shell.tsxapps/console/src/screens/AuditLog.tsxapps/console/src/screens/Contractors.tsxapps/console/src/screens/Grants.tsxapps/console/src/screens/Pay.tsxapps/console/src/screens/Payroll.tsxapps/console/src/screens/Treasury.tsxapps/console/src/ui/controls.tsxapps/console/src/ui/onchain.tsx
| onKeyDown={ | ||
| onClick | ||
| ? (e) => { | ||
| if (e.key === "Enter" || e.key === " ") { | ||
| e.preventDefault(); | ||
| onClick(); | ||
| } | ||
| } | ||
| : undefined | ||
| } | ||
| className={`${onClick ? "cursor-pointer outline-none hover:bg-border/30 focus-visible:bg-border/30 focus-visible:ring-2 focus-visible:ring-inset focus-visible:ring-primary/40" : ""} ${className}`} | ||
| {...rest} |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Preserve the row keyboard contract and ignore nested-control events.
Because {...rest} is spread after this handler, a caller-provided onKeyDown or tabIndex can overwrite Tr’s computed behavior. AuditLog.tsx supplies both, and its row handler also receives bubbled keys from nested copy and receipt controls. Destructure/combine the caller handler, keep controlled props after the spread, and only activate the row when event.target === event.currentTarget.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/console/src/ui/controls.tsx` around lines 143 - 154, Update the Tr
keyboard handling to preserve its row behavior: destructure the caller-provided
onKeyDown and tabIndex from rest, invoke the caller handler while retaining the
row handler, spread remaining props before controlled values, and set the
computed tabIndex afterward. In the row handler, only process Enter or Space
when event.target === event.currentTarget so bubbled events from nested controls
are ignored.
| setTimeout(() => setDone("idle"), 1200); | ||
| }} | ||
| className="rounded p-1 text-muted outline-none transition hover:bg-border/50 focus-visible:ring-2 focus-visible:ring-primary/40" | ||
| className="inline-flex h-6 w-6 items-center justify-center rounded text-muted outline-none transition hover:bg-border/50 focus-visible:ring-2 focus-visible:ring-primary/40" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Increase the copy control hit area to the required minimum.
h-6 w-6 renders a 24×24px target, conflicting with the PR objective requiring interactive targets of at least 40px. Keep the icon small, but make the button itself at least h-10 w-10.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@apps/console/src/ui/controls.tsx` at line 233, The copy control’s 24px hit
area is below the required minimum. In the copy button near the `className`
using `h-6 w-6`, change the button dimensions to at least `h-10 w-10` while
preserving the icon’s smaller size.
…Tr gets role=button (accessible name from cell text) and ignores clicks/keys from nested controls (buttons/links/fields keep their own action); bump inline copy control to a 28px target
|
Greptile/CodeRabbit triage (fixed): the clickable |
1 participant
Closes #40. A focused accessibility + polish sweep across the console — reuses the merged primitives, no screen redesigns. Audited every screen (Dashboard, Approvals, Contractors, Invoices, Pay, Payroll, Treasury, Grants, AuditLog, Settings, InviteClaim) plus the shell chrome; most bullets were already satisfied by the merged foundation, so this closes the remaining concrete gaps.
Keyboard-operable row actions — the shared
Trprimitive now makes any clickable row focusable (tabindex), activatable with Enter/Space, and shows an inset focus ring; non-interactive rows stay out of the tab order (covers the Audit-log detail-drawer rows and any future clickable table).Visible focus rings on the remaining raw controls that only had
hover:styles: the sharedonchain.tsxproof drill-down (trigger, close, verifier + transaction links — ripples to every proof receipt), Shell workspace/notification menu items, NetworkMenu explorer link, and the explorer/receipt links + Payslips/Export buttons in Pay, Payroll, Contractors, Treasury, and AuditLog.Non-color status signal — the notification bell's pending-approvals state was a color-only dot; the button now carries a textual
aria-label/titlewith the count ("Notifications — N awaiting approval").Prevent double-submit while a tx is pending — the two full-screen ceremony triggers (Grants "Generate attestation", AuditLog "Generate auditor packet") now pass
loadingtied to their ceremony phase so they can't be re-fired while a proof is in flight (all other submit/proof buttons already gate onloading).Target size — the inline
CopyButtonis bumped to a 24×24 hit area (WCAG 2.5.8 minimum); primary buttons are already 40/44px.Already in place and verified during the audit (left as-is):
prefers-reduced-motionhonored globally viaMotionConfig reducedMotion=\"user\"+ a CSS@media (prefers-reduced-motion)reset; theSubmitting → Confirming → Completed/Failed → Retryon-chain lifecycle via the shared SendCeremony + StatusPill; testnet/mainnet shown textually (never color-only) in the NetworkMenu chip; darkened--color-mutedfor subtitle/helper contrast; loading skeletons + empty states on every async list; icon-only controls already labelled; wide tables already wrapped inoverflow-x-auto(no horizontal body scroll).Gates: build ✓ · typecheck ✓ · lint ✓ (exit 0) · test ✓ (68 passing, no test changes needed). Part of tracker #41.
Summary by CodeRabbit
Accessibility Improvements
Bug Fixes
Greptile Summary
This PR is a focused accessibility and polish sweep across the console shell and all major screens — no redesigns or new features. It closes concrete a11y gaps identified in the audit by reusing existing primitives.
focus-visible:ring-2to every raw<a>and<button>control that previously only hadhover:styles — explorer/receipt links, dialog triggers, Shell menu items, and theOnChainDetailopen/close pair. The ring color is adapted for dark-background contexts (ring-white/50in Treasury'sFundReceipt).Trprimitive gainstabIndex,role="button", anonKeyDownhandler for Enter/Space (guarded to prevent nested-control hijacking), and an inset focus ring. Non-interactive rows stay out of the tab order.aria-labelnow includes the pending count; the Grants "Generate attestation" and AuditLog "Generate auditor packet" buttons are gated onloading={phase !== "idle"}to prevent re-fire while a ceremony is in flight.CopyButtonhit area is bumped to 28 × 28 px (WCAG 2.5.8).Confidence Score: 5/5
Safe to merge — purely additive accessibility improvements with no logic changes outside the two loading guards and the Tr keyboard enhancement.
All changes are CSS class additions (focus rings, hit-area sizing) or small, self-contained behavioral guards (loading prop tied to ceremony phase, Enter/Space key handler on Tr). The loading guard logic was verified against the paymentReducer state machine: the button is correctly disabled for every non-idle phase and re-enabled on RESET. No data paths, API calls, or business logic were altered.
apps/console/src/ui/controls.tsx — the Tr keyboard enhancement is the most structurally significant change; the role=button concern on tr was already flagged in a prior review thread.
Important Files Changed
is non-conformant per HTML-AAM (covered in prior thread).Reviews (2): Last reviewed commit: "fix(console): address Greptile/CodeRabbi..." | Re-trigger Greptile