Merge branch 'main' into feat/gh-2634-pending-question
This commit is contained in:
@@ -37,6 +37,7 @@ import { useGlobalSessionsStore } from '@/stores/useGlobalSessionsStore';
|
||||
import { getContextObligatoryMessages } from '@/lib/contextObligatoryMessages';
|
||||
import { setContextObligatoryMessage } from '@/sync/session-actions';
|
||||
import { isVSCodeRuntime } from '@/lib/desktop';
|
||||
import { focusChatInput } from './composer/editor/dom';
|
||||
|
||||
const ToolOutputDialog = lazyWithChunkRecovery(() => import('./message/ToolOutputDialog'));
|
||||
|
||||
@@ -416,6 +417,10 @@ const ChatMessage: React.FC<ChatMessageProps> = ({
|
||||
createdAt: messageCreatedAt,
|
||||
role: isUser ? 'user' : 'assistant',
|
||||
}, !isPinnedIntoContext);
|
||||
// Return focus to the composer so the user can keep typing right
|
||||
// after adding the message to context (matches the refocus pattern
|
||||
// used by the model/agent selectors).
|
||||
requestAnimationFrame(focusChatInput);
|
||||
} catch (error) {
|
||||
console.error('[chat-message] failed to update context pin', error);
|
||||
toast.error(t('chat.messageBody.actions.contextPinFailed'));
|
||||
|
||||
@@ -45,6 +45,7 @@ import {
|
||||
type EmbeddedSessionRuntimeBootstrap,
|
||||
} from './contextPanelEmbeddedChat';
|
||||
import { getContextSurfaceWidthFraction } from '@/lib/surfaces/registry';
|
||||
import { isTerminalEventTarget } from '@/lib/terminalFocus';
|
||||
import {
|
||||
type PreviewElementMetadata,
|
||||
isPreviewElementMetadata,
|
||||
@@ -2453,6 +2454,13 @@ export const ContextPanel: React.FC = () => {
|
||||
return;
|
||||
}
|
||||
|
||||
// Terminal owns Escape so the PTY receives it (e.g. Vim Normal mode).
|
||||
// ghostty-web listens in the bubble phase; stopping capture here would
|
||||
// swallow the key before the terminal ever sees it (issue #2644).
|
||||
if (isTerminalEventTarget(event.target)) {
|
||||
return;
|
||||
}
|
||||
|
||||
event.preventDefault();
|
||||
event.stopPropagation();
|
||||
handleClose();
|
||||
|
||||
@@ -49,17 +49,30 @@ type RailItemProps = {
|
||||
showActivityDot: boolean;
|
||||
label: string;
|
||||
description: string;
|
||||
/** Numeric badge (e.g. the Git changed-files count); takes precedence over the activity dot. */
|
||||
badgeCount?: number | null;
|
||||
/** Accessible label that includes the badge count; falls back to `label`. */
|
||||
badgeAriaLabel?: string | null;
|
||||
/** Extra tooltip line describing the badge; rendered under the description. */
|
||||
badgeDescription?: string | null;
|
||||
orderNumber?: number | null;
|
||||
showOrderNumber?: boolean;
|
||||
onSelect: (surface: ContextSurfaceDescriptor) => void;
|
||||
};
|
||||
|
||||
// The badge corner is 16px tall; cap large counts so the pill stays compact
|
||||
// on the 36px rail button (matching the order-number badge's footprint).
|
||||
const formatRailBadgeCount = (count: number): string => (count > 99 ? '99+' : String(count));
|
||||
|
||||
const ContextPanelRailItem: React.FC<RailItemProps> = ({
|
||||
surface,
|
||||
isActive,
|
||||
showActivityDot,
|
||||
label,
|
||||
description,
|
||||
badgeCount,
|
||||
badgeAriaLabel,
|
||||
badgeDescription,
|
||||
orderNumber,
|
||||
showOrderNumber,
|
||||
onSelect,
|
||||
@@ -68,6 +81,8 @@ const ContextPanelRailItem: React.FC<RailItemProps> = ({
|
||||
id: surface.id,
|
||||
});
|
||||
|
||||
const displayBadgeCount = badgeCount != null && badgeCount > 0 ? formatRailBadgeCount(badgeCount) : null;
|
||||
|
||||
return (
|
||||
<div
|
||||
ref={setNodeRef}
|
||||
@@ -81,7 +96,7 @@ const ContextPanelRailItem: React.FC<RailItemProps> = ({
|
||||
{...attributes}
|
||||
{...listeners}
|
||||
onClick={() => onSelect(surface)}
|
||||
aria-label={label}
|
||||
aria-label={badgeAriaLabel ?? label}
|
||||
aria-pressed={isActive}
|
||||
className={cn(
|
||||
'flex h-9 w-9 touch-none select-none items-center justify-center rounded-md transition-colors',
|
||||
@@ -95,12 +110,6 @@ const ContextPanelRailItem: React.FC<RailItemProps> = ({
|
||||
) : (
|
||||
<Icon name={surface.icon} className="h-[18px] w-[18px]" />
|
||||
)}
|
||||
{showActivityDot && !showOrderNumber ? (
|
||||
<span
|
||||
aria-hidden="true"
|
||||
className="absolute right-1 top-1 h-1.5 w-1.5 rounded-full bg-[var(--status-info)]"
|
||||
/>
|
||||
) : null}
|
||||
{showOrderNumber && orderNumber != null ? (
|
||||
<span
|
||||
aria-hidden="true"
|
||||
@@ -108,6 +117,18 @@ const ContextPanelRailItem: React.FC<RailItemProps> = ({
|
||||
>
|
||||
{orderNumber === 10 ? '0' : orderNumber}
|
||||
</span>
|
||||
) : displayBadgeCount ? (
|
||||
<span
|
||||
aria-hidden="true"
|
||||
className="absolute right-0 top-0 flex h-4 min-w-4 items-center justify-center rounded-full bg-surface-muted px-1 text-[0.625rem] font-medium leading-none text-muted-foreground"
|
||||
>
|
||||
{displayBadgeCount}
|
||||
</span>
|
||||
) : showActivityDot ? (
|
||||
<span
|
||||
aria-hidden="true"
|
||||
className="absolute right-1 top-1 h-1.5 w-1.5 rounded-full bg-[var(--status-info)]"
|
||||
/>
|
||||
) : null}
|
||||
</button>
|
||||
</TooltipTrigger>
|
||||
@@ -115,6 +136,9 @@ const ContextPanelRailItem: React.FC<RailItemProps> = ({
|
||||
<div className="flex flex-col gap-0.5">
|
||||
<span>{label}</span>
|
||||
<span className="typography-micro text-muted-foreground">{description}</span>
|
||||
{badgeDescription ? (
|
||||
<span className="typography-micro text-muted-foreground">{badgeDescription}</span>
|
||||
) : null}
|
||||
</div>
|
||||
</TooltipContent>
|
||||
</Tooltip>
|
||||
@@ -258,19 +282,43 @@ export const ContextPanelRail: React.FC = () => {
|
||||
>
|
||||
<DndContext sensors={sensors} collisionDetection={closestCenter} onDragEnd={handleDragEnd}>
|
||||
<SortableContext items={surfaces.map((surface) => surface.id)} strategy={verticalListSortingStrategy}>
|
||||
{surfaces.map((surface, index) => (
|
||||
<ContextPanelRailItem
|
||||
key={surface.id}
|
||||
surface={surface}
|
||||
isActive={activeMode === surface.mode}
|
||||
showActivityDot={surface.id === 'git' && changedFilesCount > 0}
|
||||
label={t(surface.labelKey)}
|
||||
description={t(surface.descriptionKey)}
|
||||
orderNumber={index + 1}
|
||||
showOrderNumber={revealNumbers}
|
||||
onSelect={(selected) => openContextSurface(directoryKey, selected.mode)}
|
||||
/>
|
||||
))}
|
||||
{surfaces.map((surface, index) => {
|
||||
const label = t(surface.labelKey);
|
||||
// Git shows a numeric badge instead of the old activity dot.
|
||||
// Other surfaces never inherit git's changed-files signal.
|
||||
const gitChangedCount = surface.id === 'git' ? changedFilesCount : 0;
|
||||
const badgeCount = gitChangedCount > 0 ? gitChangedCount : null;
|
||||
return (
|
||||
<ContextPanelRailItem
|
||||
key={surface.id}
|
||||
surface={surface}
|
||||
isActive={activeMode === surface.mode}
|
||||
showActivityDot={false}
|
||||
label={label}
|
||||
description={t(surface.descriptionKey)}
|
||||
badgeCount={badgeCount}
|
||||
badgeAriaLabel={badgeCount !== null
|
||||
? t(
|
||||
badgeCount === 1
|
||||
? 'contextRail.surface.git.changesCountAriaSingle'
|
||||
: 'contextRail.surface.git.changesCountAriaPlural',
|
||||
{ label, count: badgeCount },
|
||||
)
|
||||
: null}
|
||||
badgeDescription={badgeCount !== null
|
||||
? t(
|
||||
badgeCount === 1
|
||||
? 'contextRail.surface.git.changesCountTooltipSingle'
|
||||
: 'contextRail.surface.git.changesCountTooltipPlural',
|
||||
{ count: badgeCount },
|
||||
)
|
||||
: null}
|
||||
orderNumber={index + 1}
|
||||
showOrderNumber={revealNumbers}
|
||||
onSelect={(selected) => openContextSurface(directoryKey, selected.mode)}
|
||||
/>
|
||||
);
|
||||
})}
|
||||
</SortableContext>
|
||||
</DndContext>
|
||||
</nav>
|
||||
|
||||
@@ -0,0 +1,187 @@
|
||||
/**
|
||||
* Regression guard for https://github.com/openchamber/openchamber/issues/2644
|
||||
*
|
||||
* Escape while focus is inside the terminal must reach the PTY (e.g. Vim
|
||||
* Normal mode). The context panel still closes on Escape when focus is on
|
||||
* non-terminal panel chrome.
|
||||
*/
|
||||
import { describe, expect, test } from 'bun:test';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { dirname, join } from 'node:path';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
|
||||
const __dirname = dirname(fileURLToPath(import.meta.url));
|
||||
const contextPanelSource = readFileSync(join(__dirname, '..', 'ContextPanel.tsx'), 'utf-8');
|
||||
const mobileWorkspaceDrawerSource = readFileSync(
|
||||
join(__dirname, '..', '..', '..', 'apps', 'MobileWorkspaceDrawer.tsx'),
|
||||
'utf-8',
|
||||
);
|
||||
|
||||
describe('issue #2644: Escape in terminal must not close the context panel', () => {
|
||||
test('the context panel captures Escape at the panel level', () => {
|
||||
expect(contextPanelSource).toContain('onKeyDownCapture={handlePanelKeyDownCapture}');
|
||||
});
|
||||
|
||||
test('the capture handler skips closing when the event target is inside the terminal', () => {
|
||||
const start = contextPanelSource.indexOf('const handlePanelKeyDownCapture = React.useCallback(');
|
||||
expect(start).toBeGreaterThan(-1);
|
||||
const end = contextPanelSource.indexOf('}, [handleClose]);', start);
|
||||
expect(end).toBeGreaterThan(start);
|
||||
const handler = contextPanelSource.slice(start, end);
|
||||
|
||||
expect(handler).toContain("event.key !== 'Escape'");
|
||||
expect(handler).toContain('isTerminalEventTarget(event.target)');
|
||||
expect(handler).toContain('event.preventDefault()');
|
||||
expect(handler).toContain('event.stopPropagation()');
|
||||
expect(handler).toContain('handleClose()');
|
||||
|
||||
// Guard must return before preventDefault/stopPropagation so ghostty-web's
|
||||
// bubble-phase keydown listener can forward Escape to the PTY.
|
||||
const guardIndex = handler.indexOf('isTerminalEventTarget(event.target)');
|
||||
const preventIndex = handler.indexOf('event.preventDefault()');
|
||||
expect(guardIndex).toBeGreaterThan(-1);
|
||||
expect(preventIndex).toBeGreaterThan(guardIndex);
|
||||
});
|
||||
|
||||
test('ContextPanel imports the shared terminal focus helper', () => {
|
||||
expect(contextPanelSource).toContain("from '@/lib/terminalFocus'");
|
||||
expect(contextPanelSource).toContain('isTerminalEventTarget');
|
||||
});
|
||||
|
||||
test('mobile drawer keeps its terminal Escape exception', () => {
|
||||
const handlerStart = mobileWorkspaceDrawerSource.indexOf("if (event.key === 'Escape'");
|
||||
expect(handlerStart).toBeGreaterThan(-1);
|
||||
const handler = mobileWorkspaceDrawerSource.slice(handlerStart, handlerStart + 200);
|
||||
expect(handler).toContain("tabRef.current !== 'terminal'");
|
||||
});
|
||||
});
|
||||
|
||||
type Listener = { capture: boolean; onEvent: (event: SimulatedEvent) => void };
|
||||
type SimulatedEvent = {
|
||||
type: string;
|
||||
defaultPrevented: boolean;
|
||||
propagationStopped: boolean;
|
||||
target: SimNode;
|
||||
preventDefault(): void;
|
||||
stopPropagation(): void;
|
||||
};
|
||||
|
||||
class SimNode {
|
||||
readonly children: SimNode[] = [];
|
||||
private listeners: Listener[] = [];
|
||||
private parent: SimNode | null = null;
|
||||
|
||||
addListener(listener: Listener): void {
|
||||
this.listeners.push(listener);
|
||||
}
|
||||
|
||||
attach(child: SimNode): void {
|
||||
child.parent = this;
|
||||
this.children.push(child);
|
||||
}
|
||||
|
||||
dispatch(type: string): SimulatedEvent {
|
||||
const buildPath = (target: SimNode): SimNode[] => {
|
||||
const ancestors: SimNode[] = [];
|
||||
let cursor: SimNode | null = target;
|
||||
while (cursor !== null) {
|
||||
ancestors.push(cursor);
|
||||
cursor = cursor.parent;
|
||||
}
|
||||
ancestors.reverse();
|
||||
return ancestors;
|
||||
};
|
||||
const path = buildPath(this);
|
||||
|
||||
const event: SimulatedEvent = {
|
||||
type,
|
||||
defaultPrevented: false,
|
||||
propagationStopped: false,
|
||||
target: this,
|
||||
preventDefault() {
|
||||
event.defaultPrevented = true;
|
||||
},
|
||||
stopPropagation() {
|
||||
event.propagationStopped = true;
|
||||
},
|
||||
};
|
||||
|
||||
for (let i = 0; i < path.length; i += 1) {
|
||||
if (event.propagationStopped) return event;
|
||||
for (const listener of path[i].listeners) {
|
||||
if (!listener.capture) continue;
|
||||
listener.onEvent(event);
|
||||
if (event.propagationStopped) return event;
|
||||
}
|
||||
}
|
||||
for (let i = path.length - 1; i >= 0; i -= 1) {
|
||||
if (event.propagationStopped) return event;
|
||||
for (const listener of path[i].listeners) {
|
||||
if (listener.capture) continue;
|
||||
listener.onEvent(event);
|
||||
if (event.propagationStopped) return event;
|
||||
}
|
||||
}
|
||||
return event;
|
||||
}
|
||||
}
|
||||
|
||||
describe('issue #2644: fixed Escape propagation to the terminal', () => {
|
||||
test('when the panel skips terminal Escape, the terminal bubble handler receives it', () => {
|
||||
const panel = new SimNode();
|
||||
const terminalContainer = new SimNode();
|
||||
panel.attach(terminalContainer);
|
||||
|
||||
const calls: string[] = [];
|
||||
const panelEscapeHandler = (event: SimulatedEvent) => {
|
||||
// Fixed behavior: do not close / stop when the target is the terminal.
|
||||
if (event.target === terminalContainer) {
|
||||
calls.push('panel-capture-skipped');
|
||||
return;
|
||||
}
|
||||
calls.push('panel-capture-closed');
|
||||
event.preventDefault();
|
||||
event.stopPropagation();
|
||||
};
|
||||
const terminalKeydownHandler = () => {
|
||||
calls.push('terminal-bubble');
|
||||
};
|
||||
|
||||
panel.addListener({ capture: true, onEvent: panelEscapeHandler });
|
||||
terminalContainer.addListener({ capture: false, onEvent: terminalKeydownHandler });
|
||||
|
||||
const event = terminalContainer.dispatch('keydown');
|
||||
|
||||
expect(calls).toEqual(['panel-capture-skipped', 'terminal-bubble']);
|
||||
expect(event.propagationStopped).toBe(false);
|
||||
expect(event.defaultPrevented).toBe(false);
|
||||
});
|
||||
|
||||
test('Escape outside the terminal still closes via the capture handler', () => {
|
||||
const panel = new SimNode();
|
||||
const headerButton = new SimNode();
|
||||
const terminalContainer = new SimNode();
|
||||
panel.attach(headerButton);
|
||||
panel.attach(terminalContainer);
|
||||
|
||||
const calls: string[] = [];
|
||||
panel.addListener({
|
||||
capture: true,
|
||||
onEvent: (event) => {
|
||||
if (event.target === terminalContainer) return;
|
||||
calls.push('panel-capture-closed');
|
||||
event.preventDefault();
|
||||
event.stopPropagation();
|
||||
},
|
||||
});
|
||||
terminalContainer.addListener({
|
||||
capture: false,
|
||||
onEvent: () => calls.push('terminal-bubble'),
|
||||
});
|
||||
|
||||
const event = headerButton.dispatch('keydown');
|
||||
expect(calls).toEqual(['panel-capture-closed']);
|
||||
expect(event.propagationStopped).toBe(true);
|
||||
expect(event.defaultPrevented).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -463,6 +463,14 @@ export function ScheduledTasksDialog() {
|
||||
<div className="typography-micro truncate text-muted-foreground">
|
||||
{formatSchedule(task, t)}
|
||||
</div>
|
||||
{task.loopFile ? (
|
||||
<div
|
||||
className="typography-micro truncate text-muted-foreground/70"
|
||||
title={task.loopFile}
|
||||
>
|
||||
{t('sessions.scheduledTasks.dialog.loopFile.note', { file: task.loopFile })}
|
||||
</div>
|
||||
) : null}
|
||||
</div>
|
||||
|
||||
<div className="mt-3 flex flex-wrap items-center gap-x-5 gap-y-1 typography-micro text-muted-foreground">
|
||||
@@ -525,8 +533,11 @@ export function ScheduledTasksDialog() {
|
||||
className={cn(
|
||||
'inline-flex cursor-pointer items-center gap-2 typography-micro font-medium',
|
||||
task.enabled ? 'text-foreground' : 'text-muted-foreground',
|
||||
isBusy && 'cursor-not-allowed opacity-50',
|
||||
(isBusy || task.loopFile) && 'cursor-not-allowed opacity-50',
|
||||
)}
|
||||
title={task.loopFile
|
||||
? t('sessions.scheduledTasks.dialog.loopFile.toggleDisabled')
|
||||
: undefined}
|
||||
>
|
||||
<Checkbox
|
||||
checked={task.enabled}
|
||||
@@ -534,7 +545,7 @@ export function ScheduledTasksDialog() {
|
||||
ariaLabel={task.enabled
|
||||
? t('sessions.scheduledTasks.dialog.taskToggle.pauseAria', { taskName: task.name })
|
||||
: t('sessions.scheduledTasks.dialog.taskToggle.enableAria', { taskName: task.name })}
|
||||
disabled={isBusy}
|
||||
disabled={isBusy || Boolean(task.loopFile)}
|
||||
/>
|
||||
{task.enabled ? t('sessions.scheduledTasks.dialog.taskToggle.enabled') : t('sessions.scheduledTasks.dialog.taskToggle.paused')}
|
||||
</label>
|
||||
@@ -555,7 +566,10 @@ export function ScheduledTasksDialog() {
|
||||
setEditorTask(task);
|
||||
setEditorOpen(true);
|
||||
}}
|
||||
disabled={isBusy}
|
||||
disabled={isBusy || Boolean(task.loopFile)}
|
||||
title={task.loopFile
|
||||
? t('sessions.scheduledTasks.dialog.loopFile.actionsDisabled')
|
||||
: undefined}
|
||||
aria-label={t('sessions.scheduledTasks.dialog.actions.editAria', { taskName: task.name })}
|
||||
>
|
||||
<Icon name="edit-2" className="h-4 w-4" /> {t('sessions.scheduledTasks.dialog.actions.edit')}
|
||||
@@ -564,7 +578,10 @@ export function ScheduledTasksDialog() {
|
||||
variant="destructive"
|
||||
size="sm"
|
||||
onClick={() => void handleDeleteTask(task)}
|
||||
disabled={isBusy}
|
||||
disabled={isBusy || Boolean(task.loopFile)}
|
||||
title={task.loopFile
|
||||
? t('sessions.scheduledTasks.dialog.loopFile.actionsDisabled')
|
||||
: undefined}
|
||||
aria-label={t('sessions.scheduledTasks.dialog.actions.deleteAria', { taskName: task.name })}
|
||||
>
|
||||
<Icon name="delete-bin" className="h-4 w-4" />
|
||||
|
||||
Reference in New Issue
Block a user