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
1 change: 1 addition & 0 deletions packages/web-shell/client/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10149,6 +10149,7 @@ export function App({
composerInput={composerInput}
composerInputVersion={composerInputVersion}
placeholderText={composerPlaceholderText}
animatePlaceholder={isChatEmptyState}
Comment on lines 10150 to +10152

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.

Declined — not worth the diff-growth/flakiness tradeoff. The component-level default (animatePlaceholder=true) is already pinned: the pre-existing ChatEditor typewriter test renders the component with the prop omitted and asserts the typewriter mounts and plays. The remaining gap you correctly identify is the App-level wiring's positive branch (welcome state → typewriter present). Pinning that needs a welcome-state (/) e2e or an App-render test; every existing web-shell e2e visits /session/<id> against the mock daemon, so a welcome-state spec would add new scaffolding and flakiness risk disproportionate to the risk of a single-line, review-visible prop pass-through mutating to false. Happy to add a welcome-state e2e if a maintainer wants both sides of the wiring pinned.

中文说明

已拒绝——不值得以 diff 膨胀/不稳定性为代价。组件层默认值(animatePlaceholder=true)已被固定:已有的 ChatEditor 打字机测试在不传该 prop 的情况下渲染组件,并断言打字机挂载并播放。你正确指出的剩余缺口是 App 层接线的正向分支(欢迎状态 → 打字机存在)。要固定它需要一个欢迎状态(/)e2e 或 App 渲染测试;而现有所有 web-shell e2e 都在 mock daemon 下访问 /session/<id>,因此一个欢迎状态的用例会引入新的脚手架,且其不稳定性风险与“单行、审查可见的 prop 透传被改为 false”这一低风险不相称。如果维护者希望固定接线的两侧,我很乐意补充一个欢迎状态的 e2e。

/>
{CustomComposerFooter && (
<CustomComposerFooter
Expand Down
13 changes: 13 additions & 0 deletions packages/web-shell/client/components/ChatEditor.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@ function renderChatEditor(props: {
onSelectModel?: (model: string) => void;
onAttachmentsChange?: (hasAttachments: boolean) => void;
placeholderText?: string;
animatePlaceholder?: boolean;
disabled?: boolean;
followupState?: UseDaemonFollowupSuggestionReturn['followupState'];
customization?: WebShellCustomization;
Expand Down Expand Up @@ -400,6 +401,18 @@ describe('ChatEditor animation layers', () => {
expect(container.querySelector('[data-typewriter-visible]')).toBeNull();
});

it('does not mount the typewriter when placeholder animation is disabled', () => {
const container = renderChatEditor({
placeholderText: 'abc',
animatePlaceholder: false,
});

expect(
container.querySelector('[data-web-shell-composer-typewriter]'),
).toBeNull();
expect(container.querySelector('[data-typewriter-visible]')).toBeNull();
});

it('shows the full placeholder without a caret under prefers-reduced-motion', () => {
const originalMatchMedia = window.matchMedia;
window.matchMedia = vi.fn(
Expand Down
3 changes: 3 additions & 0 deletions packages/web-shell/client/components/ChatEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,7 @@ interface ChatEditorProps {
cancelArmed?: boolean;
disabled?: boolean;
placeholderText?: string;
animatePlaceholder?: boolean;
commands: CommandInfo[];
skills?: SkillInfo[];
slashCommandCategoryOrder?: CommandDisplayCategoryOrder;
Expand Down Expand Up @@ -1148,6 +1149,7 @@ export const ChatEditor = memo(
cancelArmed = false,
disabled = false,
placeholderText = 'Type a message...',
animatePlaceholder = true,
commands,
skills = [],
slashCommandCategoryOrder,
Expand Down Expand Up @@ -1285,6 +1287,7 @@ export const ChatEditor = memo(
const hasSlashMenu = Boolean(slashMenu);
const hasAtMenu = Boolean(atMenu);
const showTypewriterPlaceholder =
animatePlaceholder &&
!disabled &&
Boolean(placeholderText) &&
!core.hasInput() &&
Expand Down
1 change: 1 addition & 0 deletions packages/web-shell/client/components/ChatPane.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -881,6 +881,7 @@ export function ChatPane({
sessionId={connection.sessionId}
atWorkspaceCwd={paneWorkspaceCwd}
placeholderText={t('splitView.composerPlaceholder')}
animatePlaceholder={false}
/>
{CustomComposerFooter && (
<CustomComposerFooter
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,7 @@ describe('specular effect pointer leave', () => {
});

describe('specular effect WebGL cleanup', () => {
it('releases WebGL resources on unmount', () => {
it('removes a lost canvas and releases WebGL resources on unmount', () => {
const loseContext = vi.fn();
const glStub = {
ARRAY_BUFFER: 0x8892,
Expand Down Expand Up @@ -521,8 +521,12 @@ describe('specular effect WebGL cleanup', () => {
vi.spyOn(HTMLCanvasElement.prototype, 'getContext').mockReturnValue(
glStub as unknown as WebGL2RenderingContext,
);
vi.spyOn(window, 'requestAnimationFrame').mockImplementation(() => 1);
vi.spyOn(window, 'cancelAnimationFrame').mockImplementation(() => {});
const requestAnimationFrameSpy = vi
.spyOn(window, 'requestAnimationFrame')
.mockImplementation(() => 1);
const cancelAnimationFrameSpy = vi
.spyOn(window, 'cancelAnimationFrame')
.mockImplementation(() => {});

function Harness() {
const composerRef = useRef<HTMLDivElement>(null);
Expand All @@ -545,6 +549,38 @@ describe('specular effect WebGL cleanup', () => {
container.querySelector('[data-web-shell-composer-specular] canvas'),
).not.toBeNull();

const removeWindowListenerSpy = vi.spyOn(window, 'removeEventListener');
const removeDocumentListenerSpy = vi.spyOn(
document.documentElement,
'removeEventListener',
);

const canvas = container.querySelector(
'[data-web-shell-composer-specular] canvas',
)!;
act(() => canvas.dispatchEvent(new Event('webglcontextlost')));

expect(cancelAnimationFrameSpy).toHaveBeenCalled();
expect(removeWindowListenerSpy).toHaveBeenCalledWith(
'pointermove',
expect.any(Function),
);
expect(removeDocumentListenerSpy).toHaveBeenCalledWith(
'pointerleave',
expect.any(Function),
);
expect(
container.querySelector('[data-web-shell-composer-specular] canvas'),
).toBeNull();

requestAnimationFrameSpy.mockClear();
act(() => {
window.dispatchEvent(
new MouseEvent('pointermove', { clientX: 100, clientY: 100 }),
);
});
expect(requestAnimationFrameSpy).not.toHaveBeenCalled();

act(() => root.unmount());
container.remove();

Expand Down
36 changes: 24 additions & 12 deletions packages/web-shell/client/components/SpecularComposerEffect.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ export function SpecularComposerEffect({
};

const startLoop = () => {
if (!running) {
if (!running && !disposed) {
running = true;
lastFrame = performance.now();
frameId = window.requestAnimationFrame(draw);
Expand Down Expand Up @@ -303,6 +303,28 @@ export function SpecularComposerEffect({
};

const resizeObserver = new ResizeObserver(resize);
let disposed = false;
const teardown = () => {
if (disposed) return;
disposed = true;
running = false;
window.cancelAnimationFrame(frameId);
frameId = 0;
resizeObserver.disconnect();
window.removeEventListener('pointermove', onPointerMove);
document.documentElement.removeEventListener(
'pointerleave',
onPointerLeave,
);
target.removeEventListener('focusin', onFocusIn);
target.removeEventListener('focusout', onFocusOut);
canvas.removeEventListener('webglcontextlost', onContextLost);
canvas.remove();
};
const onContextLost = () => {
teardown();
};
canvas.addEventListener('webglcontextlost', onContextLost);
resizeObserver.observe(target);
resize();
window.addEventListener('pointermove', onPointerMove, { passive: true });
Expand All @@ -318,17 +340,7 @@ export function SpecularComposerEffect({
startLoop();

return () => {
running = false;
window.cancelAnimationFrame(frameId);
resizeObserver.disconnect();
window.removeEventListener('pointermove', onPointerMove);
document.documentElement.removeEventListener(
'pointerleave',
onPointerLeave,
);
target.removeEventListener('focusin', onFocusIn);
target.removeEventListener('focusout', onFocusOut);
canvas.remove();
teardown();
gl.deleteBuffer(buffer);
gl.deleteProgram(program);
gl.deleteShader(vertexShader);
Expand Down
32 changes: 32 additions & 0 deletions packages/web-shell/client/e2e/web-shell.composer.mobile.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,38 @@ test('renders the textarea backend instead of CodeMirror on touch devices', asyn

await expect(page.locator(COMPOSER_TEXTAREA)).toBeVisible();
await expect(page.locator('.cm-editor')).toHaveCount(0);
await expect(
page.locator('[data-web-shell-composer-typewriter]'),
).toHaveCount(0);
});

test('keeps the composer usable after WebGL context loss', async ({
page,
}, testInfo) => {
const scenario = createWebShellDaemonScenario();
const daemon = await installScenario(page, scenario, testInfo);

await gotoSession(page, scenario, daemon);
const hasWebgl2 = await page.evaluate(
() => !!document.createElement('canvas').getContext('webgl2'),
);
test.skip(!hasWebgl2, 'WebGL2 is unavailable on this runner');
const canvas = page.locator('[data-web-shell-composer-specular] canvas');
await expect(canvas).toBeVisible();

await canvas.evaluate((element) => {
const gl = element.getContext('webgl2');
const extension = gl?.getExtension('WEBGL_lose_context');
if (!extension) throw new Error('WEBGL_lose_context unavailable');
extension.loseContext();
});

await expect(canvas).toHaveCount(0);
const textarea = page.locator(COMPOSER_TEXTAREA);
await expect(textarea).toBeVisible();
await textarea.fill('still usable');
await expect(textarea).toHaveValue('still usable');
await expect(page.locator('[data-web-shell-composer-submit]')).toBeEnabled();
});

test('keeps voice controls reachable on an extra-narrow touch viewport', async ({
Expand Down
Loading