🧰 style: Hover-Revealed Tool Output Copy, Full Output, and a Virtualized My Files Table - #16640
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f3e7d98edb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| </div> | ||
| <div className="relative grid h-full max-h-[calc(100vh-20rem)] min-h-[calc(100vh-20rem)] w-full flex-1 overflow-hidden overflow-x-auto overflow-y-auto rounded-md"> | ||
| <div | ||
| ref={scrollRef} |
There was a problem hiding this comment.
Point virtualization at the actual table scroller
When the file list exceeds the viewport, the shared <Table> still creates its default overflow-auto wrapper (packages/client/src/components/Table.tsx), and as the sole child of this fixed-height grid that inner wrapper receives the scrolling. The virtualizer instead observes this outer scrollRef, whose scrollTop remains zero, so it never advances beyond the initially rendered rows; users scroll into the bottom spacer and cannot access later files. Render the table with unwrapped or attach the virtualizer to the inner scrolling element.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 2c3f1f0: the table renders unwrapped, so the virtualizer's own div is the scroller. Verified by scenario my-files-scroll-reaches-last-file (200 files, last row reached) on desktop light, dark and mobile.
| <TableRow | ||
| key={virtualRow.key} | ||
| ref={rowVirtualizer.measureElement} | ||
| data-index={virtualRow.index} |
There was a problem hiding this comment.
Expose virtual row positions to assistive technology
For file lists longer than the rendered virtual window, the table now removes most rows from the accessibility tree but provides neither the total filtered row count nor each mounted row's logical index. Screen readers therefore announce only the small mounted subset and report later windows as though they were the first rows, making selection state and position misleading. Add aria-rowcount to the table and the corresponding aria-rowindex metadata to each virtual row.
AGENTS.md reference: AGENTS.md:L199-L200
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Fixed in 2c3f1f0: aria-rowcount on the table, aria-rowindex on the header and each mounted row, spacer rows aria-hidden. Verified by scenario my-files-rows-report-position.
f3e7d98 to
6f6c230
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
@codex review |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
Summary
Part 1 of the ui-refinments stack (#16640 onto dev, then #16641, #16642, #16643; merge from the bottom up).
Tool output cards reserved a right gutter for their copy buttons and cut long output behind a "Show more" toggle, so the scrollbar sat inset and the output jumped when expanded. The copy buttons now overlay the content in the bottom right and appear only on hover or keyboard focus, and the full output renders inside the existing fixed max-height box, so the scrollbar stays at the edge. Pressing and holding a tool row header no longer flashes a background, and the sticky tool header stacks above the grouped tool icons instead of being overlapped by them.
The My Files table rendered every row through client pagination; it now virtualizes rows with
@tanstack/react-virtualand drops the pager. The compact context button gets the same inset on its bottom, left and right.Type of change
Testing
Tested environments/configuration:
Automated tests:
npx jest --findRelatedTestson the changed files (OutputRenderer, BashCall and Button specs updated)npx tsc --noEmit -p client/tsconfig.json, ESLint, Prettier,npm run static-checksScreenshots / recordings
Pending; to be added as before/after pairs from dev and this branch.
Risk / compatibility
None. Client-only presentation changes.
Checklist