Widen the macOS title-bar drag strips so the window is easy to move (#33991)
* Widen the macOS title-bar drag strips so the window is easy to move With titleBarStyle: "hidden" there is no native title bar, so the macOS window can only be moved via the -webkit-app-region: drag strips above the room and left-panel headers. Those strips were only ~13-24px tall and too thin to reliably grab, making the window awkward to move. Raise the drag strips above the room, left-panel and space headers to 32px (matching the traffic-light offset already used elsewhere in the file) for a comfortable grab zone; interactive controls keep -webkit-app-region: no-drag so they stay clickable. The injected CSS is also extracted into a pure buildTitleBarCss() helper so the string contract can be unit-tested. * Address review: use author copyright, cover setupMacosTitleBar, drop dead selector Use my own copyright on the new test file rather than New Vector's. Add tests for setupMacosTitleBar() itself: the platform guard and the enter-full-screen / leave-full-screen / did-finish-load lifecycle were previously never executed, leaving the diff below the 80% coverage gate. Drop the .mx_LeftPanel_newRoomList::before rule: #34040 removed that class from the product, so the rule matched nothing and the border-right it carried was silently lost from the left panel. Fold it back into .mx_LeftPanel::before, with a regression test covering the separator. --------- Co-authored-by: Michael Telatynski <7t3chguy@gmail.com>
This commit is contained in:
co-authored by
Michael Telatynski
parent
13aa6fcc46
commit
dd7c9ed6fe
@@ -1,70 +1,222 @@
|
|||||||
/*
|
/*
|
||||||
Copyright 2026 Spencer Poisseroux
|
Copyright 2026 Spencer Poisseroux
|
||||||
|
Copyright 2026 hayaksi1
|
||||||
|
|
||||||
SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
|
SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
|
||||||
Please see LICENSE files in the repository root for full details.
|
Please see LICENSE files in the repository root for full details.
|
||||||
*/
|
*/
|
||||||
|
|
||||||
import { expect, describe, it, beforeEach, afterEach, vi } from "vitest";
|
import { afterEach, describe, expect, it, vi, type Mock } from "vitest";
|
||||||
import type { BrowserWindow } from "electron";
|
import type { BrowserWindow } from "electron";
|
||||||
|
|
||||||
import { setupMacosTitleBar } from "./macos-titlebar.js";
|
import { buildTitleBarCss, setupMacosTitleBar } from "./macos-titlebar.js";
|
||||||
|
|
||||||
function createFakeWindow(): {
|
/**
|
||||||
window: BrowserWindow;
|
* Extract the `height` (in px) declared for the given selector.
|
||||||
emitDidFinishLoad: () => void;
|
*
|
||||||
insertCSS: ReturnType<typeof vi.fn>;
|
* A selector may appear in more than one rule block (e.g. `.mx_SpaceRoomView::before` is both grouped with
|
||||||
} {
|
* `.mx_RoomView::before` for the drag declaration and given its own block for the height). We scan every
|
||||||
const listeners: Record<string, () => void> = {};
|
* block whose selector list contains the target and return the height from the first block that declares one.
|
||||||
const insertCSS = vi.fn().mockResolvedValue("css-key");
|
*/
|
||||||
const window = {
|
function dragStripHeightPx(css: string, selector: string): number {
|
||||||
isFullScreen: vi.fn().mockReturnValue(false),
|
const escaped = selector.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
|
||||||
on: vi.fn(),
|
const blockRegex = new RegExp(`([^{}]*${escaped}[^{}]*)\\{([^}]*)\\}`, "g");
|
||||||
webContents: {
|
let match: RegExpExecArray | null;
|
||||||
insertCSS,
|
let foundBlock = false;
|
||||||
removeInsertedCSS: vi.fn(),
|
while ((match = blockRegex.exec(css)) !== null) {
|
||||||
on: vi.fn((event: string, listener: () => void) => {
|
foundBlock = true;
|
||||||
listeners[event] = listener;
|
const heightMatch = /height:\s*(\d+(?:\.\d+)?)px/.exec(match[2]);
|
||||||
}),
|
if (heightMatch) {
|
||||||
},
|
return Number.parseFloat(heightMatch[1]);
|
||||||
} as unknown as BrowserWindow;
|
}
|
||||||
|
}
|
||||||
return {
|
expect(foundBlock, `expected a rule block for "${selector}"`).toBe(true);
|
||||||
window,
|
throw new Error(`expected a px height declared for "${selector}"`);
|
||||||
insertCSS,
|
|
||||||
emitDidFinishLoad: () => listeners["did-finish-load"]?.(),
|
|
||||||
};
|
|
||||||
}
|
}
|
||||||
|
|
||||||
|
describe("buildTitleBarCss", () => {
|
||||||
|
const css = buildTitleBarCss();
|
||||||
|
|
||||||
|
it("returns a non-empty CSS string", () => {
|
||||||
|
expect(typeof css).toBe("string");
|
||||||
|
expect(css.length).toBeGreaterThan(0);
|
||||||
|
});
|
||||||
|
|
||||||
|
it.each([".mx_RoomView::before", ".mx_LeftPanel::before", ".mx_SpaceRoomView::before"])(
|
||||||
|
"marks %s as a drag handle",
|
||||||
|
(selector) => {
|
||||||
|
const escaped = selector.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
|
||||||
|
const blockMatch = new RegExp(`${escaped}[^}]*\\{([^}]*)\\}`).exec(css);
|
||||||
|
expect(blockMatch, `expected a rule block for "${selector}"`).not.toBeNull();
|
||||||
|
expect(blockMatch![1]).toMatch(/-webkit-app-region:\s*drag/);
|
||||||
|
},
|
||||||
|
);
|
||||||
|
|
||||||
|
// Regression guard for #32018: the drag strips above the headers were ~13px and too small to grab.
|
||||||
|
it("gives .mx_RoomView::before a drag strip at least 28px tall (regression #32018)", () => {
|
||||||
|
expect(dragStripHeightPx(css, ".mx_RoomView::before")).toBeGreaterThanOrEqual(28);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("gives .mx_LeftPanel::before a drag strip at least 28px tall (regression #32018)", () => {
|
||||||
|
expect(dragStripHeightPx(css, ".mx_LeftPanel::before")).toBeGreaterThanOrEqual(28);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("gives .mx_SpaceRoomView::before a drag strip at least 28px tall (regression #32018)", () => {
|
||||||
|
expect(dragStripHeightPx(css, ".mx_SpaceRoomView::before")).toBeGreaterThanOrEqual(28);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("keeps the left panel's separator on its drag strip", () => {
|
||||||
|
// The strip carries the panel's right-hand border up through the title bar band; widening it must not drop it.
|
||||||
|
expect(css).toMatch(
|
||||||
|
/\.mx_LeftPanel::before\s*\{[^}]*border-right:\s*1px\s+solid\s+var\(--cpd-color-bg-subtle-primary\)/,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
// Regression guard for #34243: against the default 68px rail the collapsed space panel's right-hand
|
||||||
|
// separator crowds the green traffic light, so the panel is widened to clear it.
|
||||||
|
it("widens the collapsed space panel so the separator clears the traffic lights", () => {
|
||||||
|
expect(css).toMatch(/\.mx_SpacePanel\.collapsed\s*\{[^}]*width:\s*76px\s*!important/);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("keeps interactive elements excluded from the drag region (no-drag)", () => {
|
||||||
|
// The UserMenu buttons must remain clickable, not act as a drag handle.
|
||||||
|
expect(css).toMatch(/\.mx_UserMenu\s*>\s*\*\s*\{[^}]*-webkit-app-region:\s*no-drag/);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("keeps iframes excluded from the drag region (no-drag)", () => {
|
||||||
|
// iframes (e.g. recaptcha, widgets) must remain interactive.
|
||||||
|
expect(css).toMatch(/iframe\s*\{[^}]*-webkit-app-region:\s*no-drag/);
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does not turn the traffic-light offset into a no-drag handle on .mx_UserMenu itself", () => {
|
||||||
|
// The UserMenu container itself stays a drag handle (only its children are no-drag).
|
||||||
|
expect(css).toMatch(/\.mx_UserMenu\s*\{[^}]*-webkit-app-region:\s*drag/);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
describe("setupMacosTitleBar", () => {
|
describe("setupMacosTitleBar", () => {
|
||||||
|
/** Minimal `BrowserWindow` stand-in: the module only ever touches these members. */
|
||||||
|
function mockWindow(): {
|
||||||
|
window: BrowserWindow;
|
||||||
|
windowHandlers: Map<string, () => void>;
|
||||||
|
webContentsHandlers: Map<string, () => void>;
|
||||||
|
insertCSS: Mock;
|
||||||
|
removeInsertedCSS: Mock;
|
||||||
|
isFullScreen: Mock;
|
||||||
|
} {
|
||||||
|
const windowHandlers = new Map<string, () => void>();
|
||||||
|
const webContentsHandlers = new Map<string, () => void>();
|
||||||
|
const insertCSS = vi.fn<(css: string) => Promise<string>>().mockResolvedValue("css-key-1");
|
||||||
|
const removeInsertedCSS = vi.fn<(key: string) => Promise<void>>().mockResolvedValue(undefined);
|
||||||
|
const isFullScreen = vi.fn<() => boolean>().mockReturnValue(false);
|
||||||
|
|
||||||
|
const window = {
|
||||||
|
on: vi.fn((event: string, handler: () => void) => {
|
||||||
|
windowHandlers.set(event, handler);
|
||||||
|
}),
|
||||||
|
isFullScreen,
|
||||||
|
webContents: {
|
||||||
|
on: vi.fn((event: string, handler: () => void) => {
|
||||||
|
webContentsHandlers.set(event, handler);
|
||||||
|
}),
|
||||||
|
insertCSS,
|
||||||
|
removeInsertedCSS,
|
||||||
|
},
|
||||||
|
} as unknown as BrowserWindow;
|
||||||
|
|
||||||
|
return { window, windowHandlers, webContentsHandlers, insertCSS, removeInsertedCSS, isFullScreen };
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The listeners are `() => void` and start `applyStyling()` without awaiting it, so awaiting a handler's
|
||||||
|
* own return value would prove nothing. Yield to the macrotask queue instead, which drains the pending
|
||||||
|
* microtasks and lets that fire-and-forget promise settle before we assert on its effects.
|
||||||
|
*/
|
||||||
|
function flushStyling(): Promise<void> {
|
||||||
|
return new Promise((resolve) => setTimeout(resolve, 0));
|
||||||
|
}
|
||||||
|
|
||||||
afterEach(() => {
|
afterEach(() => {
|
||||||
vi.restoreAllMocks();
|
vi.restoreAllMocks();
|
||||||
});
|
});
|
||||||
|
|
||||||
describe("on macOS", () => {
|
it("does nothing on non-darwin platforms", () => {
|
||||||
beforeEach(() => {
|
|
||||||
vi.spyOn(process, "platform", "get").mockReturnValue("darwin");
|
|
||||||
});
|
|
||||||
|
|
||||||
it("widens the collapsed space panel so the separator clears the traffic lights", async () => {
|
|
||||||
const { window, insertCSS, emitDidFinishLoad } = createFakeWindow();
|
|
||||||
setupMacosTitleBar(window);
|
|
||||||
emitDidFinishLoad();
|
|
||||||
// insertCSS is async; wait for the microtask queue to flush
|
|
||||||
await Promise.resolve();
|
|
||||||
|
|
||||||
expect(insertCSS).toHaveBeenCalledTimes(1);
|
|
||||||
const css = insertCSS.mock.calls[0][0] as string;
|
|
||||||
expect(css).toContain(".mx_SpacePanel.collapsed");
|
|
||||||
expect(css).toContain("width: 76px !important;");
|
|
||||||
});
|
|
||||||
});
|
|
||||||
|
|
||||||
it("does nothing on non-macOS platforms", () => {
|
|
||||||
vi.spyOn(process, "platform", "get").mockReturnValue("win32");
|
vi.spyOn(process, "platform", "get").mockReturnValue("win32");
|
||||||
const { window, insertCSS, emitDidFinishLoad } = createFakeWindow();
|
const { window, insertCSS } = mockWindow();
|
||||||
|
|
||||||
setupMacosTitleBar(window);
|
setupMacosTitleBar(window);
|
||||||
emitDidFinishLoad();
|
|
||||||
|
expect(window.on).not.toHaveBeenCalled();
|
||||||
|
expect(window.webContents.on).not.toHaveBeenCalled();
|
||||||
expect(insertCSS).not.toHaveBeenCalled();
|
expect(insertCSS).not.toHaveBeenCalled();
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("registers the full-screen and load listeners on darwin", () => {
|
||||||
|
vi.spyOn(process, "platform", "get").mockReturnValue("darwin");
|
||||||
|
const { window } = mockWindow();
|
||||||
|
|
||||||
|
setupMacosTitleBar(window);
|
||||||
|
|
||||||
|
expect(window.on).toHaveBeenCalledWith("enter-full-screen", expect.any(Function));
|
||||||
|
expect(window.on).toHaveBeenCalledWith("leave-full-screen", expect.any(Function));
|
||||||
|
expect(window.webContents.on).toHaveBeenCalledWith("did-finish-load", expect.any(Function));
|
||||||
|
});
|
||||||
|
|
||||||
|
it("injects the title bar CSS once the page has loaded", async () => {
|
||||||
|
vi.spyOn(process, "platform", "get").mockReturnValue("darwin");
|
||||||
|
const { window, webContentsHandlers, insertCSS } = mockWindow();
|
||||||
|
|
||||||
|
setupMacosTitleBar(window);
|
||||||
|
webContentsHandlers.get("did-finish-load")!();
|
||||||
|
await flushStyling();
|
||||||
|
|
||||||
|
expect(insertCSS).toHaveBeenCalledOnce();
|
||||||
|
expect(insertCSS).toHaveBeenCalledWith(buildTitleBarCss());
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does not inject the CSS if the window loads while already full screen", async () => {
|
||||||
|
vi.spyOn(process, "platform", "get").mockReturnValue("darwin");
|
||||||
|
const { window, webContentsHandlers, insertCSS, isFullScreen } = mockWindow();
|
||||||
|
isFullScreen.mockReturnValue(true);
|
||||||
|
|
||||||
|
setupMacosTitleBar(window);
|
||||||
|
webContentsHandlers.get("did-finish-load")!();
|
||||||
|
await flushStyling();
|
||||||
|
|
||||||
|
expect(insertCSS).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("removes the injected CSS when entering full screen", async () => {
|
||||||
|
vi.spyOn(process, "platform", "get").mockReturnValue("darwin");
|
||||||
|
const { window, windowHandlers, webContentsHandlers, removeInsertedCSS } = mockWindow();
|
||||||
|
|
||||||
|
setupMacosTitleBar(window);
|
||||||
|
webContentsHandlers.get("did-finish-load")!();
|
||||||
|
await flushStyling();
|
||||||
|
windowHandlers.get("enter-full-screen")!();
|
||||||
|
|
||||||
|
expect(removeInsertedCSS).toHaveBeenCalledWith("css-key-1");
|
||||||
|
});
|
||||||
|
|
||||||
|
it("does not attempt to remove the CSS if none was ever injected", async () => {
|
||||||
|
vi.spyOn(process, "platform", "get").mockReturnValue("darwin");
|
||||||
|
const { window, windowHandlers, removeInsertedCSS } = mockWindow();
|
||||||
|
|
||||||
|
setupMacosTitleBar(window);
|
||||||
|
windowHandlers.get("enter-full-screen")!();
|
||||||
|
await flushStyling();
|
||||||
|
|
||||||
|
expect(removeInsertedCSS).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it("re-injects the CSS when leaving full screen", async () => {
|
||||||
|
vi.spyOn(process, "platform", "get").mockReturnValue("darwin");
|
||||||
|
const { window, windowHandlers, insertCSS } = mockWindow();
|
||||||
|
|
||||||
|
setupMacosTitleBar(window);
|
||||||
|
windowHandlers.get("leave-full-screen")!();
|
||||||
|
await flushStyling();
|
||||||
|
|
||||||
|
expect(insertCSS).toHaveBeenCalledWith(buildTitleBarCss());
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
@@ -7,13 +7,20 @@ Please see LICENSE files in the repository root for full details.
|
|||||||
|
|
||||||
import type { BrowserWindow } from "electron";
|
import type { BrowserWindow } from "electron";
|
||||||
|
|
||||||
export function setupMacosTitleBar(window: BrowserWindow): void {
|
/**
|
||||||
if (process.platform !== "darwin") return;
|
* Build the CSS injected into the renderer to make the (native-title-bar-less) macOS window draggable.
|
||||||
|
*
|
||||||
let cssKey: string | undefined;
|
* Because `electron-main.ts` uses `titleBarStyle: "hidden"` there is no native title bar, so the only way
|
||||||
|
* to drag the window is via `-webkit-app-region: drag` strips. The `::before` strips above the room and
|
||||||
async function applyStyling(): Promise<void> {
|
* left-panel headers were previously ~13px tall and too small to reliably grab (#32018); they are raised
|
||||||
cssKey = await window.webContents.insertCSS(`
|
* to match the 32px traffic-light offset used elsewhere in this file. Interactive controls keep
|
||||||
|
* `-webkit-app-region: no-drag` so they remain clickable (an element must never be both clickable and a
|
||||||
|
* drag handle).
|
||||||
|
*
|
||||||
|
* Extracted as a pure helper so the string contract can be unit-tested (see macos-titlebar.test.ts).
|
||||||
|
*/
|
||||||
|
export function buildTitleBarCss(): string {
|
||||||
|
return `
|
||||||
/* Create margin of space for the traffic light buttons */
|
/* Create margin of space for the traffic light buttons */
|
||||||
.mx_UserMenu {
|
.mx_UserMenu {
|
||||||
/* We zero the margin and use padding as we want to use it as a drag handle */
|
/* We zero the margin and use padding as we want to use it as a drag handle */
|
||||||
@@ -124,7 +131,8 @@ export function setupMacosTitleBar(window: BrowserWindow): void {
|
|||||||
|
|
||||||
.mx_LeftPanel::before {
|
.mx_LeftPanel::before {
|
||||||
content: "";
|
content: "";
|
||||||
height: 13px;
|
/* Aligned with the 32px traffic-light offset so the empty top band is grabbable (#32018) */
|
||||||
|
height: 32px;
|
||||||
border-right: 1px solid var(--cpd-color-bg-subtle-primary);
|
border-right: 1px solid var(--cpd-color-bg-subtle-primary);
|
||||||
-webkit-app-region: drag;
|
-webkit-app-region: drag;
|
||||||
}
|
}
|
||||||
@@ -134,16 +142,27 @@ export function setupMacosTitleBar(window: BrowserWindow): void {
|
|||||||
content: "";
|
content: "";
|
||||||
-webkit-app-region: drag;
|
-webkit-app-region: drag;
|
||||||
}
|
}
|
||||||
|
|
||||||
.mx_SpaceRoomView::before {
|
.mx_SpaceRoomView::before {
|
||||||
display: block;
|
display: block;
|
||||||
height: 24px;
|
/* Enlarged to match the traffic-light offset for a comfortable drag zone (#32018) */
|
||||||
|
height: 32px;
|
||||||
}
|
}
|
||||||
|
|
||||||
.mx_RoomView::before {
|
.mx_RoomView::before {
|
||||||
height: 13px;
|
/* Enlarged from 13px to cover the empty band above the 64px room header (#32018) */
|
||||||
|
height: 32px;
|
||||||
}
|
}
|
||||||
`);
|
`;
|
||||||
|
}
|
||||||
|
|
||||||
|
export function setupMacosTitleBar(window: BrowserWindow): void {
|
||||||
|
if (process.platform !== "darwin") return;
|
||||||
|
|
||||||
|
let cssKey: string | undefined;
|
||||||
|
|
||||||
|
async function applyStyling(): Promise<void> {
|
||||||
|
cssKey = await window.webContents.insertCSS(buildTitleBarCss());
|
||||||
}
|
}
|
||||||
|
|
||||||
window.on("enter-full-screen", () => {
|
window.on("enter-full-screen", () => {
|
||||||
|
|||||||
Reference in New Issue
Block a user