Skip to content

fix(app): middle-click to close workspace tabs#214

Open
thatdaveguy1 wants to merge 1 commit intogetpaseo:mainfrom
thatdaveguy1:feat/middle-click-close-tabs
Open

fix(app): middle-click to close workspace tabs#214
thatdaveguy1 wants to merge 1 commit intogetpaseo:mainfrom
thatdaveguy1:feat/middle-click-close-tabs

Conversation

@thatdaveguy1
Copy link
Copy Markdown
Contributor

Summary

  • Add middle-click close behavior for workspace tabs on desktop/web.
  • Keep mobile/iPad/phone layouts unchanged.
  • Add a focused e2e helper and spec for the tab-close flow.

Validation

  • Confirmed on macOS desktop: middle click closes a tab.
  • Source build and typecheck were validated in the source worktree.
  • Web bundle was rebuilt and synced locally for the desktop app.

Notes

  • I did not include unrelated local changes in this PR.
  • I could not live-test the hosted web app at app.paseo.sh, so the web path is covered by the code change and automated test coverage rather than a manual browser session.

Copilot AI review requested due to automatic review settings April 7, 2026 13:43
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds middle-click-to-close behavior for workspace tabs on web/desktop and introduces an end-to-end test that verifies the tab closes and stays closed after a reload.

Changes:

  • Add web-only middle-click handling on workspace tab chips to trigger onCloseTab.
  • Add a Playwright helper to middle-click a workspace tab by test id.
  • Add an e2e spec covering middle-click close + persistence after reload.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
packages/app/src/screens/workspace/workspace-desktop-tabs-row.tsx Adds web middle-click close handling for desktop tabs (but also modifies context menu wiring).
packages/app/e2e/helpers/workspace-tabs.ts Adds middleClickWorkspaceTab Playwright helper.
packages/app/e2e/middle-click-close-tab.spec.ts Adds e2e coverage for the new middle-click close behavior.
Comments suppressed due to low confidence (2)

packages/app/src/screens/workspace/workspace-desktop-tabs-row.tsx:691

  • buildWorkspaceDesktopTabActions currently requires an onReloadAgent callback (see workspace-tab-menu.ts), but this call site no longer passes it. This will fail type-checking and/or drop the “Reload agent” menu entry unexpectedly. Either re-add onReloadAgent to ResolvedDesktopTabChip/WorkspaceDesktopTabsRowProps and pass it through, or update the menu builder + its tests to remove reload support consistently (and update all callers like workspace-screen.tsx and split-container.tsx that still pass onReloadAgent).
  const resolvedTab = useMemo(
    () =>
      buildWorkspaceDesktopTabActions({
        tab: item.tab,
        index,
        tabCount,
        onCopyResumeCommand,
        onCopyAgentId,
        onCloseTab,
        onCloseTabsToLeft,
        onCloseTabsToRight,
        onCloseOtherTabs,
      }),

packages/app/src/screens/workspace/workspace-desktop-tabs-row.tsx:363

  • The menu icon switch no longer handles entry.icon === "rotate-cw", but buildWorkspaceTabMenuEntries can still emit icon: "rotate-cw" for the “Reload agent” item. This results in a missing/blank icon on desktop. Either restore the mapping (and lucide import) or remove/rename the icon value in the menu entry builder so the UI stays consistent.
                const iconColor = theme.colors.foregroundMuted;
                switch (entry.icon) {
                  case "copy":
                    return <Copy size={16} color={iconColor} />;
                  case "arrow-left-to-line":
                    return <ArrowLeftToLine size={16} color={iconColor} />;
                  case "arrow-right-to-line":
                    return <ArrowRightToLine size={16} color={iconColor} />;
                  case "copy-x":
                    return <CopyX size={16} color={iconColor} />;
                  case "x":
                    return <X size={16} color={iconColor} />;
                  default:
                    return undefined;
                }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +167 to +169
queueMicrotask(() => {
suppressNextPressRef.current = false;
});
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suppressNextPressRef is cleared via queueMicrotask, which runs before the next browser input event task. That likely won’t suppress the subsequent synthesized onPress for the middle-click sequence, allowing a navigate to fire after closing. Consider clearing it with setTimeout(..., 0) / requestAnimationFrame, or resetting it on pointerup/mouseup instead. Also, queueMicrotask is used unguarded here, while other parts of the codebase fall back to setTimeout when it’s unavailable.

Suggested change
queueMicrotask(() => {
suppressNextPressRef.current = false;
});
setTimeout(() => {
suppressNextPressRef.current = false;
}, 0);

Copilot uses AI. Check for mistakes.
@@ -293,14 +345,11 @@ function TabChip({
disabled={entry.disabled}
destructive={entry.destructive}
onSelect={entry.onSelect}
Copy link

Copilot AI Apr 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

entry.tooltip from WorkspaceTabMenuEntry is no longer forwarded to ContextMenuItem, so the tooltip copy (e.g. for “Reload agent”) will never be shown. If tooltips are still desired for some entries, pass tooltip={entry.tooltip} through; otherwise consider removing tooltip from the menu entry model to avoid dead fields.

Suggested change
onSelect={entry.onSelect}
onSelect={entry.onSelect}
tooltip={entry.tooltip}

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants