fix(dashboard): make Hook Logs table rows keyboard accessible (#525) - #619
fix(dashboard): make Hook Logs table rows keyboard accessible (#525)#619AVPthegreat wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughExpandable rows in the Hook Logs and Activity tables now support keyboard activation, expose expanded state through ARIA, become focusable, and display focus-ring styling. ChangesTable row accessibility
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Your PR is awaiting review by a reviewer. Till then you can join the Discord for conversation: https://discord.befailproof.ai |
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #525 by improving keyboard operability and focus visibility for expandable “Hook Logs” table rows in the dashboard UI.
Changes:
- Makes hook-log table rows focusable via keyboard and togglable via Enter/Space.
- Adds expanded-state signaling (
aria-expanded) and visual focus ring styling to the interactive rows. - Applies the same accessibility treatment in both the policies activity view and the session hooks panel.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| app/policies/hooks-client.tsx | Adds keyboard/focus accessibility behaviors to expandable hook activity table rows. |
| app/components/session-hooks-panel.tsx | Mirrors the same row keyboard/focus accessibility behaviors in the session hooks panel table. |
Comments suppressed due to low confidence (2)
app/policies/hooks-client.tsx:687
- Handling Space/Enter on
onKeyDownwithout guardinge.repeatcan cause rapid re-toggles when a key is held down (key auto-repeat). Add ane.repeatguard (or handle Space on keyup) to ensure a single toggle per press.
if (e.key === "Enter" || e.key === " ") {
e.preventDefault();
toggleRow(i);
}
}}
app/components/session-hooks-panel.tsx:339
- Handling Space/Enter on
onKeyDownwithout guardinge.repeatcan cause rapid re-toggles when a key is held down (key auto-repeat). Add ane.repeatguard (or handle Space on keyup) to ensure a single toggle per press.
if (e.key === "Enter" || e.key === " ") {
e.preventDefault();
toggleRow(i);
}
}}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| <React.Fragment key={`${item.timestamp}-${i}`}> | ||
| <tr | ||
| tabIndex={0} | ||
| role="button" |
| <React.Fragment key={`${item.timestamp}-${i}`}> | ||
| <tr | ||
| tabIndex={0} | ||
| role="button" |
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@app/components/session-hooks-panel.tsx`:
- Around line 330-340: The expandable rows in
app/components/session-hooks-panel.tsx lines 330-340 and
app/policies/hooks-client.tsx lines 678-688 should retain native <tr> semantics
by removing row-level role, focus, and keyboard interaction. Add an inner
disclosure button within the appropriate cell in each row, moving aria-expanded,
toggleRow behavior, keyboard handling, and focus styling to that button while
preserving the existing expansion behavior.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 714c3afc-561d-4677-b859-dde211c95dba
📒 Files selected for processing (2)
app/components/session-hooks-panel.tsxapp/policies/hooks-client.tsx
| tabIndex={0} | ||
| role="button" | ||
| aria-expanded={isExpanded} | ||
| onClick={() => toggleRow(i)} | ||
| className={`cursor-pointer transition-colors ${ | ||
| onKeyDown={(e) => { | ||
| if (e.key === "Enter" || e.key === " ") { | ||
| e.preventDefault(); | ||
| toggleRow(i); | ||
| } | ||
| }} | ||
| className={`cursor-pointer transition-colors focus:outline-none focus:ring-1 focus:ring-ring ${ |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate files =="
git ls-files | rg 'app/(components/session-hooks-panel|policies/hooks-client)\.tsx$' || true
echo "== file sizes =="
wc -l app/components/session-hooks-panel.tsx app/policies/hooks-client.tsx
echo "== snippets around affected lines =="
sed -n '300,355p' app/components/session-hooks-panel.tsx
printf '\n---\n'
sed -n '655,705p' app/policies/hooks-client.tsx
echo "== nearby context and role/button occurrences in affected TSX =="
rg -n 'role="button"|tabIndex=0|aria-expanded=|<tr|</tr|onKeyDown=.*e\.key|window\.preventDefault' app/components/session-hooks-panel.tsx app/policies/hooks-client.tsxRepository: FailproofAI/failproofai
Length of output: 7279
🌐 Web query:
WAI-ARIA grid row role hidden semantics table row role aria-expanded button row
💡 Result:
Under WAI-ARIA standards, a row (role=row) should contain only specific child roles—typically gridcell, columnheader, or rowheader [1][2][3][4]. Placing a button directly as a child of a row is considered invalid according to strict semantic structure, as accessibility APIs expect grid cells to be the immediate children of rows [5]. Regarding your specific architectural questions: 1. Hidden Semantics: If rows or cells are hidden (e.g., via display: none or not present in the DOM), they are excluded from the accessibility tree [6]. To maintain correct row counting and indexing for assistive technologies when rows or columns are missing from the DOM, you must use aria-rowcount, aria-colcount, aria-rowindex, and aria-colindex on the appropriate elements [7][1][6][3]. 2. Button within a Row: Placing a button as a direct child of a row violates the requirement that rows contain only cells [5]. The correct, accessible pattern for an interactive element (like an expand/collapse toggle) inside a row is to place the button inside a gridcell [6][8][5]. 3. aria-expanded: The aria-expanded attribute should be placed on the interactive control (the button) that performs the toggle action, not on the row element itself [9][8]. When a grid requires hierarchical "expand/collapse" behavior (where a row controls the visibility of child rows), the pattern is a treegrid, not a standard grid [10][8]. In a treegrid, the aria-expanded state is managed on the parent row or a cell within that row, indicating whether the child rows are currently visible [9][10]. For most web applications, if you need expandable rows, using the treegrid role with the proper hierarchical ARIA attributes (aria-level, aria-expanded) is the recommended approach [10][8]. Always prefer native HTML table elements (table, tr, td) if the data is tabular, as they provide these semantics automatically [1][3][4]. If you must use ARIA roles for custom layouts, ensure your DOM structure strictly adheres to the required parent-child relationships (grid > row > gridcell) [1][2][3]. Top results: [1], [3], [9], [8], [5]
Citations:
- 1: https://www.w3.org/WAI/ARIA/apg/patterns/grid/
- 2: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Roles/grid_role
- 3: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Roles/row_role
- 4: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Roles/gridcell_role
- 5: https://support.pega.com/support-doc/why-constellation-grids-groups-applied-contain-buttons-children-rows-rolerow
- 6: https://www.w3.org/WAI/ARIA/apg/patterns/grid/examples/layout-grids/
- 7: https://www.w3.org/WAI/ARIA/apg/practices/grid-and-table-properties/
- 8: https://www.accessible-data-interfaces.com/accessible-data-tables-grid-systems/expandable-rows-nested-data/
- 9: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Attributes/aria-expanded
- 10: https://developer.mozilla.org/en-US/docs/Web/Accessibility/ARIA/Reference/Roles/treegrid_role
Keep expandable table rows as rows and use an inner disclosure button.
Both interactive rows assign role="button" to native <tr> elements, which replaces the table row semantics from the descendant cells. Keep the <tr> native and move the focus/action semantics to a proper control inside a cell, such as an inner <button> with aria-expanded, keyboard handling, and focus styling.
app/components/session-hooks-panel.tsx#L329-L331app/policies/hooks-client.tsx#L677-L679
📍 Affects 2 files
app/components/session-hooks-panel.tsx#L330-L340(this comment)app/policies/hooks-client.tsx#L678-L688
🤖 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 `@app/components/session-hooks-panel.tsx` around lines 330 - 340, The
expandable rows in app/components/session-hooks-panel.tsx lines 330-340 and
app/policies/hooks-client.tsx lines 678-688 should retain native <tr> semantics
by removing row-level role, focus, and keyboard interaction. Add an inner
disclosure button within the appropriate cell in each row, moving aria-expanded,
toggleRow behavior, keyboard handling, and focus styling to that button while
preserving the existing expansion behavior.
Source: MCP tools
Summary
Fixes #525 by adding keyboard accessibility support (
tabIndex={0},role="button",aria-expanded={isExpanded},onKeyDownhandlers) and focus ring indicators to Hook Logs table rows in the Next.js dashboard components (app/policies/hooks-client.tsxandapp/components/session-hooks-panel.tsx).❌ Root Cause Analysis
In
app/policies/hooks-client.tsxandapp/components/session-hooks-panel.tsx, table rows in the Hook Logs table rendered<tr onClick={() => toggleRow(i)}>without accessibility attributes:Why it failed:
<tr>elements lackedtabIndex={0}, making them unreachable via keyboardTab/Shift+Tabnavigation.role="button"andaria-expanded, preventing screen readers and keyboard users from understanding that the row was expandable.onKeyDownhandler, so pressingEnterorSpacewhen focused on a row did not trigger row expansion.✅ Fix Details
Updated
<trin bothapp/policies/hooks-client.tsxandapp/components/session-hooks-panel.tsx:🧪 Verification
Tabfocus,Enter/Spacerow toggle).aria-expandedstate tracking and visual focus ring indicator.Summary by CodeRabbit