fix(web): clarify terminal sidebar grouping - #7967

💭 Checking the current PR diff against prior feedback, I see both flagged issues from commit b75ecb4 were resolved: the tooltip prop was added to the close button, and cursor-pointer styling was added to the group header. Run 47180's duplication concern is also addressed by extracting PanelTabCloseButton as a shared component. Now I'm reviewing the current head for new issues, checking that the pending indicator dot still positions correctly within the relative wrapper span and hides on hover as before, verifying the tooltip component exports exist, and confirming the group/tab and group/close class hooks are properly wired between RightPanelTabs and the button primitive. I want to double-check that the TooltipTrigger's render prop correctly merges button props from the underlying primitive. I'm also noticing that in ThreadTerminalDrawer, the close button now always shows instead of only when there's more than one terminal — meaning users could now close the last remaining terminal, which is a behavior change worth flagging even if it might be intentional for consistency. I need to check whether Popover-related imports and XIcon are now unused after this refactor, which would cause lint errors, and confirm the className logic in that file closes properly. I'll also note that the grouped-terminal indentation styling was intentionally removed as a visual change. Let me pull up the actual files to confirm all this.

1. browse_code — apps/web/src/components/ui/panel-tab-close-button.tsx

2. browse_code — apps/web/src/components/ThreadTerminalDrawer.tsx

💭 I should check the tooltip primitive to verify how TooltipTrigger's render prop works and whether TooltipPopup needs anything specific, and also confirm the Popover imports are still needed in ThreadTerminalDrawer.

3. browse_code — apps/web/src/components/ui/tooltip.tsx

4. git_tools — Popover|XIcon|normalizedTerminalIds|showGroupHeaders

💭 Popover still gets used elsewhere, so that's fine. I'm checking whether TooltipTrigger with a render prop needs a Tooltip Provider ancestor in Base UI, since PanelTabCloseButton is used without a tooltip wrapper elsewhere and I want to confirm this won't break. I also verify that the terminal sidebar only shows when there's more than one terminal, so removing that length check on the close button doesn't matter. Then I run through UI consistency: the close button hover behavior per row, the tooltip trigger wrapping it correctly, and whether the identity icon swap in RightPanelTabs raises any accessibility concerns. I want to double-check that PanelTabCloseButton keeps its onClick handler and any related props like stopPropagation or title that RightPanelTabs previously had, so I need to look at that component's current code directly.

5. browse_code — apps/web/src/components/RightPanelTabs.tsx

6. git_tools — apps/web/src/components/RightPanelTabs.tsx b60a2c0..269eba5

💭 I'm noticing a small inconsistency: RightPanelTabs has cursor-pointer on the row div itself, but in ThreadTerminalDrawer the row div isn't clickable—only the inner button carries the cursor-pointer style. Worth checking if that difference matters for the click target. The tooltip default side changed from "bottom" to "top" for the close button, though that's likely inconsequential. The main hover-swap behavior—terminal icon replaced by X on row hover instead of a separate button appearing—matches the PR's intent for consistency, and I'm confirming no lingering unnamed group-hover: selectors were left behind after renaming to group/tab. I'm also checking that the group header button uses rounded instead of rounded-md like the terminal rows—a minor cosmetic inconsistency not worth flagging. More importantly, I'm scrutinizing whether group-focus-visible/close:block actually works correctly when group/close is applied to the same button containing the X icon, since Tailwind's group-focus-visible typically expects a descendant relationship rather than referring to the element itself. This existed in RightPanelTabs already but is being newly propagated into the terminal drawer, which is more of an intentional design carryover than a fresh regression given this is a desktop app. I also want to check whether group headers now show generic labels like "Single"/"Stacked"/"Side by side" instead of "Group 1"/"Group 2", which could create ambiguity across multiple groups, so I should look at where showGroupHeaders is defined.

7. browse_code — apps/web/src/components/ThreadTerminalDrawer.tsx

8. git_tools — group/tab|group/close

