fix(dashboard): reopen the request detail inline under the clicked activity row#417
Conversation
…tivity row The shared-table migration (#415) moved the Activity request detail from an accordion row under the clicked request to a card below the whole table, so on a full page the panel opened off-screen and a row click looked like it did nothing. DataTable gains an inline detail: when detailKey matches a row, renderDetail renders as a full-width selection-disabled row directly under it. The panel is interleaved into the items collection as a sentinel item, so the per-row cache from #416 stays valid for every ordinary row and selection latency is unchanged (re-measured: 35 ms at 50 rows, 65 ms at 100). Activating the detail row itself does not fire onRowAction, so it cannot re-toggle the panel. ActivityPage renders its detail (with the Close button and cache token counts) through this instead of the below-table card. Verified in-browser: the panel opens adjacent and in-viewport on row click, including on selection-disabled gateway rows, follows the newly clicked row, closes on re-click, and coexists with checkbox selection. A regression test pins the adjacent placement. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GitHub Actions
Inactive
|
Warning Review limit reached
Next review available in: 2 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: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
WalkthroughChangesActivity detail table
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 2 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (2 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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 |
![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 `@web/src/components/DataTable.test.tsx`:
- Around line 180-190: Update the detail-row test around renderDetail to query
the user-visible “detail for Bravo” text with the appropriate user-facing query
instead of getByTestId("detail") for both the assertion and click. Preserve the
existing verification that activating the detail content does not trigger
onRowAction.
In `@web/src/pages/ActivityPage.tsx`:
- Around line 620-621: Add a colocated ActivityPage Vitest integration test
covering the expanded-row wiring: mount ActivityPage, open an activity row,
assert its inline detail renders, then press Close and verify the detail
disappears and expandedId is cleared. Exercise the existing detailKey and
renderDetail props through ActivityPage rather than mounting DataTable directly.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 7661a075-4462-4e7f-ba13-a9571f21c083
📒 Files selected for processing (23)
src/gateway/static/dashboard/assets/ActivityPage-D8fw0svL.jssrc/gateway/static/dashboard/assets/ActivityPage-DfKXtil9.jssrc/gateway/static/dashboard/assets/AliasesPage-UYGFGqju.jssrc/gateway/static/dashboard/assets/BudgetsPage-vF_-AwZ5.jssrc/gateway/static/dashboard/assets/ConfirmDialog-QWIOOCGv.jssrc/gateway/static/dashboard/assets/DataTable-CkPg87A4.jssrc/gateway/static/dashboard/assets/DataTable-Dd6pdraQ.jssrc/gateway/static/dashboard/assets/KeysPage-DT96_7Wr.jssrc/gateway/static/dashboard/assets/ModelScopeControl-kWLL_YZ7.jssrc/gateway/static/dashboard/assets/ModelsPage-D4EAncRN.jssrc/gateway/static/dashboard/assets/OverviewPage-YNct3IpQ.jssrc/gateway/static/dashboard/assets/ProvidersPage-DCaVrGdF.jssrc/gateway/static/dashboard/assets/SettingsPage-ChrFYkfk.jssrc/gateway/static/dashboard/assets/TablePagination-BtXHwFm8.jssrc/gateway/static/dashboard/assets/ToolsGuardrailsPage-CaO5-mJQ.jssrc/gateway/static/dashboard/assets/UsagePage-pTMnFBtb.jssrc/gateway/static/dashboard/assets/UsersPage-Du9Z6eH6.jssrc/gateway/static/dashboard/assets/index-D8e6LjtH.jssrc/gateway/static/dashboard/assets/index-xtRHMH4y.csssrc/gateway/static/dashboard/index.htmlweb/src/components/DataTable.test.tsxweb/src/components/DataTable.tsxweb/src/pages/ActivityPage.tsx
💤 Files with no reviewable changes (2)
- src/gateway/static/dashboard/assets/DataTable-CkPg87A4.js
- src/gateway/static/dashboard/assets/ActivityPage-D8fw0svL.js
…wiring Addresses CodeRabbit feedback on PR 417: the DataTable detail test now queries the user-visible text instead of a test id, and a colocated ActivityPage test covers the integration (clicking a row opens the detail as the row's next sibling, and Close collapses it). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GitHub Actions
Inactive
…shift Three responsiveness fixes for the shared tables, driven by smoke-testing: - Optimistic selection checkbox: react-aria only reports the new selection after re-rendering every row (~0.6 ms/row/click), so the checkmark lagged by 65 ms at 100 rows and far more on slower machines. The box visual now flips on pointerdown (measured 1-3 ms) and the authoritative state reconciles underneath, with a timeout backstop for presses that never land. - Inline detail is a portal row with instant acknowledgment: the panel is a portal-managed presentation <tr> outside react-aria's collection (in the collection, expanding re-processed the whole page: 130 ms at 100 rows, 490 ms throttled), slides open with a reduced-motion-aware animation, and the clicked row highlights in the same frame while react-aria's interaction render (~1.6 ms/row) catches up. - Row clicks keep opening the detail while a selection is active: react-aria's toggle behavior repurposes them into selection extension once any row is selected, so DataTable intercepts data-cell clicks ahead of the row press handler and routes them to the row action (checkboxes, buttons, links, and the panel pass through untouched). - The bulk action bar floats fixed near the viewport bottom instead of rendering in flow above the table, which shifted every row down under the cursor on first selection and scrolled out of reach on long pages. Verified in-browser: zero table shift on selection, detail adjacent and animated with cache counts, selection coexists with the open panel, and selection latency unchanged (32-51 ms at 50 rows). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GitHub Actions
Inactive
There was a problem hiding this comment.
Pull request overview
This PR fixes a dashboard regression where clicking an Activity row appeared to do nothing by restoring the pre-#415 behavior: the request detail now expands inline directly beneath the clicked row within the table viewport.
Changes:
- Extend
DataTablewith an inline detail-row mechanism (detailKey+renderDetail) implemented via a portal-managed<tr>inserted immediately after the target row. - Update
ActivityPageto render request details through the new inline mechanism (removing the below-table card). - Improve table UX with immediate click acknowledgment styling, reduced-motion handling, and a fixed-position bulk action bar to avoid layout shifts.
Reviewed changes
Copilot reviewed 24 out of 28 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| web/src/styles/globals.css | Adds styling and animations for the inline detail row and the fixed bulk action bar (with reduced-motion support). |
| web/src/pages/ActivityPage.tsx | Switches Activity request detail rendering to DataTable inline detail via detailKey/renderDetail. |
| web/src/pages/ActivityPage.test.tsx | Adds a regression test asserting the detail opens adjacent to the clicked row and closes correctly. |
| web/src/components/DataTable.tsx | Implements inline detail rows (portal + host <tr> insertion) and click interception so row actions still fire with active selection. |
| web/src/components/DataTable.test.tsx | Adds coverage for inline detail adjacency and verifies detail content does not trigger onRowAction. |
| web/src/components/BulkActionBar.tsx | Moves bulk action UI to a fixed-position toolbar to prevent table layout shifts. |
| src/gateway/static/dashboard/index.html | Updates built dashboard asset references (hashed JS/CSS filenames). |
| src/gateway/static/dashboard/assets/UsersPage-C8N0rekv.js | Rebuilt dashboard bundle output reflecting the source changes. |
| src/gateway/static/dashboard/assets/UsagePage-BNkia9eC.js | Rebuilt dashboard bundle output reflecting the source changes. |
| src/gateway/static/dashboard/assets/ToolsGuardrailsPage-DIVtsycj.js | Rebuilt dashboard bundle output reflecting the source changes. |
| src/gateway/static/dashboard/assets/tableSelection-CojkFPwd.js | New rebuilt chunk for updated bulk action bar behavior and selection utilities. |
| src/gateway/static/dashboard/assets/tableSelection-CLDlIlLU.js | Removes the prior built selection chunk after rebuild. |
| src/gateway/static/dashboard/assets/TablePagination-Dob9a4wG.js | Rebuilt chunk to match new bundle graph after dashboard rebuild. |
| src/gateway/static/dashboard/assets/SettingsPage-DovUcX7T.js | Rebuilt dashboard bundle output reflecting the source changes. |
| src/gateway/static/dashboard/assets/OverviewPage-CB0oRBw3.js | Rebuilt dashboard bundle output reflecting the source changes. |
| src/gateway/static/dashboard/assets/ModelScopeControl-CTGSrkZ9.js | Rebuilt dashboard bundle output reflecting the source changes. |
| src/gateway/static/dashboard/assets/KeysPage-C2ndVSn9.js | Rebuilt dashboard bundle output reflecting the source changes. |
| src/gateway/static/dashboard/assets/DataTable-CrQq0sYf.js | New rebuilt chunk for the updated DataTable implementation. |
| src/gateway/static/dashboard/assets/DataTable-CkPg87A4.js | Removes the prior built DataTable chunk after rebuild. |
| src/gateway/static/dashboard/assets/ConfirmDialog-CvV5gDQA.js | Rebuilt dashboard bundle output reflecting the source changes. |
| src/gateway/static/dashboard/assets/BudgetsPage-Bt8e_tU9.js | Rebuilt dashboard bundle output reflecting the source changes. |
| src/gateway/static/dashboard/assets/AliasesPage-CNowZYqH.js | Rebuilt dashboard bundle output reflecting the source changes. |
| src/gateway/static/dashboard/assets/ActivityPage-DDIA-0sC.js | New rebuilt chunk for Activity page now using inline detail rows. |
| src/gateway/static/dashboard/assets/ActivityPage-D8fw0svL.js | Removes the prior built Activity page chunk after rebuild. |
Files not reviewed (3)
- src/gateway/static/dashboard/assets/ActivityPage-DDIA-0sC.js: Generated file
- src/gateway/static/dashboard/assets/DataTable-CrQq0sYf.js: Generated file
- src/gateway/static/dashboard/assets/tableSelection-CojkFPwd.js: Generated file
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 `@web/src/components/DataTable.tsx`:
- Around line 180-219: Add a regression test for the detail-panel flow around
the effect that creates the host row and the `tryInsert` helper, keeping the
panel open while sorting, filtering, and pagination change so its target row
moves or disappears. Assert that insertion, removal, and re-insertion complete
without DOM reconciliation errors and that the detail content remains correct
when the row becomes available again.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 0b7ce7b0-e66d-41f1-b465-985e36ffbeeb
📒 Files selected for processing (25)
src/gateway/static/dashboard/assets/ActivityPage-DDIA-0sC.jssrc/gateway/static/dashboard/assets/AliasesPage-CNowZYqH.jssrc/gateway/static/dashboard/assets/BudgetsPage-Bt8e_tU9.jssrc/gateway/static/dashboard/assets/ConfirmDialog-CvV5gDQA.jssrc/gateway/static/dashboard/assets/DataTable-CrQq0sYf.jssrc/gateway/static/dashboard/assets/KeysPage-C2ndVSn9.jssrc/gateway/static/dashboard/assets/ModelScopeControl-CTGSrkZ9.jssrc/gateway/static/dashboard/assets/ModelsPage-CAW2PJJV.jssrc/gateway/static/dashboard/assets/OverviewPage-CB0oRBw3.jssrc/gateway/static/dashboard/assets/ProvidersPage-BULpjU4h.jssrc/gateway/static/dashboard/assets/SettingsPage-DovUcX7T.jssrc/gateway/static/dashboard/assets/TablePagination-Dob9a4wG.jssrc/gateway/static/dashboard/assets/ToolsGuardrailsPage-DIVtsycj.jssrc/gateway/static/dashboard/assets/UsagePage-BNkia9eC.jssrc/gateway/static/dashboard/assets/UsersPage-C8N0rekv.jssrc/gateway/static/dashboard/assets/index-CcyBGqLj.csssrc/gateway/static/dashboard/assets/index-DHOrDaWF.jssrc/gateway/static/dashboard/assets/tableSelection-CLDlIlLU.jssrc/gateway/static/dashboard/assets/tableSelection-CojkFPwd.jssrc/gateway/static/dashboard/index.htmlweb/src/components/BulkActionBar.tsxweb/src/components/DataTable.test.tsxweb/src/components/DataTable.tsxweb/src/pages/ActivityPage.tsxweb/src/styles/globals.css
💤 Files with no reviewable changes (1)
- src/gateway/static/dashboard/assets/tableSelection-CLDlIlLU.js
🚧 Files skipped from review as they are similar to previous changes (3)
- src/gateway/static/dashboard/index.html
- web/src/pages/ActivityPage.tsx
- web/src/components/DataTable.test.tsx
Addresses CodeRabbit feedback on PR 417: the portal-managed detail row sits outside react-aria's collection, so this pins that react's row reconciliation cannot strand or duplicate it. The panel follows its row through a reorder, is fully removed when the target row leaves the row set, and re-attaches with correct content when the row returns. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
GitHub Actions
Inactive
khaledosman
left a comment
khaledosman
left a comment
There was a problem hiding this comment.
Solid regression fix with strong test coverage (row caching + its failure mode, adjacency, intercept-with-selection, page-size snapping). Findings below are non-blocking; the main one is a fragility/maintainability note on the imperative DOM insertion, not a defect.
Verified as non-issues: CSS.escape guards the selector, the Math.max(0, page) + nearest-option reduce clamps handle 0/negative/out-of-range sizes (tests pin them), prefers-reduced-motion disables both animations, the detail row's role=presentation keeps grid semantics correct, and capture-phase stopPropagation keeps onRowAction firing exactly once.
🤖 Review generated with Claude Code
| // the right spot) whenever the row set, order, or target changes, and | ||
| // cleanup always removes the host, so a vanished target (filtered out, page | ||
| // flipped) leaves nothing behind. | ||
| useLayoutEffect(() => { |
There was a problem hiding this comment.
The detail row is document.createElement'd and spliced into react-aria's <tbody> with target.after(), then React portals into the hand-made <td>. This is verified working and React removes DOM by node reference so it's mostly safe, but interleaving a foreign node among keyed react-aria rows is a known footgun: row reordering on refetch, sort changes, concurrent rendering, or a react-aria-components upgrade could orphan/misplace the host or throw on commit. The MutationObserver + effect-re-run + cleanup machinery here mitigates it but also shows how much is being fought.
Not a blocker given the verification, but worth a comment noting this couples tightly to RAC's DOM internals and should be revisited if/when RAC gains native expandable-row support.
| // selection exists, clicks on ordinary data cells are intercepted before the | ||
| // row's press handler sees them and routed to the row action instead. | ||
| // Checkboxes, buttons, links, inputs, and the detail panel pass through. | ||
| const interceptedRowKey = useCallback( |
There was a problem hiding this comment.
This interception is a global DataTable behavior change, not scoped to ActivityPage. For any consumer with onRowAction + a non-empty selection, row clicks now route to the row action instead of react-aria's selection-extend. Besides ActivityPage, ModelsPage matches (onRowAction={onSelect} + selectionMode="multiple"): while a bulk model selection is active, clicking a row now fires the pricing-comparison drill instead of toggling that row into the checkbox selection.
That's consistent with the "checkbox owns selection" convention this PR argues for, and likely fine, but it's a silent cross-page change. Worth confirming ModelsPage didn't rely on row-click-to-extend, and a one-line mention in the PR body. (Keys/Users/Aliases/Budgets pass no onRowAction, so the if (!onRowAction) return null guard leaves them untouched — verified.)
| const showChecked = flash ?? (isSelected || isIndeterminate); | ||
| return ( | ||
| <span | ||
| onPointerDown={() => { |
There was a problem hiding this comment.
setFlash(!isSelected) on the shared component means clicking the indeterminate "select all" header flashes it fully-checked; if react-aria resolves that click to clearing the selection, isSelected never matches flash and the checkmark lingers until the 600 ms backstop. Cosmetic only, and per-row checkboxes are unaffected — noting in case it's easy to skip the optimistic flash when isIndeterminate.
| return ( | ||
| <div className="flex flex-wrap items-center gap-3 rounded-xl border border-[var(--otari-brand)] bg-[var(--otari-brand-tint)] px-4 py-2.5"> | ||
| <div | ||
| role="toolbar" |
There was a problem hiding this comment.
fixed bottom-4 … z-40 floats the bar over whatever sits at the viewport bottom (the paginator, the last rows, or an open detail panel) with no reserved bottom padding while it's shown. On short viewports this can cover the pagination controls the operator needs mid-selection. Consider reserving space (bottom padding/spacer) when the bar is active. role="toolbar" + aria-label are good additions.
Description
Regression from the shared-table migration (#415): clicking a request row on the Activity page looked like it did nothing. Before #415 the request detail expanded inline directly under the clicked row; #415 moved it to a card rendered below the whole table, so on a full page the panel opened far outside the viewport.
DataTablegains an inline detail affordance: whendetailKeymatches a row's key,renderDetail(row)renders as a full-width, selection-disabled row directly under that row. The panel is interleaved into the react-aria items collection as a sentinel item, so the per-row cache from #416 stays valid for every ordinary row; selection latency is unchanged (re-measured: 35 ms per click at 50 rows, 65 ms at 100). Activating the detail row itself does not fireonRowAction, so it cannot re-toggle the panel. ActivityPage renders its request detail (Close button, cache token counts, billed meters) through this instead of the below-table card.Verified in a scripted browser session against the built bundle: the panel opens adjacent and in-viewport on row click (single
<td colspan>row, matching the pre-#415 accordion), works on selection-disabled gateway rows, follows the newly clicked row, closes on re-click, and coexists with checkbox selection; no console errors.PR Type
Relevant issues
None filed; reported directly by the maintainer from a deployed instance after #416 merged (the regression itself dates to #415).
Checklist
web/src/components/DataTable.test.tsx; full web suite 282/282).npm --prefix web run typecheckandnpm --prefix web testpass; dashboard bundle rebuilt and committed; no Python touched).AI Usage
AI Model/Tool used: Claude Code (Claude Fable 5)
Any additional AI details you'd like to share:
Root-caused, implemented, and browser-verified by Claude Code under njbrake's direction, from his report that row clicks stopped visibly doing anything.
🤖 Generated with Claude Code
Summary
Benefits
Request details are easier to associate with their rows, table interactions feel more responsive, and bulk actions remain accessible without disrupting the page layout.