diff --git a/e2e-tests/tests/isolated/README.md b/e2e-tests/tests/isolated/README.md index d6e6398a2ed..35b7b2d6f8c 100644 --- a/e2e-tests/tests/isolated/README.md +++ b/e2e-tests/tests/isolated/README.md @@ -21,4 +21,8 @@ npx playwright test --config e2e-tests/playwright.config.ts --project=isolated e ## Subdirectories - `comment-assignment/` — tests for assigning comments to users +- `navigation-history/` — tests for back/forward reference history navigation - `overlay/` — tests for the project-switch transition overlay +- `scroll-groups/` — tests for scroll-group synchronization between scripture editors +- `title-bar/` — tests for title bar layout, e.g. reserved space for native window controls +- `verse-navigation/` — tests for verse navigation keyboard shortcuts diff --git a/e2e-tests/tests/isolated/title-bar/title-bar-reserved-space.spec.ts b/e2e-tests/tests/isolated/title-bar/title-bar-reserved-space.spec.ts new file mode 100644 index 00000000000..44a03d96d3d --- /dev/null +++ b/e2e-tests/tests/isolated/title-bar/title-bar-reserved-space.spec.ts @@ -0,0 +1,45 @@ +import { test, expect } from '../../../fixtures/cdp.fixture'; +import { waitForAppReady } from '../../../fixtures/helpers'; + +/** Window Controls Overlay API isn't in TS's DOM lib yet. */ +type NavigatorWithWindowControlsOverlay = Navigator & { + windowControlsOverlay?: { getTitlebarAreaRect(): DOMRect }; +}; + +test.describe('Title bar reserved space', () => { + // titleBarOverlay (main.ts) — and navigator.windowControlsOverlay with it — is Windows-only. + // Skip must be called at describe/test scope; the equivalent call inside beforeAll is a silent + // Playwright no-op, so both tests would otherwise run (and fail) on macOS/Linux. + test.skip(process.platform !== 'win32', 'titleBarOverlay only applies on win32 (see main.ts)'); + + test('profile button stays clear of the OS-reserved title bar area', async ({ mainPage }) => { + await waitForAppReady(mainPage); + + const overlayRight = await mainPage.evaluate(() => { + // Widening navigator to the non-standard API above + // eslint-disable-next-line no-type-assertion/no-type-assertion + const { windowControlsOverlay } = navigator as NavigatorWithWindowControlsOverlay; + return windowControlsOverlay?.getTitlebarAreaRect().right; + }); + expect(overlayRight).toBeDefined(); + + const trigger = mainPage.locator('[data-testid="user-profile-popover-trigger"]'); + await expect(trigger).toBeVisible(); + const triggerBox = await trigger.boundingBox(); + expect(triggerBox).not.toBeNull(); + if (!triggerBox || overlayRight === undefined) return; + + // A plain visibility check would not have caught the original bug: the button was DOM-visible + // the whole time, just painted over by the native (non-DOM) OS buttons on top of it. + expect(triggerBox.x + triggerBox.width).toBeLessThanOrEqual(overlayRight); + }); + + test('clicking the profile button opens its popover', async ({ mainPage }) => { + await waitForAppReady(mainPage); + + const trigger = mainPage.locator('[data-testid="user-profile-popover-trigger"]'); + await trigger.click(); + + await expect(mainPage.locator('[data-testid="user-profile-name"]')).toBeVisible(); + }); +}); diff --git a/src/renderer/components/platform-bible-toolbar.test.tsx b/src/renderer/components/platform-bible-toolbar.test.tsx index 73ef99f6061..b42bac4f40a 100644 --- a/src/renderer/components/platform-bible-toolbar.test.tsx +++ b/src/renderer/components/platform-bible-toolbar.test.tsx @@ -4,6 +4,7 @@ import { vi } from 'vitest'; import React from 'react'; import { useScrollGroupScrRef, useSetting } from '@renderer/hooks/papi-hooks'; import { useNavigationTargetWebView } from '@renderer/hooks/use-navigation-target-web-view.hook'; +import { useWindowControlsOverlay } from '@renderer/hooks/use-window-controls-overlay.hook'; import { ResolvedWebView } from '@renderer/services/navigation-target.util'; import { updateWebViewDefinitionSync } from '@renderer/services/web-view.service-host'; import { sendCommand } from '@shared/services/command.service'; @@ -60,6 +61,10 @@ vi.mock('@renderer/hooks/use-navigation-target-web-view.hook', () => ({ useNavigationTargetWebView: vi.fn((): ResolvedWebView | undefined => undefined), })); +vi.mock('@renderer/hooks/use-window-controls-overlay.hook', () => ({ + useWindowControlsOverlay: vi.fn((): DOMRect | undefined => undefined), +})); + vi.mock('@renderer/services/web-view.service-host', () => ({ updateWebViewDefinitionSync: vi.fn(() => true), })); @@ -129,14 +134,18 @@ vi.mock('platform-bible-react', async (importOriginal) => { const actual = await importOriginal(); return { ...actual, + // `className` is captured on a testid'd wrapper so tests can assert whether the static + // OS-reserved-space class is applied, without depending on the real Toolbar's DOM structure. Toolbar: ({ + className, configAreaChildren, children, }: { + className?: string; configAreaChildren?: React.ReactNode; children?: React.ReactNode; }) => ( -
+
{configAreaChildren}
{children}
@@ -669,3 +678,104 @@ describe('PlatformBibleToolbar — scroll group write-back to the resolved targe expect(vi.mocked(updateWebViewDefinitionSync)).not.toHaveBeenCalled(); }); }); + +describe('PlatformBibleToolbar — title bar reserved space', () => { + const mockSendCommandForOS = (osPlatform: string) => { + vi.mocked(sendCommand).mockImplementation( + // sendCommand has a complex generic signature; cast is required for the mock implementation + // eslint-disable-next-line no-type-assertion/no-type-assertion, @typescript-eslint/no-explicit-any + (async (commandName: string) => { + if (commandName === 'platformGetResources.isSendReceiveAvailable') return true; + if (commandName === 'platform.getOSPlatform') return osPlatform; + if (commandName === 'platform.isFullScreen') return false; + return undefined; + // sendCommand has a complex generic signature; cast is required for the mock implementation + // eslint-disable-next-line no-type-assertion/no-type-assertion, @typescript-eslint/no-explicit-any + }) as any, + ); + }; + + beforeEach(() => { + vi.clearAllMocks(); + // clearAllMocks() does not reset a prior test's mockReturnValue (see precedent above), so + // restore the default explicitly + vi.mocked(useWindowControlsOverlay).mockReturnValue(undefined); + }); + + it('reserves the live-measured overlay width plus breathing room on Windows, and does not also apply the static class', async () => { + vi.mocked(useWindowControlsOverlay).mockReturnValue( + new DOMRect(0, 0, window.innerWidth - 150, 32), + ); + mockSendCommandForOS('win32'); + + render(); + + await waitFor(() => { + // OS controls area width 150px + 4px breathing room (RESERVED_SPACE_BREATHING_ROOM_PX) + expect(screen.getByTestId('toolbar-reserved-space-wrapper')).toHaveStyle({ + paddingRight: '154px', + }); + }); + expect(screen.getByTestId('toolbar-root')).not.toHaveClass('tw:pe-[calc(138px+1rem)]'); + // Toolbar's own container has an unconditional border and tw:px-4 (16px end padding); when the + // wrapper above reserves the trailing space, Toolbar's own border must be dropped entirely (not + // just the end side) and its own end-side padding suppressed, or the border stops short at + // Toolbar's narrower edge instead of enclosing the reserved strip, and the wrapper's live + // measurement stacks on top of the 16px, over-reserving space. + expect(screen.getByTestId('toolbar-root')).toHaveClass('tw:border-0'); + expect(screen.getByTestId('toolbar-root')).toHaveClass('tw:pe-0'); + // The wrapper carries an equivalent border itself (as a layout-neutral box-shadow, not an + // actual border — see the toolbarReservedSpaceStyle comment in the component), so the outline + // encloses the full toolbar-plus-reserved-space region on every side instead of stopping short + // at Toolbar's narrower edge. + expect(screen.getByTestId('toolbar-reserved-space-wrapper')).toHaveStyle({ + boxShadow: 'inset 0 0 0 1px var(--border)', + }); + }); + + it('reserves space on the left when the live-measured gap is on the left (e.g., RTL locales)', async () => { + // left = 150, right = window.innerWidth: the gap sits on the left instead of the right. + vi.mocked(useWindowControlsOverlay).mockReturnValue( + new DOMRect(150, 0, window.innerWidth - 150, 32), + ); + mockSendCommandForOS('win32'); + + render(); + + await waitFor(() => { + // 150px measured gap + 4px breathing room (RESERVED_SPACE_BREATHING_ROOM_PX) + expect(screen.getByTestId('toolbar-reserved-space-wrapper')).toHaveStyle({ + paddingLeft: '154px', + }); + }); + }); + + it('applies no inline override while the overlay geometry is not yet known, falling back to the static class', async () => { + mockSendCommandForOS('win32'); + + render(); + + await waitFor(() => { + expect(screen.getByTestId('user-profile-popover-stub')).toBeInTheDocument(); + }); + expect(screen.getByTestId('toolbar-reserved-space-wrapper')).not.toHaveAttribute('style'); + expect(screen.getByTestId('toolbar-root')).toHaveClass('tw:pe-[calc(138px+1rem)]'); + expect(screen.getByTestId('toolbar-root')).not.toHaveClass('tw:border-0'); + expect(screen.getByTestId('toolbar-root')).not.toHaveClass('tw:pe-0'); + }); + + it('does not reserve space on macOS regardless of overlay geometry, keeping the static traffic-lights class', async () => { + vi.mocked(useWindowControlsOverlay).mockReturnValue(new DOMRect(0, 0, 700, 32)); + mockSendCommandForOS('darwin'); + + render(); + + await waitFor(() => { + expect(screen.getByTestId('user-profile-popover-stub')).toBeInTheDocument(); + }); + expect(screen.getByTestId('toolbar-reserved-space-wrapper')).not.toHaveAttribute('style'); + expect(screen.getByTestId('toolbar-root')).toHaveClass('tw:ps-[85px]'); + expect(screen.getByTestId('toolbar-root')).not.toHaveClass('tw:border-0'); + expect(screen.getByTestId('toolbar-root')).not.toHaveClass('tw:pe-0'); + }); +}); diff --git a/src/renderer/components/platform-bible-toolbar.tsx b/src/renderer/components/platform-bible-toolbar.tsx index 7d6815f3259..9f7fd6989e3 100644 --- a/src/renderer/components/platform-bible-toolbar.tsx +++ b/src/renderer/components/platform-bible-toolbar.tsx @@ -12,6 +12,7 @@ import { import { useIsPowerMode } from '@renderer/hooks/use-is-power-mode.hook'; import { useProjectPickerData } from '@renderer/hooks/use-project-picker-data.hook'; import { useNavigationTargetWebView } from '@renderer/hooks/use-navigation-target-web-view.hook'; +import { useWindowControlsOverlay } from '@renderer/hooks/use-window-controls-overlay.hook'; import { PROJECT_PICKER_DIALOG_TYPE } from '@renderer/components/dialogs/dialog-definition.model'; import { app, dataProviders } from '@renderer/services/papi-frontend.service'; import { availableScrollGroupIds } from '@renderer/services/scroll-group.service-host'; @@ -59,7 +60,7 @@ import { isPlatformError, LocalizeKey, } from 'platform-bible-utils'; -import { useCallback, useEffect, useMemo, useRef, useState } from 'react'; +import { CSSProperties, useCallback, useEffect, useMemo, useRef, useState } from 'react'; const TOOLTIP_DELAY = 300; @@ -68,6 +69,11 @@ const TOOLTIP_DELAY = 300; // after that initial window, so a single retry here is sufficient. const SEND_RECEIVE_AVAILABILITY_STARTUP_RETRY_MS = 2000; +// Visual breathing room between content and the native buttons on top of the live-measured overlay +// width. Tuned by eye — smaller than the static reserved-space guess's 1rem (see +// getToolbarOSReservedSpaceClassName) because the live measurement is exact, unlike that guess. +const RESERVED_SPACE_BREATHING_ROOM_PX = 4; + const scrollGroupLocalizedStringKeys = getLocalizeKeysForScrollGroupIds(availableScrollGroupIds); const LOCALIZED_STRING_KEYS: LocalizeKey[] = [ @@ -209,6 +215,45 @@ export function PlatformBibleToolbar() { undefined, ); + // Overrides the static Windows/Linux padding guess (applied to Toolbar's own className below) + // with a wrapper div carrying the live-measured caption-button width (see + // useWindowControlsOverlay), so it isn't reserved twice. macOS's fixed-width traffic lights + // still use the static class as-is. + const windowControlsOverlayRect = useWindowControlsOverlay(); + const toolbarReservedSpaceStyle: CSSProperties | undefined = + osPlatformToReserveSpaceFor !== undefined && + osPlatformToReserveSpaceFor !== 'darwin' && + windowControlsOverlayRect + ? { + // Physical paddingLeft/paddingRight, chosen from the live-measured rect: Windows moves + // the caption buttons to the physical left in RTL locales. Deriving the side from + // windowControlsOverlayRect itself is correct in both directions since it reflects the + // buttons' actual measured position. + // +RESERVED_SPACE_BREATHING_ROOM_PX — without it, content sits pixel-flush against the + // native buttons, which reads as cramped even though nothing actually overlaps. + ...(windowControlsOverlayRect.left > 0 + ? { paddingLeft: windowControlsOverlayRect.left + RESERVED_SPACE_BREATHING_ROOM_PX } + : undefined), + ...(window.innerWidth - windowControlsOverlayRect.right > 0 + ? { + paddingRight: + window.innerWidth - + windowControlsOverlayRect.right + + RESERVED_SPACE_BREATHING_ROOM_PX, + } + : undefined), + // @ts-ignore Electron-only property, not in React's CSSProperties type. Toolbar's own + // drag area (shouldUseAsAppDragArea) doesn't extend into this wrapper, so this strip + // needs its own drag region or the window can no longer be dragged from here. + WebkitAppRegion: 'drag', + // An inset box-shadow, not a border: this div has no explicit height, so a real border + // would add to its layout height, throwing off WorkspaceUpdatingOverlay's hardcoded `top` + // whenever this branch is active. A box-shadow paints in the same place without occupying + // any layout space. var(--border) matches the color Toolbar's own tw:border resolves to. + boxShadow: 'inset 0 0 0 1px var(--border)', + } + : undefined; + const [updateMenuData, setUpdateMenuData] = useState(false); const [menuData] = usePromise( @@ -291,191 +336,202 @@ export function PlatformBibleToolbar() { useEvent(onDidReloadExtensions, checkIfSendReceiveAvailable); return ( - { - setUpdateMenuData(isOpen); - }} - onSelectMenuItem={handleMenuCommand} - className={cn( - // If the toolbar height changes, the top inset for the workspace updating overlay will need to be updated too. - 'tw:h-12 tw:bg-transparent', - getToolbarOSReservedSpaceClassName(osPlatformToReserveSpaceFor), - )} - menubarVariant="muted" - shouldUseAsAppDragArea - appMenuAreaChildren={Application Logo} - configAreaChildren={ - <> - {isSendReceiveAvailable !== false && ( - // While loading (undefined), the button stays in the DOM so layout doesn't shift, but - // is hidden via tw:invisible (visual), aria-hidden (accessibility tree), and tabIndex=-1 - // (keyboard navigation). All three are required: tw:invisible alone is still reachable - // by AT and keyboard; aria-hidden alone is still tab-focusable. - - - - + + +

+ {localizedStrings['%toolbar_sync_open_status%']} +

+ + + + )} + {marketingVersion !== '' && ( + + + + + {marketingVersion} + + + +

{marketingVersion}

+ + + + )} + + + } + > + + + + + + {localizedStrings['%mainMenu_openHome%'] && ( + +

{localizedStrings['%mainMenu_openHome%']}

+ + )} + + + {!isPowerMode && ( + { - try { - await openProject(projectId); - } catch (e: unknown) { - logger.warn( - `Toolbar caught an error while trying to open project ${projectId}: ${getErrorMessage(e)}`, - ); - } - }} - disabled={!hasProjectPickerItems} - > - - - {currentProject && ( - + )} + + + {hasProjectPickerItems && ( + + {projectPickerItems.map((p) => ( + + {p.fullName} ({p.shortName}) + + ))} + + - - )} - - )} - {typeof scrollGroupId === 'number' && ( - // Key on the scroll group so switching groups remounts and re-seeds the history state. - - )} - - {isPowerMode && ( - + + )} + + )} + {typeof scrollGroupId === 'number' && ( + // Key on the scroll group so switching groups remounts and re-seeds the history state. + + )} + - )} - + {isPowerMode && ( + + )} + +
); } diff --git a/src/renderer/hooks/use-window-controls-overlay.hook.test.ts b/src/renderer/hooks/use-window-controls-overlay.hook.test.ts new file mode 100644 index 00000000000..183aedb4aeb --- /dev/null +++ b/src/renderer/hooks/use-window-controls-overlay.hook.test.ts @@ -0,0 +1,95 @@ +import { renderHook, act } from '@testing-library/react'; +import { vi } from 'vitest'; +import { useWindowControlsOverlay } from './use-window-controls-overlay.hook'; + +type GeometryChangeListener = (event: unknown) => void; + +function installMockOverlay(initialVisible: boolean, initialRect: DOMRect) { + const listeners: GeometryChangeListener[] = []; + const overlay = { + visible: initialVisible, + getTitlebarAreaRect: vi.fn(() => initialRect), + addEventListener: vi.fn((_type: string, listener: GeometryChangeListener) => { + listeners.push(listener); + }), + removeEventListener: vi.fn((_type: string, listener: GeometryChangeListener) => { + const index = listeners.indexOf(listener); + if (index !== -1) listeners.splice(index, 1); + }), + }; + Object.defineProperty(navigator, 'windowControlsOverlay', { + value: overlay, + configurable: true, + }); + + return { + overlay, + fireGeometryChange: (visible: boolean, rect: DOMRect) => { + overlay.visible = visible; + overlay.getTitlebarAreaRect.mockReturnValue(rect); + listeners.forEach((listener) => listener({})); + }, + }; +} + +afterEach(() => { + // navigator's ambient type marks this readonly; deleting it here undoes installMockOverlay + // eslint-disable-next-line no-type-assertion/no-type-assertion + delete (navigator as { windowControlsOverlay?: unknown }).windowControlsOverlay; +}); + +describe('useWindowControlsOverlay', () => { + it('returns undefined when the Window Controls Overlay API is unavailable', () => { + const { result } = renderHook(() => useWindowControlsOverlay()); + + expect(result.current).toBeUndefined(); + }); + + it('returns the titlebar rect immediately when the overlay is visible at mount', () => { + const rect = new DOMRect(0, 0, 860, 32); + installMockOverlay(true, rect); + + const { result } = renderHook(() => useWindowControlsOverlay()); + + expect(result.current).toBe(rect); + }); + + it('returns undefined when the overlay exists but is not currently visible', () => { + installMockOverlay(false, new DOMRect(0, 0, 860, 32)); + + const { result } = renderHook(() => useWindowControlsOverlay()); + + expect(result.current).toBeUndefined(); + }); + + it('updates when a geometrychange event reports a new rect', () => { + const { fireGeometryChange } = installMockOverlay(true, new DOMRect(0, 0, 860, 32)); + const { result } = renderHook(() => useWindowControlsOverlay()); + + const narrowerRect = new DOMRect(0, 0, 700, 32); + act(() => fireGeometryChange(true, narrowerRect)); + + expect(result.current).toBe(narrowerRect); + }); + + it('returns undefined after a geometrychange event reports the overlay went invisible', () => { + const { fireGeometryChange } = installMockOverlay(true, new DOMRect(0, 0, 860, 32)); + const { result } = renderHook(() => useWindowControlsOverlay()); + + act(() => fireGeometryChange(false, new DOMRect(0, 0, 860, 32))); + + expect(result.current).toBeUndefined(); + }); + + it('removes its geometrychange listener on unmount', () => { + const { overlay } = installMockOverlay(true, new DOMRect(0, 0, 860, 32)); + const { unmount } = renderHook(() => useWindowControlsOverlay()); + + unmount(); + + expect(overlay.removeEventListener).toHaveBeenCalledWith( + 'geometrychange', + expect.any(Function), + ); + }); +}); diff --git a/src/renderer/hooks/use-window-controls-overlay.hook.ts b/src/renderer/hooks/use-window-controls-overlay.hook.ts new file mode 100644 index 00000000000..155f3ba3cb6 --- /dev/null +++ b/src/renderer/hooks/use-window-controls-overlay.hook.ts @@ -0,0 +1,33 @@ +import { useEffect, useState } from 'react'; + +/** + * Live geometry from Chromium's Window Controls Overlay API — the app-content rect once the native + * title bar buttons (rendered by Electron's `titleBarOverlay`, see main.ts) are excluded. A fixed + * pixel guess can't track DPI scaling, text-size settings, or OS theme differences that change the + * buttons' actual width; this reads the real value instead. + * + * Returns `undefined` when the API is unavailable (macOS/Linux) or the overlay isn't visible (e.g. + * full screen). + */ +export function useWindowControlsOverlay(): DOMRect | undefined { + const { windowControlsOverlay } = navigator; + + const [titlebarAreaRect, setTitlebarAreaRect] = useState(() => + windowControlsOverlay?.visible ? windowControlsOverlay.getTitlebarAreaRect() : undefined, + ); + + useEffect(() => { + if (!windowControlsOverlay) return; + + const updateRect = () => + setTitlebarAreaRect( + windowControlsOverlay.visible ? windowControlsOverlay.getTitlebarAreaRect() : undefined, + ); + + updateRect(); + windowControlsOverlay.addEventListener('geometrychange', updateRect); + return () => windowControlsOverlay.removeEventListener('geometrychange', updateRect); + }, [windowControlsOverlay]); + + return titlebarAreaRect; +} diff --git a/src/renderer/window-controls-overlay.d.ts b/src/renderer/window-controls-overlay.d.ts new file mode 100644 index 00000000000..52920ddc34d --- /dev/null +++ b/src/renderer/window-controls-overlay.d.ts @@ -0,0 +1,31 @@ +// Window Controls Overlay API — not yet included in TypeScript's lib.dom.d.ts. +// https://developer.mozilla.org/en-US/docs/Web/API/Window_Controls_Overlay_API +// Electron implements this for windows created with `titleBarOverlay` (see main.ts). + +interface WindowControlsOverlayGeometryChangeEvent extends Event { + readonly titlebarAreaRect: DOMRect; + readonly visible: boolean; +} + +interface WindowControlsOverlay { + readonly visible: boolean; + getTitlebarAreaRect(): DOMRect; + addEventListener( + type: 'geometrychange', + listener: (event: WindowControlsOverlayGeometryChangeEvent) => void, + options?: boolean | AddEventListenerOptions, + ): void; + removeEventListener( + type: 'geometrychange', + listener: (event: WindowControlsOverlayGeometryChangeEvent) => void, + options?: boolean | EventListenerOptions, + ): void; +} + +declare global { + interface Navigator { + readonly windowControlsOverlay?: WindowControlsOverlay; + } +} + +export {};