💭 I'm noticing a mismatch between the grep results and the file view at the same commit — the line numbers and code snippets don't line up, suggesting grep might be using a different default revision than the one I specified. It's possible RightPanelTabs has two separate tab list implementations, so let me directly check lines 620-660 at 269eba5 to see if there's a second occurrence I'm missing.

9. browse_code — apps/web/src/components/RightPanelTabs.tsx

💭 I'm realizing the git_grep search may have run against main's HEAD rather than the actual PR commit 269eba5, which could explain why it's showing a tab implementation that seems to already exist there. I want to check whether main genuinely already contains this feature or if I'm just looking at the wrong revision by default. Let me try re-running git_grep with rev explicitly set to 269eba5 to confirm which commit is actually being searched.

10. git_tools — group/tab|group/close|PanelTabCloseButton

💭 That's strange, getting flagged for too much output with only 3 patterns. I'll narrow things down using a path filter instead.

11. git_tools — group/close

💭 Confirmed that at 269eba5 only the primitive has group/close — the earlier grep must have matched a different version at main HEAD. I'll also check for group/tab at that same commit.

12. git_tools — group/tab

💭 Good, both prior findings look addressed now—tooltip added, cursor-pointer on the group header, and the shared primitive extracted. Now I want to check for any new issues introduced by the latest commits, so I'll diff the range since the last reviewed commit.

13. git_tools — b75ecb4..269eba5

💭 The branch got force-pushed, so now I'm going through the full diff to check for findings. I'm noting the tooltip inconsistency between the two call sites isn't really an issue since it's an optional prop, and I'm looking into whether the close button's onClick needs to stop propagation in ThreadTerminalDrawer. Checking RightPanelTabs, I see the row's onClick behavior is unchanged from the original code, so that's pre-existing rather than a new issue. I'm also confirming PanelTabCloseButton's placement in components/ui is appropriate, and that the tooltip trigger and layout differences between the two rows (w-full vs max-w-36 shrink-0) are just cosmetic, not real problems. I'm now noting a few more minor things: focus-visible behavior works correctly for keyboard users revealing the close button, but the label button and row lack focus rings elsewhere (pre-existing). Removing the group-header indentation changes the visual nesting but seems like a deliberate design choice tied to the "fix terminal group spacing" title rather than a bug, and the badge's muted color when active is a minor styling quirk. I'm also flagging that the group header lacks aria-pressed semantics, consistent with the existing pattern, before moving on to check the terminal identity icon in the drawer for a possibly real issue. I'm double-checking that the icon nesting is fine since the parent span already handles sizing, that hover-based reveal on touch devices matches existing patterns in this desktop Electron app, and that the cn utility is properly imported in ThreadTerminalDrawer since it's used at line 1675 — I want to verify this to avoid a typecheck failure. I'm also confirming the Square icon import was added correctly and that shrink-0 is present on the group icon, then moving to check whether TooltipProvider wraps the app for the Tooltip usage.

14. git_tools — ^import|from "~/lib/utils"

💭 cn is confirmed imported. I'm checking consistency between the drawer's group header height and the terminal row height, noticing a slight mismatch between the arbitrary 22px value and the standard 24px row height used elsewhere. I'm also reviewing keyboard accessibility for the close button, confirming it's tabbable and shows properly on focus via the tooltip. One behavioral concern: in the new design, hovering the terminal icon transforms it into a close button, which could accidentally close a terminal for users who habitually click that icon to select it -- a UX regression worth noting, though not a code-consistency violation. I'm double-checking the pending indicator logic in RightPanelTabs to confirm the layout and absolute positioning stay unchanged after the refactor, and I'm considering whether tests are needed given the primitive composition change. Since this is essentially a markup-preserving extraction, I lean toward not requiring new tests. I'm also verifying that the tooltip trigger's render prop merges properly with existing event handlers, matching the pattern already used elsewhere, and that the tooltip's portal z-index behaves consistently with the drawer overlay. Given the check requires a "defaults to failure" outcome unless everything's clear, but the schema demands JSON output, I'm reconciling these by framing the result as success with a title like "UI Consistency: All clear" and a matching summary, while double-checking the drawer's terminal row select button for any remaining issues before finalizing.