feat(FN-2629): stabilize terminal modal font-size refit
- Enforce terminal xterm root height fill so inline fit heights do not collapse the modal terminal area - Defer font-size-driven fit to the next animation frame and coalesce pending fits via pendingFitRef - Remove eager refit calls from zoom keyboard/button handlers and rely on shared font-size effect scheduling - Expand TerminalModal tests to assert font-size controls and keyboard zoom continue to trigger xterm refits
This commit is contained in:
@@ -513,11 +513,18 @@
|
||||
.terminal-xterm {
|
||||
width: 100%;
|
||||
height: 100%;
|
||||
min-height: 0;
|
||||
padding: var(--space-xs);
|
||||
}
|
||||
|
||||
/*
|
||||
* xterm fit may apply inline pixel heights on the `.xterm` root after
|
||||
* row/line-height recomputation (e.g. after font-size changes). Keep the root
|
||||
* stretched to the wrapper height so the terminal always fills the modal.
|
||||
*/
|
||||
.terminal-xterm .xterm {
|
||||
height: 100%;
|
||||
height: 100% !important;
|
||||
min-height: 100%;
|
||||
}
|
||||
|
||||
/*
|
||||
|
||||
@@ -758,7 +758,27 @@ export function TerminalModal({ isOpen, onClose, initialCommand, projectId }: Te
|
||||
}
|
||||
|
||||
xtermRef.current.options.fontSize = fontSize;
|
||||
refitTerminal();
|
||||
|
||||
// Defer fit until the next frame so layout reflects the new font metrics
|
||||
// before FitAddon measures rows/cols. Reuse pendingFitRef so font-size and
|
||||
// visualViewport-triggered fits are coalesced into a single scheduled fit.
|
||||
if (pendingFitRef.current !== null) {
|
||||
cancelAnimationFrame(pendingFitRef.current);
|
||||
pendingFitRef.current = null;
|
||||
}
|
||||
|
||||
const frame = requestAnimationFrame(() => {
|
||||
pendingFitRef.current = null;
|
||||
refitTerminal();
|
||||
});
|
||||
pendingFitRef.current = frame;
|
||||
|
||||
return () => {
|
||||
if (pendingFitRef.current === frame) {
|
||||
cancelAnimationFrame(frame);
|
||||
pendingFitRef.current = null;
|
||||
}
|
||||
};
|
||||
}, [fontSize, xtermReady, refitTerminal]);
|
||||
|
||||
// Handle keyboard shortcuts (zoom)
|
||||
@@ -772,7 +792,6 @@ export function TerminalModal({ isOpen, onClose, initialCommand, projectId }: Te
|
||||
if (e.code === "Equal" || e.code === "NumpadAdd") {
|
||||
e.preventDefault();
|
||||
setFontSize((current) => clampTerminalFontSize(current + 1));
|
||||
refitTerminal();
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -780,7 +799,6 @@ export function TerminalModal({ isOpen, onClose, initialCommand, projectId }: Te
|
||||
if (e.code === "Minus" || e.code === "NumpadSubtract") {
|
||||
e.preventDefault();
|
||||
setFontSize((current) => clampTerminalFontSize(current - 1));
|
||||
refitTerminal();
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -788,7 +806,6 @@ export function TerminalModal({ isOpen, onClose, initialCommand, projectId }: Te
|
||||
if (e.code === "Digit0" || e.code === "Numpad0") {
|
||||
e.preventDefault();
|
||||
setFontSize(DEFAULT_FONT_SIZE);
|
||||
refitTerminal();
|
||||
return;
|
||||
}
|
||||
};
|
||||
@@ -976,13 +993,11 @@ export function TerminalModal({ isOpen, onClose, initialCommand, projectId }: Te
|
||||
|
||||
const handleIncreaseFontSize = useCallback(() => {
|
||||
setFontSize((current) => clampTerminalFontSize(current + 1));
|
||||
refitTerminal();
|
||||
}, [refitTerminal]);
|
||||
}, []);
|
||||
|
||||
const handleDecreaseFontSize = useCallback(() => {
|
||||
setFontSize((current) => clampTerminalFontSize(current - 1));
|
||||
refitTerminal();
|
||||
}, [refitTerminal]);
|
||||
}, []);
|
||||
|
||||
const toggleModifier = useCallback((modifier: "ctrl" | "alt") => {
|
||||
setStickyModifier((current) => (current === modifier ? null : modifier));
|
||||
|
||||
@@ -21,6 +21,8 @@ vi.mock("../../api", () => ({
|
||||
}));
|
||||
|
||||
// Mock xterm modules to prevent DOM errors in jsdom
|
||||
const mockFitAddonFit = vi.fn();
|
||||
|
||||
const mockTerminalInstance = {
|
||||
loadAddon: vi.fn(),
|
||||
open: vi.fn(),
|
||||
@@ -40,7 +42,7 @@ vi.mock("@xterm/xterm", () => ({
|
||||
|
||||
vi.mock("@xterm/addon-fit", () => ({
|
||||
FitAddon: vi.fn(() => ({
|
||||
fit: vi.fn(),
|
||||
fit: mockFitAddonFit,
|
||||
dispose: vi.fn(),
|
||||
})),
|
||||
}));
|
||||
@@ -120,6 +122,7 @@ describe("TerminalModal", () => {
|
||||
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks();
|
||||
mockFitAddonFit.mockClear();
|
||||
window.localStorage.removeItem(TERMINAL_FONT_SIZE_KEY);
|
||||
mockTerminalInstance.options.fontSize = 14;
|
||||
mockCreateTerminalSession.mockResolvedValue({
|
||||
@@ -640,25 +643,37 @@ describe("TerminalModal", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("increases font size via button and persists to localStorage", async () => {
|
||||
it("increases font size via button, persists, and refits xterm", async () => {
|
||||
render(<TerminalModal isOpen={true} onClose={mockOnClose} />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId("terminal-font-size-value").textContent).toBe("14px");
|
||||
});
|
||||
const fitCallBaseline = mockFitAddonFit.mock.calls.length;
|
||||
|
||||
fireEvent.click(screen.getByTestId("terminal-font-size-increase"));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId("terminal-font-size-value").textContent).toBe("15px");
|
||||
expect(window.localStorage.getItem(TERMINAL_FONT_SIZE_KEY)).toBe("15");
|
||||
expect(mockFitAddonFit.mock.calls.length).toBeGreaterThan(fitCallBaseline);
|
||||
});
|
||||
});
|
||||
|
||||
it("decreases font size via button and persists to localStorage", async () => {
|
||||
it("decreases font size via button, persists, and refits xterm", async () => {
|
||||
render(<TerminalModal isOpen={true} onClose={mockOnClose} />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId("terminal-font-size-value").textContent).toBe("14px");
|
||||
});
|
||||
const fitCallBaseline = mockFitAddonFit.mock.calls.length;
|
||||
|
||||
fireEvent.click(screen.getByTestId("terminal-font-size-decrease"));
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId("terminal-font-size-value").textContent).toBe("13px");
|
||||
expect(window.localStorage.getItem(TERMINAL_FONT_SIZE_KEY)).toBe("13");
|
||||
expect(mockFitAddonFit.mock.calls.length).toBeGreaterThan(fitCallBaseline);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -698,23 +713,39 @@ describe("TerminalModal", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("keeps keyboard zoom shortcuts wired to shared font-size state", async () => {
|
||||
it("keeps keyboard zoom shortcuts wired to shared font-size state and refits xterm", async () => {
|
||||
render(<TerminalModal isOpen={true} onClose={mockOnClose} />);
|
||||
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId("terminal-font-size-value").textContent).toBe("14px");
|
||||
});
|
||||
|
||||
const baseline = mockFitAddonFit.mock.calls.length;
|
||||
|
||||
fireEvent.keyDown(window, { ctrlKey: true, code: "Equal" });
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId("terminal-font-size-value").textContent).toBe("15px");
|
||||
expect(mockFitAddonFit.mock.calls.length).toBeGreaterThan(baseline);
|
||||
});
|
||||
|
||||
const afterEqual = mockFitAddonFit.mock.calls.length;
|
||||
fireEvent.keyDown(window, { ctrlKey: true, code: "Minus" });
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId("terminal-font-size-value").textContent).toBe("14px");
|
||||
expect(mockFitAddonFit.mock.calls.length).toBeGreaterThan(afterEqual);
|
||||
});
|
||||
|
||||
fireEvent.keyDown(window, { ctrlKey: true, code: "Equal" });
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId("terminal-font-size-value").textContent).toBe("15px");
|
||||
});
|
||||
|
||||
fireEvent.keyDown(window, { ctrlKey: true, code: "Minus" });
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId("terminal-font-size-value").textContent).toBe("14px");
|
||||
});
|
||||
|
||||
const afterSecondEqual = mockFitAddonFit.mock.calls.length;
|
||||
fireEvent.keyDown(window, { ctrlKey: true, code: "Digit0" });
|
||||
await waitFor(() => {
|
||||
expect(screen.getByTestId("terminal-font-size-value").textContent).toBe("14px");
|
||||
expect(window.localStorage.getItem(TERMINAL_FONT_SIZE_KEY)).toBe("14");
|
||||
expect(mockFitAddonFit.mock.calls.length).toBeGreaterThan(afterSecondEqual);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user