Skip to content

fix(web-shell): polish embedded shell interactions #7477

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
ytahdn merged 3 commits into from
Jul 22, 2026
Merged

fix(web-shell): polish embedded shell interactions #7477

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
293ad2d
fix(web-shell): polish embedded shell interactions
Jul 22, 2026
809b82c
fix(web-shell): complete file preview integration
Jul 22, 2026
30b990c
test(web-shell): enable worktree toggle in app tests
Jul 22, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 7 additions & 7 deletions packages/web-shell/client/App.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1852,7 +1852,7 @@ describe('App session callbacks', () => {
}

it('shows the toggle in the empty state for a trusted git workspace', async () => {
const { container } = renderApp();
const { container } = renderApp({ showWorktreeToggle: true });

@chiga0 chiga0 Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Minor] No test locks in the new default-hidden behavior

All seven updated tests now pass showWorktreeToggle: true, but the core contract this PR introduces — embedded consumers (prop absent/false) do not see the worktree action — has no assertion. A counterpart test would prevent the original leak from silently returning:

it('hides the toggle by default for embedded consumers', async () => {
  const { container } = renderApp();
  await flush();
  await flush();
  expect(container.querySelector(toggleSelector)).toBeNull();
});

(Also flagged by @yiliang114 in the overview.)

This review was generated by QoderWork AI

await waitForToggle(container);
});

Expand All @@ -1862,7 +1862,7 @@ describe('App session callbacks', () => {
{ id: 'primary', cwd: '/workspace', primary: true, trusted: false },
],
};
const { container } = renderApp();
const { container } = renderApp({ showWorktreeToggle: true });
await flush();
await flush();
expect(container.querySelector(toggleSelector)).toBeNull();
Expand All @@ -1873,14 +1873,14 @@ describe('App session callbacks', () => {
workspaceGit: vi.fn().mockRejectedValue(new Error('not a git repo')),
workspaceSkills: mockWorkspaceActions.loadSkillsStatus,
}));
const { container } = renderApp();
const { container } = renderApp({ showWorktreeToggle: true });
await flush();
await flush();
expect(container.querySelector(toggleSelector)).toBeNull();
});

