mirror of
https://github.com/nexu-io/open-design.git
synced 2026-05-31 19:04:39 +07:00
fix(web): exit PreviewModal fullscreen on first Esc press (#168)
Closes #141. When the user clicked the Fullscreen button, requestFullscreen() put the stage element into native browser fullscreen and React's `fullscreen` state was set true. Pressing Esc was meant to exit the overlay, but in browsers like Firefox the browser consumes Esc to drop its native fullscreen element without delivering keydown to JS. The React state stayed true, the `ds-modal-fullscreen` class lingered, and only a second Esc reached the keydown handler that flipped the state. Subscribe to `fullscreenchange` so the React state mirrors the native state. When the browser exits its fullscreen element, the overlay drops on the same keystroke. The keydown handler is still needed for the fallback path (no native fullscreen API support, where requestFullscreen is undefined and only React state is set). Adds three regression tests in e2e/tests/preview-modal-fullscreen.test.tsx covering the bug fix path, the keydown fallback, and a non-collapse guard for transitions where another element is still fullscreen. Co-authored-by: d 🔹 <258577966+voidborne-d@users.noreply.github.com>
This commit is contained in:
parent
2f8d998ae8
commit
f8af2cd875
2 changed files with 125 additions and 0 deletions
|
|
@ -72,6 +72,22 @@ export function PreviewModal({
|
||||||
return () => document.removeEventListener('keydown', onKey);
|
return () => document.removeEventListener('keydown', onKey);
|
||||||
}, [onClose, fullscreen]);
|
}, [onClose, fullscreen]);
|
||||||
|
|
||||||
|
// Mirror native fullscreen state into React. Without this, a user in
|
||||||
|
// browser fullscreen has to press Esc twice: the first Esc exits the
|
||||||
|
// native fullscreen element (consumed by the browser; in some browsers no
|
||||||
|
// keydown is delivered) while our `fullscreen` state stays true and the
|
||||||
|
// overlay keeps its `ds-modal-fullscreen` class. Listening to
|
||||||
|
// fullscreenchange lets one Esc dismiss both layers in lock-step.
|
||||||
|
useEffect(() => {
|
||||||
|
const onFsChange = () => {
|
||||||
|
if (!document.fullscreenElement) {
|
||||||
|
setFullscreen(false);
|
||||||
|
}
|
||||||
|
};
|
||||||
|
document.addEventListener('fullscreenchange', onFsChange);
|
||||||
|
return () => document.removeEventListener('fullscreenchange', onFsChange);
|
||||||
|
}, []);
|
||||||
|
|
||||||
// Close share popover on outside click / Escape.
|
// Close share popover on outside click / Escape.
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (!shareOpen) return;
|
if (!shareOpen) return;
|
||||||
|
|
|
||||||
109
e2e/tests/preview-modal-fullscreen.test.tsx
Normal file
109
e2e/tests/preview-modal-fullscreen.test.tsx
Normal file
|
|
@ -0,0 +1,109 @@
|
||||||
|
import { act, cleanup, fireEvent, render } from '@testing-library/react';
|
||||||
|
import { afterEach, describe, expect, it, vi } from 'vitest';
|
||||||
|
import { PreviewModal } from '../../apps/web/src/components/PreviewModal';
|
||||||
|
|
||||||
|
// Regression coverage for nexu-io/open-design#141: pressing Esc in fullscreen
|
||||||
|
// used to require two presses because the browser exits its native fullscreen
|
||||||
|
// element on the first press without delivering a keydown to JS, leaving the
|
||||||
|
// React `fullscreen` state stuck on. The fix listens to fullscreenchange and
|
||||||
|
// mirrors the native state into React.
|
||||||
|
|
||||||
|
const baseProps = {
|
||||||
|
title: 'Sample',
|
||||||
|
views: [{ id: 'main', label: 'Main', html: '<p>hi</p>' }],
|
||||||
|
exportTitleFor: (id: string) => id,
|
||||||
|
};
|
||||||
|
|
||||||
|
function dispatchFullscreenChange() {
|
||||||
|
act(() => {
|
||||||
|
document.dispatchEvent(new Event('fullscreenchange'));
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
function setNativeFullscreenElement(el: Element | null) {
|
||||||
|
Object.defineProperty(document, 'fullscreenElement', {
|
||||||
|
configurable: true,
|
||||||
|
get: () => el,
|
||||||
|
});
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('PreviewModal fullscreen exit', () => {
|
||||||
|
afterEach(() => {
|
||||||
|
cleanup();
|
||||||
|
setNativeFullscreenElement(null);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('drops the fullscreen overlay when the browser exits native fullscreen', () => {
|
||||||
|
const onClose = vi.fn();
|
||||||
|
const { container } = render(
|
||||||
|
<PreviewModal {...baseProps} onClose={onClose} />,
|
||||||
|
);
|
||||||
|
|
||||||
|
// Click the Fullscreen button. jsdom does not implement requestFullscreen
|
||||||
|
// on plain elements, so PreviewModal's fallback path runs and just sets
|
||||||
|
// the React state — exactly matching what happens after a successful
|
||||||
|
// browser fullscreen request.
|
||||||
|
const fsButton = container.querySelector(
|
||||||
|
'button[title="Fullscreen"]',
|
||||||
|
) as HTMLButtonElement;
|
||||||
|
expect(fsButton).toBeTruthy();
|
||||||
|
fireEvent.click(fsButton);
|
||||||
|
const stage = container.querySelector('.ds-modal') as HTMLElement;
|
||||||
|
expect(stage.classList.contains('ds-modal-fullscreen')).toBe(true);
|
||||||
|
|
||||||
|
// Simulate the user pressing Esc in browser fullscreen: the browser
|
||||||
|
// exits its native fullscreen element and fires fullscreenchange, but
|
||||||
|
// (in browsers like Firefox) does not deliver the keydown to JS.
|
||||||
|
setNativeFullscreenElement(null);
|
||||||
|
dispatchFullscreenChange();
|
||||||
|
|
||||||
|
expect(stage.classList.contains('ds-modal-fullscreen')).toBe(false);
|
||||||
|
expect(onClose).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('keeps the modal mounted on Esc while fullscreen, and closes only on a second Esc', () => {
|
||||||
|
const onClose = vi.fn();
|
||||||
|
const { container } = render(
|
||||||
|
<PreviewModal {...baseProps} onClose={onClose} />,
|
||||||
|
);
|
||||||
|
const fsButton = container.querySelector(
|
||||||
|
'button[title="Fullscreen"]',
|
||||||
|
) as HTMLButtonElement;
|
||||||
|
fireEvent.click(fsButton);
|
||||||
|
const stage = container.querySelector('.ds-modal') as HTMLElement;
|
||||||
|
expect(stage.classList.contains('ds-modal-fullscreen')).toBe(true);
|
||||||
|
|
||||||
|
// First Esc — drops fullscreen, must not close the modal.
|
||||||
|
fireEvent.keyDown(document, { key: 'Escape' });
|
||||||
|
expect(stage.classList.contains('ds-modal-fullscreen')).toBe(false);
|
||||||
|
expect(onClose).not.toHaveBeenCalled();
|
||||||
|
|
||||||
|
// Second Esc — closes the modal.
|
||||||
|
fireEvent.keyDown(document, { key: 'Escape' });
|
||||||
|
expect(onClose).toHaveBeenCalledTimes(1);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('ignores fullscreenchange when another element is still fullscreen', () => {
|
||||||
|
const onClose = vi.fn();
|
||||||
|
const { container } = render(
|
||||||
|
<PreviewModal {...baseProps} onClose={onClose} />,
|
||||||
|
);
|
||||||
|
const fsButton = container.querySelector(
|
||||||
|
'button[title="Fullscreen"]',
|
||||||
|
) as HTMLButtonElement;
|
||||||
|
fireEvent.click(fsButton);
|
||||||
|
const stage = container.querySelector('.ds-modal') as HTMLElement;
|
||||||
|
expect(stage.classList.contains('ds-modal-fullscreen')).toBe(true);
|
||||||
|
|
||||||
|
// Some other element is the active fullscreen target — our overlay must
|
||||||
|
// not collapse to non-fullscreen on transitions that leave a different
|
||||||
|
// element fullscreen.
|
||||||
|
const other = document.createElement('div');
|
||||||
|
document.body.appendChild(other);
|
||||||
|
setNativeFullscreenElement(other);
|
||||||
|
dispatchFullscreenChange();
|
||||||
|
|
||||||
|
expect(stage.classList.contains('ds-modal-fullscreen')).toBe(true);
|
||||||
|
document.body.removeChild(other);
|
||||||
|
});
|
||||||
|
});
|
||||||
Loading…
Reference in a new issue