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
56 changes: 56 additions & 0 deletions apps/desktop/src/main/__tests__/menu.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';

const mocks = vi.hoisted(() => ({
close: vi.fn(),
template: [] as Electron.MenuItemConstructorOptions[],
}));

vi.mock('electron', () => ({
app: {
commandLine: {
getSwitchValue: () => '',
hasSwitch: () => false,
},
isPackaged: false,
},
BrowserWindow: {
getAllWindows: () => [],
getFocusedWindow: () => ({ close: mocks.close }),
},
dialog: { showErrorBox: vi.fn() },
Menu: {
buildFromTemplate(template: Electron.MenuItemConstructorOptions[]) {
mocks.template = template;
return {};
},
},
}));

beforeEach(() => {
vi.resetModules();
vi.spyOn(process, 'platform', 'get').mockReturnValue('darwin');
mocks.close.mockReset();
mocks.template = [];
});

afterEach(() => {
vi.restoreAllMocks();
});

describe('desktop app menu', () => {
it('leaves Cmd+W to the renderer', async () => {
const { buildAppMenu } = await import('../menu');

buildAppMenu();

const fileMenu = mocks.template.find((item) => item.label === 'File');
const closeItem = Array.isArray(fileMenu?.submenu) ? fileMenu.submenu[0] : undefined;
expect(closeItem).toMatchObject({ label: 'Close Window' });
expect(closeItem).not.toHaveProperty('accelerator');

if (typeof closeItem === 'object' && 'click' in closeItem && closeItem.click) {
Reflect.apply(closeItem.click, undefined, []);
}
expect(mocks.close).toHaveBeenCalledOnce();
});
});
6 changes: 5 additions & 1 deletion apps/desktop/src/main/menu.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ function openSettings(): void {
win?.webContents.send(SETTINGS_OPEN_CHANNEL);
}

function closeWindow(): void {
BrowserWindow.getFocusedWindow()?.close();
}