it('toggles the pending badge on and off', async () => {
const { container } = renderApp();
const { container } = renderApp({ showWorktreeToggle: true });
await waitForToggle(container);

await clickButton(container, toggleSelector);
Expand All @@ -1893,7 +1893,7 @@ describe('App session callbacks', () => {
});

it('creates the session with worktree when the toggle is enabled', async () => {
const { container } = renderApp();
const { container } = renderApp({ showWorktreeToggle: true });
await waitForToggle(container);
await clickButton(container, toggleSelector);

Expand All @@ -1910,7 +1910,7 @@ describe('App session callbacks', () => {
});

it('creates the session without worktree when the toggle is off', async () => {
renderApp();
renderApp({ showWorktreeToggle: true });
await flush();

await act(async () => {
Expand All @@ -1926,7 +1926,7 @@ describe('App session callbacks', () => {
});

it('clears the pending worktree intent when starting a new session from the sidebar', async () => {
const { container } = renderApp();
const { container } = renderApp({ showWorktreeToggle: true });
await waitForToggle(container);
await clickButton(container, toggleSelector);
expect(container.textContent).toContain(badgeDesc);
Expand Down
15 changes: 13 additions & 2 deletions packages/web-shell/client/App.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,8 @@ export interface WebShellProps {
renderToolHeaderExtra?: ToolHeaderExtraRenderer;
/** Custom renderer for the welcome header. Receives version, cwd, model, and mode. */
renderWelcomeHeader?: WelcomeHeaderRenderer;
/** Show the worktree-isolation action in the empty welcome state. Defaults to false. */
showWorktreeToggle?: boolean;
/** Custom renderer shown below the chat composer in the empty welcome state. */
renderWelcomeFooter?: WelcomeFooterRenderer;
/**
Expand Down Expand Up @@ -1058,6 +1060,7 @@ export function App({
composerTagIcons,
renderToolHeaderExtra,
renderWelcomeHeader,
showWorktreeToggle = false,
renderWelcomeFooter,
mobileWelcomeFooterMiddle = false,
parseUserMessageContent,
Expand Down Expand Up @@ -6369,7 +6372,8 @@ export function App({
// session would land in is trusted and is a git repository — the daemon
// rejects worktree creation otherwise. Mirrors the sidebar entry's gating.
const worktreeToggleEligible = Boolean(
workspaces.find((entry) => entry.cwd === activeWorkspaceCwd)?.trusted &&
showWorktreeToggle &&
workspaces.find((entry) => entry.cwd === activeWorkspaceCwd)?.trusted &&
selectedWorkspaceGitStatus?.branch,
);
const worktreeToggleRef = useRef<HTMLButtonElement>(null);
Expand All @@ -6385,6 +6389,11 @@ export function App({
setWorktreePending(false);
worktreeFocusTarget.current = 'toggle';
}, []);
useEffect(() => {
if (showWorktreeToggle) return;
pendingWorktreeRef.current = undefined;
setWorktreePending(false);
}, [showWorktreeToggle]);
Comment on lines +6392 to +6396

@qwen-code-ci-bot qwen-code-ci-bot Jul 22, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

[Suggestion] The new useEffect that clears worktreePending state when showWorktreeToggle flips to false has no test.

Failure scenario: if a consumer initially renders with showWorktreeToggle={true}, the user enables the worktree intent, and then the consumer re-renders with showWorktreeToggle={false} (e.g., due to a config change), the effect should clear the pending intent. Without a test, a regression that leaves stale worktreePending=true — showing the worktree badge when the toggle feature is disabled — would go undetected.

— qwen3.7-max via Qwen Code /review

@ytahdn ytahdn Jul 22, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, this is a valid regression-test suggestion. The current implementation both gates the badge rendering on showWorktreeToggle and clears the pending intent when the prop becomes false. To keep this UI-polish PR scoped to the requested behavior, we are not adding the dynamic prop-transition test in this PR.

useEffect(() => {
if (!worktreeFocusTarget.current) return;
const target = worktreeFocusTarget.current;
Expand All @@ -6403,7 +6412,7 @@ export function App({
) : (
<WelcomeHeader {...welcomeHeaderProps} />
)}
{worktreePending ? (
{showWorktreeToggle && worktreePending ? (
<div className={styles.worktreeWelcomeBadge}>
<span className={styles.worktreeBadgeIcon}>
<GitForkIcon size={18} strokeWidth={1.8} />
Expand Down Expand Up @@ -6454,6 +6463,7 @@ export function App({
),
[
renderWelcomeHeader,
showWorktreeToggle,
welcomeHeaderProps,
worktreePending,
worktreeToggleEligible,
Expand Down Expand Up @@ -8032,6 +8042,7 @@ export function App({
error={artifactsError}
onSelectTab={setActiveArtifactPanelTabId}
onCloseTab={closeArtifactPanelTab}
onOpenFilePreview={openFilePreview}
onClose={closeArtifactPanel}
variant="drawer"
/>
Expand Down
39 changes: 8 additions & 31 deletions packages/web-shell/client/components/ChatEditor.module.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,14 +190,8 @@
}

:global([data-web-shell-slash-menu]) {
width: min(
max(
240px,
calc(
var(--slash-command-col) + var(--slash-desc-col) +
var(--slash-column-gap) + 20px
)
),
width: 620px;
Comment thread
ytahdn marked this conversation as resolved.
max-width: min(
calc(var(--radix-popover-trigger-width) - 32px),
var(--radix-popover-content-available-width),
calc(100vw - 24px)
Expand All @@ -208,11 +202,7 @@

:global([data-web-shell-slash-detail]) {
z-index: calc(var(--web-shell-popover-z-index, 1000) + 1);
width: min(
320px,
var(--radix-popover-content-available-width),
calc(100vw - 24px)
);
width: min(320px, calc(100vw - 24px));
max-height: min(200px, var(--radix-popover-content-available-height));
overflow: hidden;
}
Expand Down Expand Up @@ -303,8 +293,8 @@
width: 100%;
min-width: 0;
grid-template-columns:
minmax(0, 2fr)
minmax(0, 3fr);
minmax(0, 220px)
minmax(0, 1fr);
column-gap: 8px;
align-items: baseline;
padding: 5px 8px;
Expand Down Expand Up @@ -379,6 +369,7 @@
font-size: 12px;
font-weight: 400;
line-height: 20px;
text-align: right;
text-overflow: ellipsis;
white-space: nowrap;
}
Expand Down Expand Up @@ -695,18 +686,6 @@
font-size: 12px;
}

@container (max-width: 699px) {
.slashList {
width: 100%;
}

.slashItem {
width: 100%;
grid-template-columns: minmax(0, 1fr);
row-gap: 2px;
}
}

.attachments {
display: flex;
min-height: 0;
Expand Down Expand Up @@ -958,14 +937,12 @@
border: 0;
background: transparent;
cursor: pointer;
font: inherit;
color: inherit;
padding: 0;
margin: 0;
}

.gitBranchChipButton:hover {
background: var(--subtle-bg);
background: var(--chat-editor-bg-tertiary);
color: var(--chat-editor-text-primary);
}

.gitBranchChipButton:focus-visible {
Expand Down
48 changes: 26 additions & 22 deletions packages/web-shell/client/components/ChatEditor.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import {
useRef,
useState,
} from 'react';
import type { CSSProperties, ReactNode, RefObject } from 'react';
import type { ReactNode, RefObject } from 'react';
import { Tooltip as TooltipPrimitive } from 'radix-ui';
import { DAEMON_APPROVAL_MODES } from '@qwen-code/webui/daemon-react-sdk';
import type { CommandInfo } from '../adapters/types';
Expand Down Expand Up @@ -838,6 +838,7 @@ function SlashCommandPanel({
const [hoverDetail, setHoverDetail] = useState<{
label: string;
detail: string;
side: 'top' | 'right' | 'bottom' | 'left';
} | null>(null);

useEffect(() => {
Expand Down Expand Up @@ -885,25 +886,6 @@ function SlashCommandPanel({
}, []);

const rowPlans = planSlashSectionRows(menu.items, menu.kind);
const maxLabelLength = Math.max(
...menu.items.map((item) => Array.from(item.label).length),
0,
);
const maxDetailLength = Math.max(
...menu.items.map((item) => Array.from(item.detail ?? '').length),
0,
);
const hasDetailColumn = maxDetailLength > 0;
const panelStyle = {
'--slash-command-col': `${Math.min(
Math.max(maxLabelLength + 1, 10),
24,
)}ch`,
'--slash-desc-col': hasDetailColumn
? `${Math.min(Math.max(maxDetailLength + 1, 18), 36)}ch`
: '0px',
'--slash-column-gap': hasDetailColumn ? '2ch' : '0px',
} as CSSProperties;

return (
<>
Expand All @@ -924,11 +906,12 @@ function SlashCommandPanel({
align="start"
alignOffset={16}
sideOffset={8}
avoidCollisions={false}
Comment thread
ytahdn marked this conversation as resolved.
collisionPadding={12}
collisionBoundary={collisionBoundary ?? undefined}
className="duration-0 data-open:animate-none data-closed:animate-none"
role="listbox"
data-web-shell-slash-menu
style={panelStyle}
onOpenAutoFocus={(event) => event.preventDefault()}
onCloseAutoFocus={(event) => event.preventDefault()}
onInteractOutside={(event) => {
Expand Down Expand Up @@ -1001,9 +984,29 @@ function SlashCommandPanel({
return;
}
hoverAnchorRef.current = event.currentTarget;
const rowRect =
event.currentTarget.getBoundingClientRect();
const boundaryRect =
collisionBoundary?.getBoundingClientRect();
const left = boundaryRect?.left ?? 0;
const right =
boundaryRect?.right ?? window.innerWidth;
const top = boundaryRect?.top ?? 0;
const bottom =
boundaryRect?.bottom ?? window.innerHeight;
const detailWidth = Math.min(320, right - left - 24);
const side =
right - rowRect.right >= detailWidth + 8
? 'right'
: rowRect.left - left >= detailWidth + 8
? 'left'
: rowRect.top - top >= bottom - rowRect.bottom
? 'top'
: 'bottom';
setHoverDetail({
label: item.label,
detail: item.detail,
side,
});
}}
onMouseDown={(event) => {
Expand Down Expand Up @@ -1043,11 +1046,12 @@ function SlashCommandPanel({
{hoverDetail && (
<PopoverContent
ref={detailRef}
side="right"
side={hoverDetail.side}
align="start"
sideOffset={8}
collisionPadding={12}
collisionBoundary={collisionBoundary ?? undefined}
className="duration-0 data-open:animate-none data-closed:animate-none"
data-web-shell-slash-detail
onOpenAutoFocus={(event) => event.preventDefault()}
onCloseAutoFocus={(event) => event.preventDefault()}
Expand Down
4 changes: 4 additions & 0 deletions packages/web-shell/client/components/ChatPane.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -376,6 +376,10 @@ export function ChatPane({
const handleRightPanelOpen = useCallback(
(request: TurnOutputOpenRequest) => {
if (!onRightPanelOpen) return;
if (request.kind === 'subagent') {
onRightPanelOpen(request);
return;
}
onRightPanelOpen({ ...request, workspaceActions });
},
[onRightPanelOpen, workspaceActions],
Expand Down
Loading
Loading