Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
63 changes: 63 additions & 0 deletions apps/desktop/e2e/session-workbar.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,69 @@ async function createSession(page: Page, prompt: string) {
return { composer, sessionId: sessionId!, sidebar };
}

test('a collapsed workbar never flashes during the first send', async ({
window: page,
}) => {
await page.evaluate(() => {
const watch = { visibleRightWorkbar: false };
const inspect = () => {
const panel = document.querySelector<HTMLElement>(
'.maka-session-workbar[data-placement="right"]',
);
if (
panel &&
getComputedStyle(panel).display !== 'none' &&
panel.getBoundingClientRect().width > 0
) {
watch.visibleRightWorkbar = true;
}
};
const observer = new MutationObserver(inspect);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The observer detects by sampling rather than by record: inspect() re-queries the live DOM, so a card added and removed inside one mutation batch leaves records behind but nothing for the query to find. It caught the real regression — you verified it red on main — so this is not a problem today, just the part that would quietly stop catching things. Reading the added nodes out of records would make the detection independent of how fast the flash is.

observer.observe(document.body, {
subtree: true,
childList: true,
attributes: true,
});
(
window as typeof window & {
__makaFirstSendWorkbarWatch?: typeof watch;
__makaFirstSendWorkbarWatchStop?: () => void;
}
).__makaFirstSendWorkbarWatch = watch;
(
window as typeof window & {
__makaFirstSendWorkbarWatchStop?: () => void;
}
).__makaFirstSendWorkbarWatchStop = () => {
inspect();
observer.disconnect();
};
});

const composer = page.locator(COMPOSER_INPUT);
await composer.fill('create a session without opening the workbar');
await page.getByRole('button', { name: '发送' }).click();
const expandWorkbar = page.getByRole('button', { name: '展开任务工作栏' });
await expect(expandWorkbar).toBeVisible({
timeout: 20_000,
});

const watch = await page.evaluate(() => {
const target = window as typeof window & {
__makaFirstSendWorkbarWatch?: { visibleRightWorkbar: boolean };
__makaFirstSendWorkbarWatchStop?: () => void;
};
target.__makaFirstSendWorkbarWatchStop?.();
return target.__makaFirstSendWorkbarWatch;
});
expect(watch?.visibleRightWorkbar, 'the collapsed right workbar stayed hidden').toBe(false);

await expandWorkbar.evaluate((button) => button.click());
await expect(
page.locator('.maka-session-workbar[data-placement="right"]'),
).toBeVisible();
});

async function waitForCompanionForkId(page: Page, sourceSessionId: string) {
let forkId: string | undefined;
await expect
Expand Down
51 changes: 35 additions & 16 deletions apps/desktop/src/renderer/features/workbar/ui/workbar-host.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -46,24 +46,35 @@ const WorkbarSurface = lazy(() =>
})),
);

function SessionWorkbarFallback() {
function SessionWorkbarFallback(props: {
hidden: boolean;
rightCollapsed: boolean;
bottomOpen: boolean;
}) {
const copy = getShellCopy(useUiLocale()).app;
if (props.hidden || (props.rightCollapsed && !props.bottomOpen)) return null;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

[P3] This re-derives a rule that already has an owner. workbar-surface.tsx:742 computes the same fact as

const visible =
  !props.hidden &&
  (placement === 'right' ? !props.rightCollapsed : props.bottomOpen);

and the fallback now expresses it a second time, in a different shape. The two agree today, which is why this is P3 — but a fallback drifting from the resolved surface is exactly the bug this PR is fixing, so leaving a second copy of the rule behind reopens the same seam.

The surface also renders both cards always and hides the invisible one with data-collapsed, while the fallback omits it instead. Same pixels (the CSS makes them equivalent), different DOM. Exporting one predicate and letting the fallback mirror the surface's structure collapses both differences, and the hidden || (rightCollapsed && !bottomOpen) early return then falls out — it is already implied by the placement list being empty.

const placements: SessionWorkbarPlacement[] = [];
if (!props.rightCollapsed) placements.push('right');
if (props.bottomOpen) placements.push('bottom');
return (
<div className="maka-workbar-workspace-contents">
<Card
variant="transparent"
padding={0}
height="100%"
className="maka-session-workbar maka-session-workbar-frame"
data-placement="right"
role="status"
aria-busy="true"
aria-label={copy.loadingWorkbarLabel}
>
<div className="maka-lazy-fallback" data-surface="panel">
<Spinner size="sm" shade="subtle" label={copy.loadingWorkbar} />
</div>
</Card>
{placements.map((placement) => (
<Card
key={placement}
variant="transparent"
padding={0}
height="100%"
className="maka-session-workbar maka-session-workbar-frame"
data-placement={placement}
role="status"
aria-busy="true"
aria-label={copy.loadingWorkbarLabel}
>
<div className="maka-lazy-fallback" data-surface="panel">
<Spinner size="sm" shade="subtle" label={copy.loadingWorkbar} />
</div>
</Card>
))}
</div>
);
}
Expand Down Expand Up @@ -163,7 +174,15 @@ export function WorkbarHost({ model: props }: { model: WorkbarHostModel }) {
)}
{props.activeId && (
<div className="maka-workbar-layout-vars" style={style}>
<Suspense fallback={<SessionWorkbarFallback />}>
<Suspense
fallback={
<SessionWorkbarFallback
hidden={props.hidden}
rightCollapsed={props.rightCollapsed}
bottomOpen={props.bottomOpen}
/>
}
>
<WorkbarSurface
key={props.activeId}
sessionId={props.activeId}
Expand Down