export function buildAppMenu(): Menu {
const isMac = process.platform === 'darwin';
const settingsItem: MenuItemConstructorOptions = {
Expand Down Expand Up @@ -45,7 +49,7 @@ export function buildAppMenu(): Menu {
{
label: 'File',
submenu: isMac
? [{ role: 'close' }]
? [{ label: 'Close Window', click: closeWindow }]
: [settingsItem, { type: 'separator' }, { role: 'quit' }],
},
{ role: 'editMenu' },
Expand Down
44 changes: 28 additions & 16 deletions apps/desktop/src/renderer/src/shell/desktop-shell.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -322,6 +322,9 @@ export function DesktopShell({
useDesktopShellShortcuts({
navigation,
owner: shellRootRef,
closeBottomTerminalTab: closeTab,
closeRightTerminalTab,
closeWindow: () => systemBridge.window.close(),
togglePanel,
updateSidebarOpen,
});
Expand Down Expand Up @@ -526,17 +529,23 @@ export function DesktopShell({
const items = rightPanel.terminal.tabs.map((tab) => ({
id: tab.id,
active: activeIsTerminal && tab.id === rightPanel.terminal.activeTabId,
node: tab.id.startsWith('attach:') ? (
<AttachedTerminalPanel
terminalId={tab.id.slice('attach:'.length)}
suspended={rightTransition.phase !== 'open' || shellAnimating}
/>
) : (
<TerminalPanel
sessionKey={tab.id}
cwd={active?.cwd}
suspended={rightTransition.phase !== 'open' || shellAnimating}
/>
node: (
<div className="h-full" data-terminal-panel="right" data-terminal-tab={tab.id}>
{tab.id.startsWith('attach:') ? (
<AttachedTerminalPanel
terminalId={tab.id.slice('attach:'.length)}
suspended={rightTransition.phase !== 'open' || shellAnimating}
onExit={() => closeRightTerminalTab(tab.id)}
/>
) : (
<TerminalPanel
sessionKey={tab.id}
cwd={active?.cwd}
suspended={rightTransition.phase !== 'open' || shellAnimating}
onExit={() => closeRightTerminalTab(tab.id)}
/>
)}
</div>
),
}));
// Browser webviews live here permanently: unmounting or DOM-moving a webview
Expand All @@ -559,11 +568,14 @@ export function DesktopShell({
active: tab.id === bottomPanel.activeTabId,
node:
tab.type === 'terminal' ? (
<TerminalPanel
sessionKey={tab.id}
cwd={active?.cwd}
suspended={bottomTransition.phase !== 'open' || shellAnimating}
/>
<div className="h-full" data-terminal-panel="bottom" data-terminal-tab={tab.id}>
<TerminalPanel
sessionKey={tab.id}
cwd={active?.cwd}
suspended={bottomTransition.phase !== 'open' || shellAnimating}
onExit={() => closeTab(tab.id)}
/>
</div>
) : (
<PanelStubContent type={tab.type} />
),
Expand Down
29 changes: 29 additions & 0 deletions apps/desktop/src/renderer/src/shell/use-desktop-shell-shortcuts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type { PanelSide } from '@linkcode/ui/shell/panels';
import type { WorkbenchShellNavigation } from '@linkcode/workbench';

const TOGGLE_SIDEBAR_SHORTCUT = { code: 'KeyB', modifiers: ['primary'] } as const;
const CLOSE_TERMINAL_TAB_SHORTCUT = { code: 'KeyW', modifiers: ['primary'] } as const;
const TOGGLE_BOTTOM_PANEL_SHORTCUT = { code: 'KeyJ', modifiers: ['primary'] } as const;
const TOGGLE_RIGHT_PANEL_SHORTCUT = {
code: 'KeyB',
Expand All @@ -20,16 +21,44 @@ const GO_FORWARD_SHORTCUT = {
interface UseDesktopShellShortcutsOptions {
navigation: WorkbenchShellNavigation;
owner: React.RefObject<Element | null>;
closeBottomTerminalTab: (id: string) => void;
closeRightTerminalTab: (id: string) => void;
closeWindow: () => unknown;
togglePanel: (side: PanelSide) => void;
updateSidebarOpen: (updater: boolean | ((current: boolean) => boolean)) => void;
}

export function useDesktopShellShortcuts({
navigation,
owner,
closeBottomTerminalTab,
closeRightTerminalTab,
closeWindow,
togglePanel,
updateSidebarOpen,
}: UseDesktopShellShortcutsOptions): void {
useKeyboardShortcut({
Comment thread
AprilNEA marked this conversation as resolved.
actionId: 'desktop.close-terminal-tab',
shortcut: CLOSE_TERMINAL_TAB_SHORTCUT,
owner,
handler(event) {
const terminal =
event.target instanceof Element
? event.target.closest<HTMLElement>('[data-terminal-panel][data-terminal-tab]')
: null;
if (terminal) {
const id = terminal.dataset.terminalTab;
if (id !== undefined) {
if (terminal.dataset.terminalPanel === 'right') closeRightTerminalTab(id);
else closeBottomTerminalTab(id);
}
} else {
closeWindow();
}
return true;
},
});

useKeyboardShortcut({
actionId: 'desktop.toggle-sidebar',
shortcut: TOGGLE_SIDEBAR_SHORTCUT,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,24 @@ describe('terminal session registry', () => {
await vi.advanceTimersByTimeAsync(1000);
});

it('delivers an exit once across a panel handoff', async () => {
const client = createFakeClient();
const firstOwner = vi.fn();
const secondOwner = vi.fn();
const first = acquireTerminalSession(client, 'tab-1', dims, firstOwner);
await vi.advanceTimersByTimeAsync(0);

first.release();
client.exitCbs.get('term-1')?.(0);
const second = acquireTerminalSession(client, 'tab-1', dims, secondOwner);
client.exitCbs.get('term-1')?.(0);

expect(firstOwner).not.toHaveBeenCalled();
expect(secondOwner).toHaveBeenCalledOnce();
second.release();
await vi.advanceTimersByTimeAsync(1000);
});

it('preserves a signal exit as distinct from a running terminal', async () => {
const client = createFakeClient();
const lease = acquireTerminalSession(client, 'tab-1', dims);
Expand Down
14 changes: 13 additions & 1 deletion packages/client/workbench/src/terminal/attached-panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { useLinkCodeClient } from '@linkcode/client-core';
import { LiveTerminal } from '@linkcode/ui/shell/terminal';
import { Button } from 'coss-ui/components/button';
import { useEffect } from 'foxact/use-abortable-effect';
import { useCallback, useMemo, useState, useSyncExternalStore } from 'react';
import { useCallback, useEffectEvent, useMemo, useState, useSyncExternalStore } from 'react';
import { useTranslations } from 'use-intl';
import { useTerminalPrefsStore } from '../settings/terminal-prefs-store';
import { createTransportTerminalSession } from './transport-session';
Expand All @@ -16,9 +16,12 @@ import { createTransportTerminalSession } from './transport-session';
export function AttachedTerminalPanel({
terminalId,
suspended,
onExit,
}: {
terminalId: string;
suspended?: boolean;
/** Called when the attached shell process exits. */
onExit?: () => void;
}): React.ReactNode {
const t = useTranslations('workbench.panel');
const client = useLinkCodeClient();
Expand Down Expand Up @@ -55,6 +58,7 @@ export function AttachedTerminalPanel({
const fontFamily = useTerminalPrefsStore((state) => state.fontFamily);
const fontSize = useTerminalPrefsStore((state) => state.fontSize);
const colorScheme = useTerminalPrefsStore((state) => state.colorScheme);
const handleExit = useEffectEvent(() => onExit?.());

useEffect(
(signal) => {
Expand All @@ -76,6 +80,14 @@ export function AttachedTerminalPanel({
[client, terminalId],
);

useEffect(
(signal) =>
client.subscribeTerminalExit(terminalId, () => {
if (!signal.aborted) handleExit();
}),
[client, terminalId],
);

const current = attachment?.terminalId === terminalId ? attachment : null;
if (!current || 'failed' in current) {
return (
Expand Down
12 changes: 10 additions & 2 deletions packages/client/workbench/src/terminal/panel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,15 @@ export function TerminalPanel({
sessionKey,
cwd,
suspended,
onExit,
}: {
sessionKey: string;
/** Working directory for the shell, captured when the terminal first opens (host home if omitted). */
cwd?: string;
/** Freeze the terminal's box while the host panel animates shut/open — see {@link LiveTerminal}. */
suspended?: boolean;
/** Called once when the shell process exits. */
onExit?: () => void;
}): React.ReactNode {
const t = useTranslations('workbench.panel');
const client = useLinkCodeClient();
Expand All @@ -37,7 +40,12 @@ export function TerminalPanel({

const subscribe = useCallback(
(onStoreChange: () => void) => {
const lease = acquireTerminalSession(client, sessionKey, { ...TERMINAL_INITIAL_SIZE, cwd });
const lease = acquireTerminalSession(
client,
sessionKey,
{ ...TERMINAL_INITIAL_SIZE, cwd },
onExit,
);
leaseRef.current = lease;
const unsubscribe = lease.subscribe(onStoreChange);
return () => {
Expand All @@ -46,7 +54,7 @@ export function TerminalPanel({
if (leaseRef.current === lease) leaseRef.current = null;
};
},
[client, sessionKey, cwd],
[client, sessionKey, cwd, onExit],
);
const snapshot = useSyncExternalStore(subscribe, () => peekTerminalSnapshot(client, sessionKey));

Expand Down
21 changes: 21 additions & 0 deletions packages/client/workbench/src/terminal/session-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ interface RegistryEntry {
unsubController: (() => void) | null;
snapshot: TerminalSnapshot;
listeners: Set<() => void>;
exitListeners: Map<symbol, () => void>;
exitDelivered: boolean;
}

/**
Expand Down Expand Up @@ -79,6 +81,14 @@ function notify(entry: RegistryEntry): void {
for (const listener of entry.listeners) listener();
}

function deliverExit(entry: RegistryEntry): void {
if (entry.exitDelivered || entry.snapshot.exit === null) return;
const onExit = Array.from(entry.exitListeners.values()).at(-1);
if (!onExit) return;
entry.exitDelivered = true;
onExit();
}

function startOpen(
client: TerminalSessionClient,
registry: Map<string, RegistryEntry>,
Expand All @@ -87,6 +97,7 @@ function startOpen(
opts: TerminalOpenOptions,
): void {
entry.attempt += 1;
entry.exitDelivered = false;
const attempt = entry.attempt;
const isCurrent = (): boolean => registry.get(key) === entry && entry.attempt === attempt;

Expand Down Expand Up @@ -136,6 +147,7 @@ function startOpen(
exit: { code: exitCode },
canControl: false,
};
deliverExit(entry);
notify(entry);
});
notify(entry);
Expand Down Expand Up @@ -177,6 +189,7 @@ export function acquireTerminalSession(
client: TerminalSessionClient,
key: string,
opts: TerminalOpenOptions,
onExit?: () => void,
): TerminalSessionLease {
const registry = getRegistry(client);
let entry = registry.get(key);
Expand All @@ -197,6 +210,8 @@ export function acquireTerminalSession(
unsubController: null,
snapshot: OPENING_SNAPSHOT,
listeners: new Set(),
exitListeners: new Map(),
exitDelivered: false,
};
registry.set(key, created);
entry = created;
Expand All @@ -205,6 +220,11 @@ export function acquireTerminalSession(

const leased = entry;
let released = false;
const exitListenerId = Symbol('terminal-exit-listener');
if (onExit) {
leased.exitListeners.set(exitListenerId, onExit);
deliverExit(leased);
}

return {
getSnapshot: () => leased.snapshot,
Expand All @@ -222,6 +242,7 @@ export function acquireTerminalSession(
release() {
if (released) return;
released = true;
leased.exitListeners.delete(exitListenerId);
leased.refCount -= 1;
if (leased.refCount > 0) return;
leased.closeTimer = setTimeout(() => {
Expand Down