FN-8729: move terminal close control after new tab
Keep the terminal close control last in the non-embedded terminal header across responsive layouts. - Render one shared close control after terminal tabs, workspace picker, and status. - Preserve the mobile corner ordering and prevent close-control shrinkage. - Add ordering coverage for desktop, mobile, workspace, and status states. - Add a patch changeset for the toolbar fix. Files changed: .changeset/fn-8729-terminal-toolbar.md | 7 +++ .../dashboard/app/components/TerminalModal.css | 15 +++--- .../dashboard/app/components/TerminalModal.tsx | 56 ++++++++-------------- .../components/__tests__/TerminalModal.test.tsx | 51 ++++++++++++++++++++ 4 files changed, 83 insertions(+), 46 deletions(-) Fusion-Task-Id: FN-8729 Fusion-Task-Lineage: ba9b039c-557d-4692-8203-bf764388d039 Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
7
.changeset/fn-8729-terminal-toolbar.md
Normal file
7
.changeset/fn-8729-terminal-toolbar.md
Normal file
@@ -0,0 +1,7 @@
|
|||||||
|
---
|
||||||
|
"@runfusion/fusion": patch
|
||||||
|
---
|
||||||
|
|
||||||
|
summary: Keep the terminal close control after New terminal at every screen size.
|
||||||
|
category: fix
|
||||||
|
dev: Unifies the non-embedded TerminalModal close render site and adds responsive ordering coverage.
|
||||||
@@ -700,6 +700,7 @@ The worktree listbox must escape the mobile terminal header's clipped overflow.
|
|||||||
|
|
||||||
.terminal-close {
|
.terminal-close {
|
||||||
display: flex;
|
display: flex;
|
||||||
|
flex: 0 0 auto;
|
||||||
align-items: center;
|
align-items: center;
|
||||||
justify-content: center;
|
justify-content: center;
|
||||||
width: 44px;
|
width: 44px;
|
||||||
@@ -1607,15 +1608,11 @@ Android folded Chrome can keep a wide layout viewport while visualViewport is th
|
|||||||
}
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
FNXC:TerminalHeader 2026-07-04-20:45:
|
FNXC:TerminalModalControls 2026-08-03-00:21:
|
||||||
FN-7565: pin the mobile close (X) button to the top-right corner of the header.
|
The shared non-embedded close control is rendered after the new-terminal tab action.
|
||||||
Without an explicit order, the close button (a direct .terminal-header child)
|
On mobile its explicit order stays above the tab dropdown and workspace picker so the
|
||||||
defaults to order:0 and sorts BEFORE .terminal-mobile-tabs (order:1) /
|
same final control is also flush right in the wrapped phone header, including the
|
||||||
.terminal-workspace-picker (order:2), landing at the far left instead of the
|
folded-Android visualViewport path where the media query below may not apply.
|
||||||
corner. Giving it the highest order plus margin-inline-start:auto renders it
|
|
||||||
last and flush against the right edge, matching where users expect an
|
|
||||||
app-sheet close control. Covers the folded-Android visualViewport path (this
|
|
||||||
selector applies even when the max-width media query below does not).
|
|
||||||
*/
|
*/
|
||||||
.modal.terminal-modal.terminal-modal--mobile .terminal-close--corner {
|
.modal.terminal-modal.terminal-modal--mobile .terminal-close--corner {
|
||||||
order: 3;
|
order: 3;
|
||||||
|
|||||||
@@ -2480,6 +2480,7 @@ export function TerminalModal({ isOpen, onClose, initialCommand, initialCommandG
|
|||||||
onClick={measuring ? undefined : () => void createTab()}
|
onClick={measuring ? undefined : () => void createTab()}
|
||||||
title={t("terminal.newTerminal", "New terminal")}
|
title={t("terminal.newTerminal", "New terminal")}
|
||||||
aria-label={t("terminal.newTerminal", "New terminal")}
|
aria-label={t("terminal.newTerminal", "New terminal")}
|
||||||
|
data-testid="terminal-new-tab"
|
||||||
>
|
>
|
||||||
+
|
+
|
||||||
</button>
|
</button>
|
||||||
@@ -2699,52 +2700,33 @@ export function TerminalModal({ isOpen, onClose, initialCommand, initialCommandG
|
|||||||
</div>
|
</div>
|
||||||
)}
|
)}
|
||||||
|
|
||||||
{/*
|
{/* Status indicator */}
|
||||||
FNXC:TerminalFooter 2026-07-11-20:20:
|
{(!isMobileTerminal || embedded) && (
|
||||||
FN-7829 removes `.terminal-actions` from the header at every width. Mobile keeps the corner-pinned close button, while tablet/desktop/embedded headers keep the title/status, tab affordance, workspace picker, and a plain close button; the shared `terminalActionControls` fragment renders only in the bottom `.terminal-status-bar` footer so the control handlers cannot drift or leave an empty header shell.
|
<div className="terminal-title" data-testid="terminal-title">
|
||||||
|
<TerminalIcon size={16} />
|
||||||
|
{getStatusIndicator()}
|
||||||
|
</div>
|
||||||
|
)}
|
||||||
|
|
||||||
FNXC:TerminalHeader 2026-07-04-20:45:
|
{/*
|
||||||
FN-7565: on mobile, being a direct child of `.terminal-header` (not
|
FNXC:TerminalModalControls 2026-08-03-00:21:
|
||||||
nested in `.terminal-actions`) is necessary but not sufficient to land
|
Every non-embedded terminal has exactly one modal-close control, rendered after the
|
||||||
in the top-right corner — flex items without an explicit `order` fall
|
tab region (including its new-terminal affordance), optional workspace picker, and
|
||||||
back to `order: 0`, which sorts BEFORE `.terminal-mobile-tabs`
|
status title. Keeping one shared final render site makes the close-after-plus,
|
||||||
(`order: 1`) and `.terminal-workspace-picker` (`order: 2`), pushing the
|
far-right contract structural for desktop, tablet, ResizeObserver overflow, and mobile.
|
||||||
close button to the far LEFT of the header instead of the corner users
|
Mobile keeps the corner class so its explicit flex order remains last; embedded terminals
|
||||||
expect for an app-sheet close control. The `terminal-close--corner`
|
intentionally render no modal-close control because their parent owns dismissal.
|
||||||
class (CSS: highest `order` + `margin-inline-start: auto`) fixes this
|
|
||||||
so the X renders last in flex order and hugs the right edge regardless
|
|
||||||
of how wide the tab dropdown / workspace picker grow. Tablet keeps the
|
|
||||||
desktop `.terminal-tabs` (not the mobile dropdown) ahead of title/close
|
|
||||||
in normal DOM order, so its plain close button needs no order override.
|
|
||||||
*/}
|
*/}
|
||||||
{isMobileTerminal && !embedded ? (
|
{!embedded && (
|
||||||
<button
|
<button
|
||||||
className="terminal-close terminal-close--corner"
|
className={`terminal-close${isMobileTerminal ? " terminal-close--corner" : ""}`}
|
||||||
onClick={onClose}
|
onClick={onClose}
|
||||||
data-testid="terminal-close-btn"
|
data-testid="terminal-close-btn"
|
||||||
title={t("terminal.closeTerminal", "Close terminal")}
|
title={t("terminal.closeTerminal", "Close terminal")}
|
||||||
|
aria-label={t("terminal.closeTerminal", "Close terminal")}
|
||||||
>
|
>
|
||||||
<X size={20} />
|
<X size={20} />
|
||||||
</button>
|
</button>
|
||||||
) : (
|
|
||||||
<>
|
|
||||||
{/* Status indicator */}
|
|
||||||
<div className="terminal-title" data-testid="terminal-title">
|
|
||||||
<TerminalIcon size={16} />
|
|
||||||
{getStatusIndicator()}
|
|
||||||
</div>
|
|
||||||
|
|
||||||
{!embedded ? (
|
|
||||||
<button
|
|
||||||
className="terminal-close"
|
|
||||||
onClick={onClose}
|
|
||||||
data-testid="terminal-close-btn"
|
|
||||||
title={t("terminal.closeTerminal", "Close terminal")}
|
|
||||||
>
|
|
||||||
<X size={20} />
|
|
||||||
</button>
|
|
||||||
) : null}
|
|
||||||
</>
|
|
||||||
)}
|
)}
|
||||||
</div>
|
</div>
|
||||||
|
|
||||||
|
|||||||
@@ -47,6 +47,22 @@ function defineMetric(element: Element, property: "clientWidth" | "scrollWidth",
|
|||||||
Object.defineProperty(element, property, { configurable: true, value });
|
Object.defineProperty(element, property, { configurable: true, value });
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function expectTerminalCloseAfterNewTerminal(newTerminalTestId: string): void {
|
||||||
|
const header = document.querySelector<HTMLElement>(".terminal-header");
|
||||||
|
expect(header).not.toBeNull();
|
||||||
|
|
||||||
|
const closeButtons = screen.getAllByTestId("terminal-close-btn");
|
||||||
|
expect(closeButtons).toHaveLength(1);
|
||||||
|
const closeButton = closeButtons[0];
|
||||||
|
const newTerminalButton = screen.getByTestId(newTerminalTestId);
|
||||||
|
|
||||||
|
expect(header).toContainElement(closeButton);
|
||||||
|
expect(newTerminalButton.compareDocumentPosition(closeButton) & Node.DOCUMENT_POSITION_FOLLOWING)
|
||||||
|
.toBe(Node.DOCUMENT_POSITION_FOLLOWING);
|
||||||
|
expect(Array.from(header!.querySelectorAll("button")).at(-1)).toBe(closeButton);
|
||||||
|
expect(header!.querySelector(".terminal-actions")).toBeNull();
|
||||||
|
}
|
||||||
|
|
||||||
// Mock hooks and API
|
// Mock hooks and API
|
||||||
vi.mock("../../hooks/useTerminal", () => ({
|
vi.mock("../../hooks/useTerminal", () => ({
|
||||||
useTerminal: vi.fn(),
|
useTerminal: vi.fn(),
|
||||||
@@ -2026,6 +2042,7 @@ describe("TerminalModal", () => {
|
|||||||
expect(mockSetActiveTab).toHaveBeenCalledWith("tab-2");
|
expect(mockSetActiveTab).toHaveBeenCalledWith("tab-2");
|
||||||
fireEvent.click(screen.getByTestId("terminal-mobile-new-tab"));
|
fireEvent.click(screen.getByTestId("terminal-mobile-new-tab"));
|
||||||
expect(mockCreateTab).toHaveBeenCalledWith();
|
expect(mockCreateTab).toHaveBeenCalledWith();
|
||||||
|
expectTerminalCloseAfterNewTerminal("terminal-mobile-new-tab");
|
||||||
fireEvent.click(screen.getByTestId("terminal-mobile-close-tab"));
|
fireEvent.click(screen.getByTestId("terminal-mobile-close-tab"));
|
||||||
expect(mockCloseTab).toHaveBeenCalledWith("tab-1");
|
expect(mockCloseTab).toHaveBeenCalledWith("tab-1");
|
||||||
|
|
||||||
@@ -4674,12 +4691,42 @@ describe("TerminalModal — mobile layout contract", () => {
|
|||||||
expect(footer.contains(closeBtn)).toBe(false);
|
expect(footer.contains(closeBtn)).toBe(false);
|
||||||
const mobileTabs = screen.getByTestId("terminal-mobile-tabs");
|
const mobileTabs = screen.getByTestId("terminal-mobile-tabs");
|
||||||
expect(header?.contains(mobileTabs)).toBe(true);
|
expect(header?.contains(mobileTabs)).toBe(true);
|
||||||
|
expectTerminalCloseAfterNewTerminal("terminal-mobile-new-tab");
|
||||||
});
|
});
|
||||||
} finally {
|
} finally {
|
||||||
Object.defineProperty(window, "innerWidth", { value: previousInnerWidth, configurable: true });
|
Object.defineProperty(window, "innerWidth", { value: previousInnerWidth, configurable: true });
|
||||||
}
|
}
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("keeps the populated-workspace mobile close control after the new-terminal action", async () => {
|
||||||
|
const previousInnerWidth = window.innerWidth;
|
||||||
|
Object.defineProperty(window, "innerWidth", { value: 390, configurable: true });
|
||||||
|
mockUseWorkspaces.mockReturnValue({
|
||||||
|
projectName: "kb",
|
||||||
|
workspaces: [
|
||||||
|
{ id: "FN-8729", label: "FN-8729", title: "Terminal toolbar ordering", worktree: "/repo/.worktrees/FN-8729", kind: "task" },
|
||||||
|
],
|
||||||
|
loading: false,
|
||||||
|
error: null,
|
||||||
|
});
|
||||||
|
|
||||||
|
try {
|
||||||
|
render(<TerminalModal isOpen={true} onClose={mockOnClose} />);
|
||||||
|
|
||||||
|
await waitFor(() => {
|
||||||
|
expect(screen.getByTestId("terminal-workspace-picker")).toBeInTheDocument();
|
||||||
|
expectTerminalCloseAfterNewTerminal("terminal-mobile-new-tab");
|
||||||
|
});
|
||||||
|
|
||||||
|
fireEvent.click(screen.getByTestId("terminal-mobile-new-tab"));
|
||||||
|
fireEvent.click(screen.getByTestId("terminal-close-btn"));
|
||||||
|
expect(manyTabsSessionState.createTab).toHaveBeenCalledWith();
|
||||||
|
expect(mockOnClose).toHaveBeenCalledTimes(1);
|
||||||
|
} finally {
|
||||||
|
Object.defineProperty(window, "innerWidth", { value: previousInnerWidth, configurable: true });
|
||||||
|
}
|
||||||
|
});
|
||||||
|
|
||||||
it("renders terminal action controls in a tablet footer, not the header, and keeps pin/pop-out toggles (FN-7684)", async () => {
|
it("renders terminal action controls in a tablet footer, not the header, and keeps pin/pop-out toggles (FN-7684)", async () => {
|
||||||
// FN-7684: tablet width (769-1024px) must relocate the same shared
|
// FN-7684: tablet width (769-1024px) must relocate the same shared
|
||||||
// terminalActionControls fragment into the footer, exactly as FN-7560
|
// terminalActionControls fragment into the footer, exactly as FN-7560
|
||||||
@@ -4740,6 +4787,7 @@ describe("TerminalModal — mobile layout contract", () => {
|
|||||||
if (workspacePicker) {
|
if (workspacePicker) {
|
||||||
expect(header?.contains(workspacePicker)).toBe(true);
|
expect(header?.contains(workspacePicker)).toBe(true);
|
||||||
}
|
}
|
||||||
|
expectTerminalCloseAfterNewTerminal("terminal-new-tab");
|
||||||
});
|
});
|
||||||
} finally {
|
} finally {
|
||||||
Object.defineProperty(window, "innerWidth", { value: previousInnerWidth, configurable: true });
|
Object.defineProperty(window, "innerWidth", { value: previousInnerWidth, configurable: true });
|
||||||
@@ -4793,6 +4841,7 @@ describe("TerminalModal — mobile layout contract", () => {
|
|||||||
const closeBtn = screen.getByTestId("terminal-close-btn");
|
const closeBtn = screen.getByTestId("terminal-close-btn");
|
||||||
expect(closeBtn.parentElement).toBe(header);
|
expect(closeBtn.parentElement).toBe(header);
|
||||||
expect(closeBtn.className).toContain("terminal-close--corner");
|
expect(closeBtn.className).toContain("terminal-close--corner");
|
||||||
|
expectTerminalCloseAfterNewTerminal("terminal-mobile-new-tab");
|
||||||
// Reconnect control lives in the footer, not the header, so it cannot
|
// Reconnect control lives in the footer, not the header, so it cannot
|
||||||
// crowd the corner-pinned close button.
|
// crowd the corner-pinned close button.
|
||||||
expect(screen.getByTestId("terminal-reconnect-btn").closest(".terminal-header")).toBeNull();
|
expect(screen.getByTestId("terminal-reconnect-btn").closest(".terminal-header")).toBeNull();
|
||||||
@@ -4820,6 +4869,7 @@ describe("TerminalModal — mobile layout contract", () => {
|
|||||||
const closeBtn = screen.getByTestId("terminal-close-btn");
|
const closeBtn = screen.getByTestId("terminal-close-btn");
|
||||||
expect(closeBtn.parentElement).toBe(header);
|
expect(closeBtn.parentElement).toBe(header);
|
||||||
expect(closeBtn.className).toContain("terminal-close--corner");
|
expect(closeBtn.className).toContain("terminal-close--corner");
|
||||||
|
expectTerminalCloseAfterNewTerminal("terminal-mobile-new-tab");
|
||||||
// Restart control + exit code live in the footer, not the header.
|
// Restart control + exit code live in the footer, not the header.
|
||||||
expect(screen.getByTestId("terminal-restart-btn").closest(".terminal-header")).toBeNull();
|
expect(screen.getByTestId("terminal-restart-btn").closest(".terminal-header")).toBeNull();
|
||||||
});
|
});
|
||||||
@@ -4844,6 +4894,7 @@ describe("TerminalModal — mobile layout contract", () => {
|
|||||||
expect(header?.contains(closeBtn)).toBe(true);
|
expect(header?.contains(closeBtn)).toBe(true);
|
||||||
expect(screen.queryByTestId("terminal-actions")).toBeNull();
|
expect(screen.queryByTestId("terminal-actions")).toBeNull();
|
||||||
expect(closeBtn.className).not.toContain("terminal-close--corner");
|
expect(closeBtn.className).not.toContain("terminal-close--corner");
|
||||||
|
expectTerminalCloseAfterNewTerminal("terminal-new-tab");
|
||||||
});
|
});
|
||||||
} finally {
|
} finally {
|
||||||
Object.defineProperty(window, "innerWidth", { value: previousInnerWidth, configurable: true });
|
Object.defineProperty(window, "innerWidth", { value: previousInnerWidth, configurable: true });
|
||||||
|
|||||||
Reference in New Issue
Block a user