mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-25 03:42:06 +00:00
feat(a11y): give the attachment viewer a real modal contract (TASK-2429)
`Lightbox` was a `role="presentation"` fixed div with a local `<svelte:window>`
keydown handler and no focus management at all. 3a's later tasks delete the
editor's hand-rolled `showModal()` dialog and route inline body images here, so
everything that dialog was getting from the platform for free has to exist here
first (DR-4b).
The contract:
- `role="dialog"` + `aria-modal="true"` + an accessible name (the image alt,
else "Attachment viewer"), tracking the image CURRENTLY shown. The controls
get real `aria-label`s too — their text is "✕" / "‹" / "›", and `title` does
not win over element content for the accessible name.
- Portaled to `<body>` DIRECTLY — deliberately not `portalAction.ts`, which
targets the nearest ancestor `<dialog>` when one exists: the opposite of
what a top-most surface needs. `<body>` is also the parent the backdrop
manager's inert bookkeeping requires, and the only one with no ancestor that
could establish a containing block and silently trap a `position: fixed`
overlay.
- `wsSlug` CAPTURED at open, not read live: the pane switches workspace
without remounting what is above it, so a live read could rebuild
already-captured attachment ids against a different workspace.
- Focus entry to the first TABBABLE DESCENDANT via `paneFocusables` (the root
is `tabindex="-1"` only as the no-controls-yet fallback); restore on close to
an OPTIONAL `invoker` prop, verified still connected AND focusable — else
focus is parked on `<body>` deliberately. With no invoker threaded it falls
back to whatever held focus at open, so the producers that thread one only in
TASK-2431 don't come out of this commit worse than they went in. The restore
declines when something else already owns focus.
- Background inertness via TASK-2427's manager (`acquire(exemptRoot)`), never
a hand-rolled `inert`. Released BEFORE the focus restore, and the returned
`stackEmpty` decides: with a viewer still open the manager has already handed
focus into it and this one stands down.
- Tab trap through `paneFocusables` / `nextTrapTarget` — the pane's tested trap
math, not a second implementation.
- ONLY the frontmost viewer (`isViewerFrontmost`) traps Tab, handles ←/→ and
consumes Escape. Handlers are global and `nextTrapTarget` deliberately
redirects out-of-container focus INWARD, so a background viewer would
otherwise drag focus out of the viewer in front of it. And the viewer stands
down entirely (`isBlockedByModal`) while a `showModal()` dialog is open over
it: the top layer is above any body-portaled surface, so the frontmost LEASE
is not always the frontmost SURFACE, and the manager keeps such a dialog
operable on purpose.
- Escape: the local branch is DELETED, not gated — it ignored
`defaultPrevented`, so alongside the stack it gave Escape two owners and let
one press collapse two layers. `escapeStack` is now the sole owner, at a new
`viewer` priority (50) above `menu` (40).
`AttachmentViewerHost` stops restoring focus itself and threads `invoker` down
instead. Its own restore ran while the viewer still held the backdrop lease —
i.e. while the invoker sat inside an `inert` body child, where it is not
focusable at all — so it would have silently become a no-op the moment this
commit landed. The only correct moment is after the lease is released, which is
inside the viewer's teardown.
Atomic with the route guards, because the stack is unreachable without them:
both `[collection]` and `[collection]/[slug]` bailed out of the ESC chain on
`document.querySelector('dialog[open], [role="dialog"]:not(.item-pane)')`, which
the viewer now matches — Escape would have been dead. Both call the shared
`hasForeignEscapeOwner()` instead. It KEEPS the ARIA branch (`BottomSheet` /
`DockedSheet` are shipped `role="dialog"` Escape owners with no stack
registration; dropping it would regress them) and narrows only the NATIVE branch
to a feature-detected `dialog:modal`, falling back to today's `dialog[open]`
where the pseudo-class is unsupported.
The backdrop's `z-index` goes to 100000. At 1000 it was under the desktop emoji
picker's body-portaled dropdown (99999) — and the app shell wrapper is
`display: contents`, so every fixed overlay in the tree competes in the ROOT
stacking context; being a body child is not protection. A surface that paints
over a viewer that has inerted it is visible-but-untouchable, the worst of both.
The full sweep, and the rule that a new overlay above this value is a bug, are
recorded at the declaration. The one thing legitimately above it is a native
`showModal()` dialog, which gets there via the top layer and no z-index at all.
Collision audit for `[role="dialog"]`-as-foreign-modal consumers:
- `app.css` `@media print` — the viewer matches, and should: a transient
overlay must not print. Documented in place (a JS-only grep misses this).
- `paneFocus.ts` `PANE_EXEMPT_SURFACE_SELECTOR` and `PaneHost`'s mobile trap +
focus-follows classifier — the viewer matches, and must: it runs its own
trap and key handling.
- `web/e2e/**` incl. `e2e/lib/*.ts` — no shared dialog selector constants; the
only bare `getByRole('dialog')` is
`workspace-bundle-roundtrip.spec.ts:204`, claimed by TASK-2430 by name.
Tests: 36 cases for the modal contract, 10 for `hasForeignEscapeOwner`, plus the
fallout of portaling (host tests can no longer scope by container — ownership is
proven by distinguishable payloads and destroying a known host; the strip's
Escape case now drives the stack). Comments state what jsdom cannot prove — real
inertness, layout/stacking, real Tab traversal — which is TASK-2436's browser
suite, and mark the one assertion whose guard is genuinely indistinguishable from
its neighbour in jsdom. Verified by mutation: 26 mutations run, all caught except
that documented one.
`escapeGuardWiring.svelte.test.ts` is a deliberately narrow addition: the
Lightbox tests drive a route-SHAPED driver they define themselves, so they prove
the shape and not that either route still calls it. This asserts against the real
source of both route files that `hasForeignEscapeOwner()` is imported, called as
an early return, and called BEFORE `runTopEscape()` — catching deletion,
re-inlining of the old selector, and reordering, which is the actual regression
risk. Mounting a route under vitest to prove it behaviourally is not worth its
cost; the behavioural proof is TASK-2436's browser suite. Two things stop it
being a grep that lies: comments are stripped first (every one of these strings
now appears in prose in those files, so a whole-file search could be satisfied by
a comment), and the assertions are scoped to the handler that actually calls
`runTopEscape` rather than to the file. Verified against a commented-out guard, a
guard moved to an unrelated helper, and a reverted selector under different
quoting — while a prettier-style reflow of the guard still passes.
TASK-2429
This commit is contained in:
xarmian
+6
-1
@@ -854,7 +854,12 @@ dialog.attachment-image-lightbox .attachment-image-lightbox-close:hover {
|
||||
is covered by the `dialog[open]` selector above.
|
||||
`:not(.item-pane)` keeps the detail pane printable: on mobile the pane
|
||||
carries role="dialog" (TASK-2131) but IS the content being printed, not a
|
||||
transient overlay to strip. */
|
||||
transient overlay to strip.
|
||||
The body-portaled attachment viewer (`.lightbox-backdrop.attachment-viewer`,
|
||||
TASK-2429) DOES match here, and should: it is a transient full-screen
|
||||
overlay, and printing with one open must print the item underneath, not a
|
||||
black rectangle. Audited deliberately — a JS-only grep for `role="dialog"`
|
||||
consumers misses this rule entirely. */
|
||||
[role="dialog"]:not(.item-pane),
|
||||
[role="tooltip"],
|
||||
[role="menu"] {
|
||||
|
||||
@@ -0,0 +1,115 @@
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { readFileSync } from 'node:fs';
|
||||
import { fileURLToPath } from 'node:url';
|
||||
|
||||
/**
|
||||
* WIRING CONTRACT for the two route ESC guards (TASK-2429).
|
||||
*
|
||||
* The viewer's Escape has exactly one owner — `escapeStack` — and the only code
|
||||
* that runs it is the keydown handler in each of these two route files. Both
|
||||
* used to bail on an inline
|
||||
* `document.querySelector('dialog[open], [role="dialog"]:not(.item-pane)')`,
|
||||
* which the body-portaled viewer now MATCHES; without the shared
|
||||
* `hasForeignEscapeOwner()` (which excludes it) the guard returns early, the
|
||||
* stack never runs, and Escape closes nothing at all. That is a silent,
|
||||
* app-wide dead key.
|
||||
*
|
||||
* WHY A SOURCE ASSERTION. The unit tests around `Lightbox` drive a
|
||||
* ROUTE-SHAPED driver they define themselves, so they prove the shape works —
|
||||
* not that either route still calls it. Mounting a SvelteKit route under vitest
|
||||
* to prove the real thing costs far more than it is worth (a 2,900-line
|
||||
* component, its stores, its data loaders), while the actual regression risk is
|
||||
* mundane: someone deletes or reverts the call, or re-inlines the old selector
|
||||
* during a merge. A grep-shaped contract catches exactly that, and nothing else
|
||||
* pretends to be covered here.
|
||||
*
|
||||
* TWO THINGS KEEP IT FROM BEING A GREP THAT LIES:
|
||||
* • COMMENTS ARE STRIPPED FIRST. Every one of these strings appears in prose
|
||||
* in these files (this commit added several), so a whole-file search could
|
||||
* be satisfied by a comment while the handler ran unguarded.
|
||||
* • ASSERTIONS ARE SCOPED TO THE HANDLER that actually calls `runTopEscape`,
|
||||
* not to the file. A guard in some unrelated helper is not this contract.
|
||||
*
|
||||
* The BEHAVIOURAL proof — a real Escape press, in a real browser, closing
|
||||
* exactly the viewer and not the pane beneath it — belongs to TASK-2436's
|
||||
* Playwright suite (DR-9), together with the inertness and stacking guarantees
|
||||
* jsdom cannot see either.
|
||||
*/
|
||||
|
||||
const ROUTES = [
|
||||
'../../routes/[username]/[workspace]/[collection]/+page.svelte',
|
||||
'../../routes/[username]/[workspace]/[collection]/[slug]/+page.svelte',
|
||||
] as const;
|
||||
|
||||
/** The guard call, tolerant of formatting (prettier reflow, added braces). */
|
||||
const GUARD = /if\s*\(\s*hasForeignEscapeOwner\(\)\s*\)\s*\{?\s*return\s*;/;
|
||||
/** Any `querySelector` whose selector string reaches for a `role="dialog"`. */
|
||||
const RAW_DIALOG_QUERY = /querySelector\w*\(\s*(['"`])[^'"`]*\[\s*role\s*=\s*\\?["']?dialog/;
|
||||
|
||||
/**
|
||||
* Source with comments removed. Line comments are matched only when the `//`
|
||||
* is not preceded by `:`, so URLs (`https://…`) survive — the point is to drop
|
||||
* PROSE, not to be a parser.
|
||||
*/
|
||||
function stripComments(source: string): string {
|
||||
return source
|
||||
.replace(/\/\*[\s\S]*?\*\//g, '')
|
||||
.replace(/<!--[\s\S]*?-->/g, '')
|
||||
.replace(/(^|[^:])\/\/[^\n]*/g, '$1');
|
||||
}
|
||||
|
||||
/**
|
||||
* The body of the function that calls `runTopEscape()`, from its `function`
|
||||
* keyword up to that call — i.e. exactly the region the guard must sit in.
|
||||
* Returns null when there is no such call at all, which is itself a failure.
|
||||
*/
|
||||
function escapeHandlerPrologue(code: string): string | null {
|
||||
const call = code.search(/runTopEscape\s*\(/);
|
||||
if (call === -1) return null;
|
||||
const start = code.lastIndexOf('function ', call);
|
||||
if (start === -1) return null;
|
||||
return code.slice(start, call);
|
||||
}
|
||||
|
||||
describe.each(ROUTES)('ESC guard wiring in %s', (relative) => {
|
||||
const source = readFileSync(fileURLToPath(new URL(relative, import.meta.url)), 'utf8');
|
||||
const code = stripComments(source);
|
||||
|
||||
it('imports the shared guard from the a11y module', () => {
|
||||
expect(code).toMatch(
|
||||
/import\s*\{[^}]*\bhasForeignEscapeOwner\b[^}]*\}\s*from\s*'\$lib\/a11y\/viewerBackdrop'/
|
||||
);
|
||||
});
|
||||
|
||||
it('calls it as an early return INSIDE the handler that runs the stack', () => {
|
||||
// Scoped, not file-wide: this is the assertion that would otherwise be
|
||||
// satisfiable by prose or by an unrelated helper. `return` (not
|
||||
// `preventDefault`) is the point — a foreign modal owns the key outright,
|
||||
// so the stack must not run at all.
|
||||
const prologue = escapeHandlerPrologue(code);
|
||||
expect(prologue).not.toBeNull();
|
||||
expect(prologue!).toMatch(GUARD);
|
||||
});
|
||||
|
||||
it('does not reach the stack without passing the guard first', () => {
|
||||
// The ordering IS the contract: running the stack first would close the
|
||||
// viewer out from under a modal that owns the key, and guarding after the
|
||||
// fact would do nothing. Falls out of the scoping above — the prologue
|
||||
// ends AT the call — so this re-states it against the whole file, where a
|
||||
// second, unguarded `runTopEscape` would also show up.
|
||||
const occurrences = code.match(/runTopEscape\s*\(/g) ?? [];
|
||||
expect(occurrences).toHaveLength(1);
|
||||
const guardAt = code.search(GUARD);
|
||||
const stackAt = code.search(/runTopEscape\s*\(/);
|
||||
expect(guardAt).toBeGreaterThan(-1);
|
||||
expect(guardAt).toBeLessThan(stackAt);
|
||||
});
|
||||
|
||||
it('no longer bails on a raw role="dialog" query', () => {
|
||||
// The pre-TASK-2429 inline check, re-introduced by hand or by a bad merge
|
||||
// resolution, is the regression this file exists to catch: it matches the
|
||||
// viewer's own `role="dialog"` root and kills its Escape. Matched by shape
|
||||
// rather than by one exact quoting, so a reformatted revert still trips it.
|
||||
expect(code).not.toMatch(RAW_DIALOG_QUERY);
|
||||
});
|
||||
});
|
||||
@@ -3,6 +3,8 @@ import {
|
||||
acquire,
|
||||
isBlockedByModal,
|
||||
isViewerFrontmost,
|
||||
hasForeignEscapeOwner,
|
||||
VIEWER_ROOT_CLASS,
|
||||
__resetViewerBackdropForTests,
|
||||
} from './viewerBackdrop';
|
||||
|
||||
@@ -701,3 +703,116 @@ describe('isBlockedByModal', () => {
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('hasForeignEscapeOwner', () => {
|
||||
// The shared form of the existence check the two route ESC guards used to
|
||||
// hand-roll (TASK-2429). It answers a NARROWER question than
|
||||
// `isBlockedByModal`: not "is something in front of me" but "does a surface
|
||||
// that owns Escape ITSELF exist" — so a driver of the escape stack knows
|
||||
// whether to stand down entirely.
|
||||
|
||||
/** The shape a portaled viewer root has, including the marker class. */
|
||||
function viewerRoot(): HTMLElement {
|
||||
const el = bodyChild('viewer');
|
||||
el.setAttribute('role', 'dialog');
|
||||
el.classList.add(VIEWER_ROOT_CLASS);
|
||||
return el;
|
||||
}
|
||||
|
||||
/**
|
||||
* Emulate an engine where `dialog:modal` IS supported (jsdom throws on it —
|
||||
* probed, not assumed), with `modals` as the open modal dialogs. Only the
|
||||
* combined selector the module builds is intercepted.
|
||||
*/
|
||||
function mockModalSupport(modals: Element[]): void {
|
||||
const real = document.querySelector.bind(document);
|
||||
vi.spyOn(document, 'querySelector').mockImplementation((selector: string) => {
|
||||
if (!selector.startsWith('dialog:modal')) return real(selector);
|
||||
const rest = selector.slice(selector.indexOf(',') + 1).trim();
|
||||
return modals[0] ?? real(rest);
|
||||
});
|
||||
}
|
||||
|
||||
it('is false with nothing open, so today’s guards are unchanged', () => {
|
||||
bodyChild('app', '<button id="a">a</button>');
|
||||
expect(hasForeignEscapeOwner()).toBe(false);
|
||||
});
|
||||
|
||||
it('is true for an ARIA sheet — the shipped BottomSheet / DockedSheet', () => {
|
||||
// The regression fence for the branch that must NOT be dropped: both are
|
||||
// `role="dialog"` Escape owners with no escape-stack registration, so a
|
||||
// guard that stopped seeing them would let one press close two layers.
|
||||
bodyChild('sheet').setAttribute('role', 'dialog');
|
||||
expect(hasForeignEscapeOwner()).toBe(true);
|
||||
});
|
||||
|
||||
it('is false for the pane’s own mobile overlay', () => {
|
||||
const pane = bodyChild('pane');
|
||||
pane.setAttribute('role', 'dialog');
|
||||
pane.classList.add('item-pane');
|
||||
expect(hasForeignEscapeOwner()).toBe(false);
|
||||
});
|
||||
|
||||
it('is false for the attachment viewer: its Escape is on the stack', () => {
|
||||
// The whole reason this helper exists. A guard that treated the viewer as
|
||||
// a foreign modal would return before running the stack, and Escape would
|
||||
// close nothing at all.
|
||||
viewerRoot();
|
||||
expect(hasForeignEscapeOwner()).toBe(false);
|
||||
});
|
||||
|
||||
it('still sees a sheet opened WHILE a viewer is up', () => {
|
||||
// The exclusion is targeted at the viewer, not a blanket "a viewer is
|
||||
// open, so nothing else counts".
|
||||
viewerRoot();
|
||||
bodyChild('sheet').setAttribute('role', 'dialog');
|
||||
expect(hasForeignEscapeOwner()).toBe(true);
|
||||
});
|
||||
|
||||
it('falls back to `dialog[open]` where `:modal` is unsupported', () => {
|
||||
// jsdom is that engine (it throws on the pseudo-class), so this is the
|
||||
// path every other test here runs on. The fallback is deliberately the
|
||||
// PRE-TASK-2429 selector: where the narrower question can't be asked, the
|
||||
// answer is exactly today's behaviour, never something wider.
|
||||
const dialog = openModal();
|
||||
dialog.setAttribute('open', '');
|
||||
expect(hasForeignEscapeOwner()).toBe(true);
|
||||
});
|
||||
|
||||
it('does not count a CLOSED native <dialog> in the fallback path', () => {
|
||||
// `Modal.svelte` keeps its native <dialog> mounted at all times and drives
|
||||
// it with showModal()/close(), so a fallback that dropped the `[open]`
|
||||
// qualifier would report a foreign Escape owner on every page that merely
|
||||
// HAS a Modal — swallowing the viewer's Escape everywhere.
|
||||
openModal();
|
||||
expect(hasForeignEscapeOwner()).toBe(false);
|
||||
});
|
||||
|
||||
it('asks for `dialog:modal` before falling back', () => {
|
||||
// Guards against an implementation that simply hard-codes `[open]`.
|
||||
const seen: string[] = [];
|
||||
const real = document.querySelector.bind(document);
|
||||
vi.spyOn(document, 'querySelector').mockImplementation((selector: string) => {
|
||||
seen.push(selector);
|
||||
if (selector.startsWith('dialog:modal')) throw new SyntaxError('unsupported');
|
||||
return real(selector);
|
||||
});
|
||||
hasForeignEscapeOwner();
|
||||
expect(seen.some((s) => s.startsWith('dialog:modal'))).toBe(true);
|
||||
expect(seen.some((s) => s.includes('dialog[open]'))).toBe(true);
|
||||
});
|
||||
|
||||
it('on a supporting engine, an open NON-modal <dialog> does not count', () => {
|
||||
// The one behaviour change from the hand-rolled `dialog[open]` string: a
|
||||
// `show()` / declarative-open dialog never owned Escape, and
|
||||
// `Modal.svelte` keeps a native <dialog> mounted at all times.
|
||||
const dialog = openModal();
|
||||
dialog.setAttribute('open', '');
|
||||
mockModalSupport([]);
|
||||
expect(hasForeignEscapeOwner()).toBe(false);
|
||||
|
||||
// ...while a real `showModal()` one does.
|
||||
mockModalSupport([dialog]);
|
||||
expect(hasForeignEscapeOwner()).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -342,6 +342,58 @@ export function isBlockedByModal(owner?: Element | null): boolean {
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Class carried by every body-portaled VIEWER root (TASK-2429). The viewer is a
|
||||
* `role="dialog"`, so without a marker it is indistinguishable from the foreign
|
||||
* modals the app's ESC guards stand down for — and standing down for it would
|
||||
* leave Escape with NO owner, since the viewer's own Escape lives on
|
||||
* `escapeStack`. Exported (rather than typed twice) so the markup and the
|
||||
* selector below cannot drift apart.
|
||||
*/
|
||||
export const VIEWER_ROOT_CLASS = 'attachment-viewer';
|
||||
|
||||
/**
|
||||
* Is a modal surface open that owns Escape ITSELF, so an escape-stack driver
|
||||
* must stand down entirely?
|
||||
*
|
||||
* This is the shared form of the existence check the two route keydown handlers
|
||||
* hand-rolled as `document.querySelector('dialog[open], [role="dialog"]:not(.item-pane)')`.
|
||||
* Two deliberate differences from that string:
|
||||
*
|
||||
* • The NATIVE branch is feature-detected `dialog:modal`, not `dialog[open]`.
|
||||
* A non-modal `show()` / declarative `<dialog open>` never owned Escape, and
|
||||
* `Modal.svelte` is always mounted — so `[open]` was over-broad. Where the
|
||||
* pseudo-class is unsupported (jsdom, legacy engines) it falls back to
|
||||
* `dialog[open]`, i.e. exactly today's behaviour, never to something wider.
|
||||
* • The ARIA branch additionally excludes {@link VIEWER_ROOT_CLASS}. It is
|
||||
* otherwise UNCHANGED and deliberately kept: `BottomSheet` and `DockedSheet`
|
||||
* are shipped `role="dialog"` Escape owners with no stack registration, and
|
||||
* dropping the branch would regress both. `.item-pane` stays excluded for
|
||||
* the reason it always was (TASK-2131) — it is on the stack too.
|
||||
*
|
||||
* Existence-based, not target-based, for the reason recorded at the call sites:
|
||||
* a sheet that doesn't move focus into itself leaves `document.activeElement`
|
||||
* on the trigger underneath, so a `closest()` test would miss it.
|
||||
*
|
||||
* TASK-2430 folds this into {@link isBlockedByModal}'s three-way precedence
|
||||
* across all seven global Escape/key owners; 3a needs only the two route
|
||||
* guards to stop swallowing the viewer's Escape.
|
||||
*/
|
||||
export function hasForeignEscapeOwner(): boolean {
|
||||
if (!hasDocument()) return false;
|
||||
const aria = `[role="dialog"]:not(.item-pane):not(.${VIEWER_ROOT_CLASS})`;
|
||||
if (modalSelectorSupported !== false) {
|
||||
try {
|
||||
const found = !!document.querySelector(`dialog:modal, ${aria}`);
|
||||
modalSelectorSupported = true;
|
||||
return found;
|
||||
} catch {
|
||||
modalSelectorSupported = false;
|
||||
}
|
||||
}
|
||||
return !!document.querySelector(`dialog[open], ${aria}`);
|
||||
}
|
||||
|
||||
/** Test seam: drop all leases and observers without running focus handoff. */
|
||||
export function __resetViewerBackdropForTests(): void {
|
||||
stack.length = 0;
|
||||
|
||||
@@ -64,6 +64,14 @@ export function paneFocusables(
|
||||
* the `:not`, `closest()` from any in-pane element would match the pane and
|
||||
* wrongly mark the whole pane exempt (killing the mobile Tab trap and confusing
|
||||
* the classifier). A genuinely nested dialog opened FROM the pane still matches.
|
||||
*
|
||||
* The attachment viewer (TASK-2429) matches the ARIA branch, and MUST: it is a
|
||||
* body-portaled `role="dialog"` that runs its own Tab trap and key handling, so
|
||||
* both consumers have to leave it alone — exactly the case this set exists for.
|
||||
* That is the opposite of the route ESC guards, which have to look PAST it
|
||||
* (`hasForeignEscapeOwner`, `$lib/a11y/viewerBackdrop`) because its Escape is on
|
||||
* the shared stack. Same attribute, two different questions; audited as part of
|
||||
* TASK-2429's collision sweep.
|
||||
*/
|
||||
export const PANE_EXEMPT_SURFACE_SELECTOR =
|
||||
'dialog, [role="dialog"]:not(.item-pane), [role="menu"], [role="listbox"], .block-context-menu';
|
||||
|
||||
@@ -81,23 +81,26 @@
|
||||
return () => {
|
||||
if (target && request !== target) return;
|
||||
request = null;
|
||||
// Focus returns to the element that opened the viewer — the only use
|
||||
// this host makes of `invoker`. `Lightbox` manages no focus of its
|
||||
// own, so without this a close leaves focus on <body> and the
|
||||
// keyboard user restarts from the top of the document.
|
||||
// FOCUS RESTORE IS NOT DONE HERE (TASK-2429). It used to be — this
|
||||
// host focused `target.invoker` itself, because `Lightbox` managed no
|
||||
// focus at all. Now the viewer holds a backdrop lease that makes every
|
||||
// other body child `inert`, and the invoker is inside one of them: an
|
||||
// inert element is NOT FOCUSABLE, so a focus() from here — which runs
|
||||
// while the viewer is still mounted and the lease still held — would
|
||||
// silently do nothing and leave the keyboard user on <body>.
|
||||
//
|
||||
// `isConnected` because the opener can be gone by now: an editor
|
||||
// NodeView is re-rendered on any document change, and focusing a
|
||||
// detached node silently does nothing on some engines and moves
|
||||
// focus to <body> on others.
|
||||
const invoker = target?.invoker;
|
||||
if (invoker?.isConnected) invoker.focus();
|
||||
// Deliberately only on a USER close. The lifecycle teardown below
|
||||
// does not return focus: it fires because the item under the viewer
|
||||
// is being replaced, so the invoker is part of a subtree on its way
|
||||
// out — putting focus back into it would land the user inside
|
||||
// content that is about to disappear. Where focus goes after a
|
||||
// switch is the pane's business, not the viewer's.
|
||||
// The only correct moment is AFTER the lease is released, which is
|
||||
// inside the viewer's own teardown. So the invoker is threaded down as
|
||||
// a prop instead (see the markup below) and `Lightbox` owns the whole
|
||||
// restore, including the still-connected / still-focusable check that
|
||||
// an editor NodeView re-render makes necessary.
|
||||
//
|
||||
// This does mean the restore now also runs on the LIFECYCLE teardown
|
||||
// (an item switch), which this host previously refused on the grounds
|
||||
// that the invoker is in a subtree on its way out. That refusal is no
|
||||
// longer worth a special case: the viewer verifies the invoker still
|
||||
// takes focus, and where it doesn't, focus lands on <body> — which is
|
||||
// exactly where the old no-op left it.
|
||||
};
|
||||
}
|
||||
|
||||
@@ -156,7 +159,9 @@
|
||||
through `untrack`, so prop-sync does not work — opening a second image while
|
||||
the first viewer is up must produce a NEW component instance.
|
||||
|
||||
`wsSlug` comes off the request, per the note above.
|
||||
`wsSlug` comes off the request, per the note above. So does `invoker`: the
|
||||
viewer restores focus itself, after releasing the inert lease — see
|
||||
`closeRequest` for why this host must not do it.
|
||||
-->
|
||||
{#key request}
|
||||
{#if request}
|
||||
@@ -164,6 +169,7 @@
|
||||
images={[...request.images]}
|
||||
index={request.index}
|
||||
wsSlug={request.workspaceSlug}
|
||||
invoker={request.invoker}
|
||||
onClose={closeRequest(request)}
|
||||
/>
|
||||
{/if}
|
||||
|
||||
@@ -1,5 +1,7 @@
|
||||
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
|
||||
import { flushSync, mount, unmount } from 'svelte';
|
||||
import { __resetViewerBackdropForTests } from '$lib/a11y/viewerBackdrop';
|
||||
import { _resetEscapeStackForTests } from '$lib/stores/escapeStack';
|
||||
|
||||
// TASK-2428. The viewer is exercised THROUGH its host, because the host is
|
||||
// where the two rules that matter live: an event is consumed only when both
|
||||
@@ -71,12 +73,16 @@ function openEvent(over: Partial<ViewerEvent> = {}): ViewerEvent {
|
||||
}
|
||||
|
||||
/**
|
||||
* The viewer is a fixed-position overlay rendered into its host's own mount
|
||||
* container, which is what lets a two-host test say WHICH host opened rather
|
||||
* than only how many viewers exist.
|
||||
* Every open viewer in the document.
|
||||
*
|
||||
* DOM SCOPE CANNOT ANSWER "WHICH HOST" ANY MORE (TASK-2429): the viewer now
|
||||
* portals to `<body>`, so both hosts' viewers are siblings there regardless of
|
||||
* where their host is mounted. A two-host test proves ownership by DESTROYING
|
||||
* one host and watching which viewer goes with it, plus the two-direction count
|
||||
* below — not by querying inside a container.
|
||||
*/
|
||||
function viewers(scope: ParentNode = document): HTMLElement[] {
|
||||
return Array.from(scope.querySelectorAll<HTMLElement>('.lightbox-backdrop'));
|
||||
function viewers(): HTMLElement[] {
|
||||
return Array.from(document.body.querySelectorAll<HTMLElement>('.lightbox-backdrop'));
|
||||
}
|
||||
|
||||
function viewer(): HTMLElement | null {
|
||||
@@ -116,6 +122,11 @@ describe('AttachmentViewerHost', () => {
|
||||
while (mounted.length) unmount(mounted.pop()!);
|
||||
target.remove();
|
||||
document.querySelectorAll('.viewer-host-target').forEach((el) => el.remove());
|
||||
// The viewer holds a backdrop lease and an ESC-stack registration
|
||||
// (TASK-2429). Both are module-global, so a case that ends with one open
|
||||
// would otherwise leak `inert` writes and a handler into the next test.
|
||||
__resetViewerBackdropForTests();
|
||||
_resetEscapeStackForTests();
|
||||
});
|
||||
|
||||
/** Mounts a host in its OWN container, and hands that container back. */
|
||||
@@ -148,24 +159,37 @@ describe('AttachmentViewerHost', () => {
|
||||
});
|
||||
|
||||
it('ignores an event addressed to the OTHER host, with both mounted', () => {
|
||||
const containerA = mountHost(propsA);
|
||||
const containerB = mountHost(propsB);
|
||||
mountHost(propsA);
|
||||
const hostA = mounted[mounted.length - 1];
|
||||
mountHost(propsB);
|
||||
|
||||
// Same item, other host token: exactly one viewer may open, and it must
|
||||
// be the ADDRESSED one — counting viewers alone would pass if the wrong
|
||||
// host opened. Matching on itemId alone would open two.
|
||||
// DISTINGUISHABLE PAYLOADS, because counting alone cannot see a
|
||||
// CROSS-SWAP — A answering host-2's event while B answers host-1's
|
||||
// produces exactly the same counts as the correct routing. Each event
|
||||
// carries its own image id, so the surviving viewer's `src` says which
|
||||
// EVENT it came from, and destroying a known host says which HOST held it.
|
||||
notifyViewerOpen(openEvent({ hostToken: 'host-2' }));
|
||||
flushSync();
|
||||
|
||||
// Exactly one viewer may open. Matching on itemId alone would open two.
|
||||
expect(viewers()).toHaveLength(1);
|
||||
expect(viewers(containerB)).toHaveLength(1);
|
||||
expect(viewers(containerA)).toHaveLength(0);
|
||||
|
||||
// ...and the reverse direction, so neither host is simply inert.
|
||||
notifyViewerOpen(openEvent({ hostToken: 'host-1' }));
|
||||
notifyViewerOpen(
|
||||
openEvent({ hostToken: 'host-1', attachmentId: ATT_ID_2, images: [image({ id: ATT_ID_2 })] })
|
||||
);
|
||||
flushSync();
|
||||
expect(viewers(containerA)).toHaveLength(1);
|
||||
expect(viewers()).toHaveLength(2);
|
||||
|
||||
// Destroy host A: the viewer that goes with it must be the one opened by
|
||||
// A's OWN token (the ATT_ID_2 event), leaving B's ATT_ID one behind. A
|
||||
// cross-swap fails here — it would leave the ATT_ID_2 viewer standing.
|
||||
unmount(mounted.splice(mounted.indexOf(hostA), 1)[0]);
|
||||
flushSync();
|
||||
const survivors = viewers();
|
||||
expect(survivors).toHaveLength(1);
|
||||
expect(
|
||||
survivors[0].querySelector('.lightbox-image')?.getAttribute('src')
|
||||
).toContain(ATT_ID);
|
||||
});
|
||||
|
||||
it('ignores an event for a different item on its own token', () => {
|
||||
@@ -380,34 +404,35 @@ describe('AttachmentViewerHost', () => {
|
||||
// runs and its viewer stays on screen. Exercised open → mutate →
|
||||
// teardown → RE-open → mutate, because a hazard that no-ops on its
|
||||
// first write only shows once the state has actually moved.
|
||||
const first = mountHost(propsA);
|
||||
const neighbour = mountHost(propsA);
|
||||
//
|
||||
// Counted across the whole document rather than per container (the
|
||||
// viewer portals to <body> now): both hosts answer the same address, so
|
||||
// a stranded teardown shows up as a count that fails to reach 0.
|
||||
mountHost(propsA);
|
||||
mountHost(propsA);
|
||||
|
||||
notifyViewerOpen(openEvent());
|
||||
flushSync();
|
||||
expect(viewers(first)).toHaveLength(1);
|
||||
expect(viewers(neighbour)).toHaveLength(1);
|
||||
expect(viewers()).toHaveLength(2);
|
||||
|
||||
propsA.resourceGen = 2;
|
||||
flushSync();
|
||||
expect(viewers(first)).toHaveLength(0);
|
||||
expect(viewers(neighbour)).toHaveLength(0);
|
||||
expect(viewers()).toHaveLength(0);
|
||||
|
||||
// Re-open on the same (now current) resource and drive it again.
|
||||
notifyViewerOpen(openEvent());
|
||||
flushSync();
|
||||
expect(viewers(neighbour)).toHaveLength(1);
|
||||
expect(viewers()).toHaveLength(2);
|
||||
|
||||
propsA.itemId = 'item-b';
|
||||
flushSync();
|
||||
expect(viewers(first)).toHaveLength(0);
|
||||
expect(viewers(neighbour)).toHaveLength(0);
|
||||
expect(viewers()).toHaveLength(0);
|
||||
|
||||
// And the neighbour is still LIVE, not merely emptied: it answers the
|
||||
// new address, which it could not do if its effects had been stranded.
|
||||
// And both are still LIVE, not merely emptied: they answer the new
|
||||
// address, which they could not do if their effects had been stranded.
|
||||
notifyViewerOpen(openEvent({ itemId: 'item-b' }));
|
||||
flushSync();
|
||||
expect(viewers(neighbour)).toHaveLength(1);
|
||||
expect(viewers()).toHaveLength(2);
|
||||
});
|
||||
|
||||
// The bound-close invariant is NOT tested here: driving it needs the close
|
||||
|
||||
@@ -129,26 +129,22 @@ describe('AttachmentViewerHost — bound close', () => {
|
||||
expect(stub()).not.toBeNull();
|
||||
});
|
||||
|
||||
it('returns focus to the invoker only on the bound close', () => {
|
||||
it('threads the invoker down and never moves focus itself', () => {
|
||||
// TASK-2429 moved the restore INTO the viewer. It has to happen after the
|
||||
// backdrop lease is released — while the lease is held, the invoker is
|
||||
// inside an `inert` body child and simply will not take focus — and the
|
||||
// only code that runs at that moment is the viewer's own teardown. So the
|
||||
// host's job is reduced to handing the invoker over, and its close handler
|
||||
// must leave `document.activeElement` exactly where it was.
|
||||
const invoker = target.appendChild(document.createElement('button'));
|
||||
const other = target.appendChild(document.createElement('button'));
|
||||
other.focus();
|
||||
|
||||
notifyViewerOpen(openEvent({ invoker }));
|
||||
flushSync();
|
||||
lightboxStubCalls[0].onClose();
|
||||
flushSync();
|
||||
expect(document.activeElement).toBe(invoker);
|
||||
expect(lightboxStubCalls[0].invoker).toBe(invoker);
|
||||
|
||||
// A stale close returns nothing: it did not close anything, so moving
|
||||
// the user's focus would be a jump out of whatever they are now in.
|
||||
notifyViewerOpen(openEvent({ invoker }));
|
||||
flushSync();
|
||||
const staleClose = lightboxStubCalls[1].onClose;
|
||||
notifyViewerOpen(openEvent({ attachmentId: ATT_ID_2, invoker: null }));
|
||||
flushSync();
|
||||
other.focus();
|
||||
staleClose();
|
||||
lightboxStubCalls[0].onClose();
|
||||
flushSync();
|
||||
expect(document.activeElement).toBe(other);
|
||||
});
|
||||
|
||||
@@ -12,15 +12,16 @@
|
||||
images: { id: string }[];
|
||||
index?: number;
|
||||
wsSlug: string;
|
||||
invoker?: HTMLElement | null;
|
||||
onClose: () => void;
|
||||
}
|
||||
|
||||
let { images, index = 0, wsSlug, onClose }: Props = $props();
|
||||
let { images, index = 0, wsSlug, invoker = null, onClose }: Props = $props();
|
||||
|
||||
// Captured ONCE at mount, `untrack`ed like the real component's index seed:
|
||||
// the host remounts per open, so a recorded call belongs to exactly one
|
||||
// viewer instance — which is the whole point of the recording.
|
||||
const call: LightboxStubCall = untrack(() => ({ images, index, wsSlug, onClose }));
|
||||
const call: LightboxStubCall = untrack(() => ({ images, index, wsSlug, invoker, onClose }));
|
||||
lightboxStubCalls.push(call);
|
||||
</script>
|
||||
|
||||
|
||||
@@ -10,6 +10,8 @@ export interface LightboxStubCall {
|
||||
images: { id: string }[];
|
||||
index: number;
|
||||
wsSlug: string;
|
||||
/** Threaded down by the host since TASK-2429; the viewer owns the restore. */
|
||||
invoker: HTMLElement | null;
|
||||
onClose: () => void;
|
||||
}
|
||||
|
||||
|
||||
@@ -350,6 +350,9 @@
|
||||
// menu). Recognised by role/tag/class so pane-owned popups that portal out
|
||||
// to <body> are all covered — the shared `inExemptSurface` set (paneFocus.ts)
|
||||
// is the SAME one the host's focus-follows classifier reuses (PLAN-2179).
|
||||
// The attachment viewer (TASK-2429) is one of those portaled dialogs and is
|
||||
// exempt for the same reason: it runs its own trap, so this one must not
|
||||
// pull focus back out of it.
|
||||
// (Focus already inside `.item-pane` is handled by the `region.contains`
|
||||
// check at each call site.)
|
||||
function onTrapKeydown(e: KeyboardEvent) {
|
||||
|
||||
@@ -6,11 +6,28 @@
|
||||
* (un-variant) blob so the expanded view is full resolution regardless
|
||||
* of the thumbnail variant shown inline.
|
||||
*
|
||||
* Keyboard: Esc closes, ←/→ navigate when multiple images were passed.
|
||||
* Backdrop click closes; clicking the image itself does not.
|
||||
* MODAL CONTRACT (PLAN-2392 phase 3a / TASK-2429, DR-4b). This is a real
|
||||
* modal now: `role="dialog"` + `aria-modal`, portaled to `<body>`, focus
|
||||
* entry + restore, a Tab trap, background inertness through the shared
|
||||
* viewer-backdrop manager, and Escape via the shared `escapeStack`. It is
|
||||
* the contract the editor's hand-rolled `showModal()` dialog was getting
|
||||
* from the platform for free, written out by hand — because 3a's later
|
||||
* tasks delete that dialog and route the inline body images here.
|
||||
*
|
||||
* Keyboard: Esc closes (through `escapeStack`, NOT a local listener),
|
||||
* ←/→ navigate when multiple images were passed, Tab cycles within the
|
||||
* viewer. Backdrop click closes; clicking the image itself does not.
|
||||
*/
|
||||
import { untrack } from 'svelte';
|
||||
import { attachmentDownloadUrl } from '$lib/markdown/attachments';
|
||||
import { paneFocusables, nextTrapTarget } from '$lib/collections/paneFocus';
|
||||
import {
|
||||
acquire,
|
||||
isBlockedByModal,
|
||||
isViewerFrontmost,
|
||||
VIEWER_ROOT_CLASS,
|
||||
} from '$lib/a11y/viewerBackdrop';
|
||||
import { pushEscapeHandler, ESCAPE_PRIORITY } from '$lib/stores/escapeStack';
|
||||
|
||||
export interface LightboxImage {
|
||||
id: string;
|
||||
@@ -23,9 +40,16 @@
|
||||
index?: number;
|
||||
wsSlug: string;
|
||||
onClose: () => void;
|
||||
/**
|
||||
* The control that opened the viewer — focus goes back to it on close.
|
||||
* OPTIONAL: the strip, the timeline and the NodeView thread real values in
|
||||
* TASK-2431 / TASK-2433. Until then it falls back to whatever held focus
|
||||
* at open (see below), which is the same element in every current path.
|
||||
*/
|
||||
invoker?: HTMLElement | null;
|
||||
}
|
||||
|
||||
let { images, index = 0, wsSlug, onClose }: Props = $props();
|
||||
let { images, index = 0, wsSlug, onClose, invoker = null }: Props = $props();
|
||||
|
||||
// Seeded once at mount — the host remounts (null → set) on each open, so
|
||||
// no prop-sync effect is needed. untrack makes the initial-value capture
|
||||
@@ -34,9 +58,39 @@
|
||||
untrack(() => Math.min(Math.max(index, 0), Math.max(images.length - 1, 0)))
|
||||
);
|
||||
|
||||
// CAPTURED AT OPEN, never read live (TASK-2429). The pane switches workspace
|
||||
// without remounting whatever is above it, so a live read could rebuild the
|
||||
// URLs of already-captured attachment ids against a DIFFERENT workspace —
|
||||
// serving a 404, or worse, another workspace's attachment at the same id.
|
||||
// `invoker` is captured for the same reason: the value that opened this
|
||||
// viewer is the one to return focus to.
|
||||
const openWsSlug = untrack(() => wsSlug);
|
||||
// The invoker falls back to WHATEVER HELD FOCUS AT OPEN (the pattern
|
||||
// `BottomSheet` uses), not to nothing. Focus entry is about to move focus
|
||||
// into the viewer, so without this the producers that don't thread an
|
||||
// invoker yet — the strip and the timeline, until TASK-2431 — would be
|
||||
// strictly worse off than before this component managed focus at all: they
|
||||
// keep focus on the clicked tile today, and a null invoker would drop it to
|
||||
// `<body>` on close. Captured at init, BEFORE the entry focus runs.
|
||||
const openInvoker = untrack(() => {
|
||||
if (invoker) return invoker;
|
||||
if (typeof document === 'undefined') return null;
|
||||
const active = document.activeElement;
|
||||
return active && active !== document.body ? (active as HTMLElement) : null;
|
||||
});
|
||||
|
||||
let hasMultiple = $derived(images.length > 1);
|
||||
let img = $derived(images[current]);
|
||||
let src = $derived(img ? attachmentDownloadUrl(wsSlug, img.id) : '');
|
||||
let src = $derived(img ? attachmentDownloadUrl(openWsSlug, img.id) : '');
|
||||
// The accessible name: the image's own alt where there is one, else a
|
||||
// generic label. Never empty — an unnamed `role="dialog"` is announced as
|
||||
// nothing at all.
|
||||
let dialogLabel = $derived(img?.alt || 'Attachment viewer');
|
||||
|
||||
// The portaled root. `$state` so the effect below re-runs once `bind:this`
|
||||
// lands; read-only inside every effect, so nothing here can self-invalidate
|
||||
// a flush (CONVE-1688).
|
||||
let rootEl = $state<HTMLElement | null>(null);
|
||||
|
||||
function prev() {
|
||||
current = (current - 1 + images.length) % images.length;
|
||||
@@ -45,14 +99,144 @@
|
||||
current = (current + 1) % images.length;
|
||||
}
|
||||
|
||||
function onKeydown(e: KeyboardEvent) {
|
||||
if (e.key === 'Escape') {
|
||||
/**
|
||||
* Return focus where it came from, on close.
|
||||
*
|
||||
* Declines when focus has ALREADY moved somewhere outside this viewer —
|
||||
* a surface opened over the viewer owns focus outright, and so does a
|
||||
* producer that moves focus from its own close handler (which runs BEFORE
|
||||
* this teardown). `AttachmentViewerHost` used to be exactly that producer;
|
||||
* TASK-2429 moved the restore here precisely because its version ran while
|
||||
* the invoker was still inert, so this guard is now about the surfaces the
|
||||
* viewer does not control rather than about that host. Focus resting on
|
||||
* `<body>` / nowhere is the adrift state a teardown leaves behind, and
|
||||
* counts as ours to move.
|
||||
*
|
||||
* The invoker is verified rather than trusted: it can have been detached
|
||||
* (an editor NodeView is re-rendered on any document change), hidden, or
|
||||
* inerted since it was captured, and `focus()` on such an element silently
|
||||
* does nothing on some engines and drops focus to `<body>` on others. So we
|
||||
* focus it and CHECK, falling back to parking focus on `<body>` — which is
|
||||
* where the browser would have put it anyway, but deterministically, and
|
||||
* without leaving focus inside a subtree that is about to be removed.
|
||||
*/
|
||||
function restoreFocus(root: HTMLElement): void {
|
||||
const active = document.activeElement;
|
||||
if (active !== null && active !== document.body && !root.contains(active)) return;
|
||||
|
||||
if (openInvoker?.isConnected) {
|
||||
openInvoker.focus?.({ preventScroll: true });
|
||||
if (document.activeElement === openInvoker) return;
|
||||
}
|
||||
(document.activeElement as HTMLElement | null)?.blur?.();
|
||||
}
|
||||
|
||||
// ONE effect owns the whole modal contract, because the steps are ordered
|
||||
// with respect to each other and to teardown: portal → lease → focus entry →
|
||||
// Escape registration, unwound in reverse.
|
||||
$effect(() => {
|
||||
const el = rootEl;
|
||||
if (!el) return;
|
||||
|
||||
// PORTAL TO <body> DIRECTLY — deliberately NOT `portalAction.ts`, which
|
||||
// targets the nearest ancestor `<dialog>` when there is one: exactly the
|
||||
// wrong target for a surface that must sit ABOVE everything. A `position:
|
||||
// fixed` overlay is only viewport-fixed while no ancestor establishes a
|
||||
// containing block, and `transform` / `filter` / `contain: layout` on any
|
||||
// ancestor silently does (see the container-query foot-gun). `<body>` is
|
||||
// the only parent with no such ancestor, and it is what the backdrop
|
||||
// manager's inert bookkeeping requires: it writes `inert` on BODY CHILDREN
|
||||
// and exempts this one.
|
||||
document.body.appendChild(el);
|
||||
|
||||
// Background inertness is the manager's, not ours — no hand-rolled
|
||||
// `inert`, no `aria-hidden` sweep. It refcounts, so two viewers stacked
|
||||
// (the strip's and a NodeView's) can't clobber each other's writes.
|
||||
const lease = acquire(el);
|
||||
|
||||
// Focus ENTRY goes to the first tabbable DESCENDANT (the close button),
|
||||
// not the root: a screen-reader user landing on the container has to
|
||||
// discover the controls, and the trap below cycles from wherever focus is.
|
||||
// The root is `tabindex="-1"` purely as the fallback for a viewer with no
|
||||
// tabbable control yet (single image, still loading) — focus must not stay
|
||||
// on whatever is behind the backdrop.
|
||||
const entry = paneFocusables(el)[0] ?? el;
|
||||
entry.focus({ preventScroll: true });
|
||||
|
||||
// Escape has ONE owner: the shared stack. The local `<svelte:window>`
|
||||
// Escape branch this component used to carry was DELETED rather than
|
||||
// gated — it ignored `defaultPrevented`, so with the stack also running,
|
||||
// a single press closed the viewer AND the layer beneath it.
|
||||
//
|
||||
// Registered ABOVE `menu` (40) so the viewer is the innermost layer.
|
||||
// Declines (returns false) unless this viewer is the FRONTMOST lease, so
|
||||
// with two viewers open one press closes exactly the top one and the
|
||||
// stack falls through to it rather than to an unrelated layer.
|
||||
const unregisterEscape = pushEscapeHandler(() => {
|
||||
if (!isViewerFrontmost(el)) return false;
|
||||
onClose();
|
||||
} else if (e.key === 'ArrowLeft' && hasMultiple) {
|
||||
return true;
|
||||
}, ESCAPE_PRIORITY.viewer);
|
||||
|
||||
return () => {
|
||||
unregisterEscape();
|
||||
// Release BEFORE restoring focus, and let the RESULT decide: when a
|
||||
// viewer remains beneath this one the manager has already handed focus
|
||||
// into it, and restoring our own invoker would yank focus out of a
|
||||
// viewer the user is still looking at. Only the last one out restores.
|
||||
const { stackEmpty } = lease.release();
|
||||
if (stackEmpty) restoreFocus(el);
|
||||
// Svelte removes the node itself, but it is reparented out of its
|
||||
// anchor — remove it explicitly so the DOM can never be left with a
|
||||
// stranded backdrop, and so the manager's next reconcile sees the
|
||||
// body child list it expects.
|
||||
el.remove();
|
||||
};
|
||||
});
|
||||
|
||||
function onKeydown(e: KeyboardEvent) {
|
||||
// A control that already handled this key owns it.
|
||||
if (e.defaultPrevented) return;
|
||||
const el = rootEl;
|
||||
// Every mounted viewer listens on `window`, so ONLY the frontmost may act.
|
||||
// This matters more than the usual layer-isolation argument: `nextTrapTarget`
|
||||
// deliberately pulls out-of-container focus back INWARD, so a background
|
||||
// viewer running the trap would drag focus into itself, out of the viewer
|
||||
// in front of it.
|
||||
if (!el || !isViewerFrontmost(el)) return;
|
||||
// ...and a `showModal()` dialog opened OVER the viewer owns the top layer,
|
||||
// above any body-portaled surface, so the frontmost LEASE is not
|
||||
// necessarily the frontmost SURFACE. Without this the viewer's trap would
|
||||
// fight a native modal for Tab and pull focus back out of it — the exact
|
||||
// inward-redirect hazard the frontmost gate exists to prevent, one layer
|
||||
// up. The manager already keeps such a dialog out of the inert set
|
||||
// (`keepInteractiveAsDialog`), so it is reachable and this is real.
|
||||
if (isBlockedByModal(el)) return;
|
||||
|
||||
if (e.key === 'Tab') {
|
||||
// Reuses the pane's tested trap math (paneFocus.ts) — one trap
|
||||
// implementation for the whole app, not a second one here.
|
||||
const target = nextTrapTarget(
|
||||
paneFocusables(el),
|
||||
document.activeElement,
|
||||
e.shiftKey,
|
||||
el
|
||||
);
|
||||
if (target) {
|
||||
e.preventDefault();
|
||||
target.focus({ preventScroll: true });
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
if (e.key === 'ArrowLeft' && hasMultiple) {
|
||||
e.preventDefault();
|
||||
prev();
|
||||
} else if (e.key === 'ArrowRight' && hasMultiple) {
|
||||
e.preventDefault();
|
||||
next();
|
||||
}
|
||||
// NO Escape branch. See the registration above.
|
||||
}
|
||||
|
||||
// Close only on a click of the backdrop itself — clicks on the image or
|
||||
@@ -65,20 +249,55 @@
|
||||
|
||||
<svelte:window onkeydown={onKeydown} />
|
||||
|
||||
<!--
|
||||
The backdrop click has no keyboard twin ON THIS ELEMENT by design: the
|
||||
keyboard route out is Escape (through the shared stack) and the close
|
||||
button, which is the first thing focus lands on. Adding a key handler here
|
||||
would put a second, undocumented dismissal on the container.
|
||||
-->
|
||||
<!-- svelte-ignore a11y_click_events_have_key_events -->
|
||||
<div
|
||||
class="lightbox-backdrop"
|
||||
role="presentation"
|
||||
bind:this={rootEl}
|
||||
class="lightbox-backdrop {VIEWER_ROOT_CLASS}"
|
||||
role="dialog"
|
||||
aria-modal="true"
|
||||
aria-label={dialogLabel}
|
||||
tabindex="-1"
|
||||
onclick={onBackdropClick}
|
||||
>
|
||||
<button class="lightbox-close" type="button" title="Close (Esc)" onclick={onClose}>
|
||||
<!--
|
||||
Explicit `aria-label`s: the glyph IS the label otherwise ("✕", "‹", "›"),
|
||||
and `title` does not win over element content for the accessible name. The
|
||||
browser suite (TASK-2436) also addresses these controls BY NAME, never by
|
||||
class or a bare `[role="dialog"]`.
|
||||
-->
|
||||
<button
|
||||
class="lightbox-close"
|
||||
type="button"
|
||||
title="Close (Esc)"
|
||||
aria-label="Close"
|
||||
onclick={onClose}
|
||||
>
|
||||
✕
|
||||
</button>
|
||||
|
||||
{#if hasMultiple}
|
||||
<button class="lightbox-nav prev" type="button" title="Previous (←)" onclick={prev}>
|
||||
<button
|
||||
class="lightbox-nav prev"
|
||||
type="button"
|
||||
title="Previous (←)"
|
||||
aria-label="Previous image"
|
||||
onclick={prev}
|
||||
>
|
||||
‹
|
||||
</button>
|
||||
<button class="lightbox-nav next" type="button" title="Next (→)" onclick={next}>
|
||||
<button
|
||||
class="lightbox-nav next"
|
||||
type="button"
|
||||
title="Next (→)"
|
||||
aria-label="Next image"
|
||||
onclick={next}
|
||||
>
|
||||
›
|
||||
</button>
|
||||
{/if}
|
||||
@@ -96,7 +315,39 @@
|
||||
.lightbox-backdrop {
|
||||
position: fixed;
|
||||
inset: 0;
|
||||
z-index: 1000;
|
||||
/*
|
||||
* ABOVE EVERY OTHER OVERLAY IN THE APP (TASK-2429). The viewer is a modal:
|
||||
* it inerts every other body child, so anything that PAINTS over it is a
|
||||
* surface the user can see but cannot touch — the worst of both. The app
|
||||
* shell wrapper is `display: contents` (`app.html`), which creates no
|
||||
* stacking context, so every fixed overlay in the tree competes with this
|
||||
* one in the ROOT stacking context — being a body child is not by itself
|
||||
* protection.
|
||||
*
|
||||
* What this value must out-rank, highest first (swept for TASK-2429 — CSS
|
||||
* declarations AND `style.cssText` written from TypeScript, which a CSS-only
|
||||
* grep misses):
|
||||
* 99999 EmojiPickerButton's desktop dropdown (`EmojiPickerButton.svelte`,
|
||||
* body-portaled via `portalAction`) — the highest in the tree by
|
||||
* two orders of magnitude, and the reason 1000 was not enough
|
||||
* 1000 the editor's block-drag ghost, appended to `<body>` and styled
|
||||
* inline (`editor/block-drag-handle.ts::createGhost`)
|
||||
* 200 Menu (body-portaled) and the editor's slash/link popovers
|
||||
* 199 the editor's block context menu
|
||||
* 100 toasts, the notification panel, the editor's mobile toolbar
|
||||
* 45-60 DockedSheet / BottomSheet / Sidebar / CommandPalette / TopBar
|
||||
* (`ItemDetail`'s 1000 is a `@media print` footer, not a runtime overlay.)
|
||||
*
|
||||
* A NEW OVERLAY ABOVE THIS VALUE IS A BUG unless it is meant to cover a
|
||||
* modal viewer. One thing legitimately renders above it and needs no
|
||||
* z-index to do so: a native `showModal()` dialog, which the platform puts
|
||||
* in the TOP LAYER — the manager deliberately leaves those interactive
|
||||
* (`viewerBackdrop.ts::keepInteractiveAsDialog`) and the key handler stands
|
||||
* down for them (`isBlockedByModal`).
|
||||
*
|
||||
* jsdom cannot see any of this; the paint-order proof is TASK-2436's.
|
||||
*/
|
||||
z-index: 100000;
|
||||
display: flex;
|
||||
align-items: center;
|
||||
justify-content: center;
|
||||
|
||||
@@ -0,0 +1,753 @@
|
||||
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
|
||||
import { flushSync, mount, unmount } from 'svelte';
|
||||
import Lightbox from './Lightbox.svelte';
|
||||
import {
|
||||
acquire,
|
||||
hasForeignEscapeOwner,
|
||||
__resetViewerBackdropForTests,
|
||||
VIEWER_ROOT_CLASS,
|
||||
} from '$lib/a11y/viewerBackdrop';
|
||||
import {
|
||||
pushEscapeHandler,
|
||||
runTopEscape,
|
||||
ESCAPE_PRIORITY,
|
||||
_resetEscapeStackForTests,
|
||||
} from '$lib/stores/escapeStack';
|
||||
|
||||
// TASK-2429 — the DR-4b modal contract on the attachment viewer.
|
||||
//
|
||||
// WHAT JSDOM CANNOT PROVE, and is therefore TASK-2436's browser suite (DR-9):
|
||||
//
|
||||
// • REAL INERTNESS. jsdom parses the `inert` attribute but does not implement
|
||||
// its semantics: a control inside an inert subtree is still clickable and
|
||||
// focusable here. So the tests below assert that the manager WAS ASKED (the
|
||||
// attribute lands on the right body children and is removed again), never
|
||||
// that the background is genuinely unreachable.
|
||||
// • LAYOUT AND STACKING. There is no layout engine, so "fixed, covering the
|
||||
// viewport, above everything" is unassertable — including the one that
|
||||
// actually bites: an ancestor with `transform` / `filter` / `contain` making
|
||||
// a `position: fixed` overlay scroll with the page. What IS assertable is
|
||||
// the structural precondition, and it is asserted: the root is a DIRECT
|
||||
// child of `<body>`, so no ancestor can establish a containing block.
|
||||
// • REAL TAB TRAVERSAL. jsdom does not move focus on a Tab keydown at all, so
|
||||
// the trap is exercised through the handler's own decision (it preventDefaults
|
||||
// and focuses explicitly) rather than through browser behaviour.
|
||||
// • VISIBILITY. `offsetParent` / `getClientRects` report everything hidden, so
|
||||
// `paneFocusables` would return an empty set for every element. The stub
|
||||
// below (the shape `viewerBackdrop.svelte.test.ts` uses) makes the real
|
||||
// selection path run instead of always seeing nothing.
|
||||
|
||||
const realGetClientRects = HTMLElement.prototype.getClientRects;
|
||||
|
||||
const IMG_A = 'aaaaaaaa-1111-4111-8111-aaaaaaaaaaaa';
|
||||
const IMG_B = 'bbbbbbbb-2222-4222-8222-bbbbbbbbbbbb';
|
||||
|
||||
interface Props {
|
||||
images: { id: string; alt: string }[];
|
||||
index?: number;
|
||||
wsSlug: string;
|
||||
onClose: () => void;
|
||||
invoker?: HTMLElement | null;
|
||||
}
|
||||
|
||||
// Reactive props for the capture-at-open cases ($state may only initialize a
|
||||
// declaration, hence top level).
|
||||
const liveProps = $state<Props>({
|
||||
images: [{ id: IMG_A, alt: 'a diagram' }],
|
||||
index: 0,
|
||||
wsSlug: 'ws-one',
|
||||
onClose: () => {},
|
||||
});
|
||||
|
||||
/** The app shell's stand-in: a body child, so the manager has something to inert. */
|
||||
let appRoot: HTMLElement;
|
||||
const mounted: ReturnType<typeof mount>[] = [];
|
||||
|
||||
function mountViewer(props: Partial<Props> = {}): ReturnType<typeof mount> {
|
||||
const app = mount(Lightbox, {
|
||||
target: appRoot,
|
||||
props: {
|
||||
images: [{ id: IMG_A, alt: 'a diagram' }],
|
||||
index: 0,
|
||||
wsSlug: 'ws-one',
|
||||
onClose: () => {},
|
||||
...props,
|
||||
},
|
||||
});
|
||||
mounted.push(app);
|
||||
flushSync();
|
||||
return app;
|
||||
}
|
||||
|
||||
function roots(): HTMLElement[] {
|
||||
return Array.from(document.body.querySelectorAll<HTMLElement>('.lightbox-backdrop'));
|
||||
}
|
||||
|
||||
function root(): HTMLElement {
|
||||
const found = roots();
|
||||
if (found.length === 0) throw new Error('no viewer mounted');
|
||||
return found[found.length - 1];
|
||||
}
|
||||
|
||||
function imageSrc(scope: HTMLElement = root()): string {
|
||||
return scope.querySelector<HTMLImageElement>('.lightbox-image')?.getAttribute('src') ?? '';
|
||||
}
|
||||
|
||||
function closeButton(scope: HTMLElement = root()): HTMLButtonElement {
|
||||
return scope.querySelector<HTMLButtonElement>('.lightbox-close')!;
|
||||
}
|
||||
|
||||
/** A cancelable window keydown, returning whether the app consumed it. */
|
||||
function press(key: string, init: KeyboardEventInit = {}): boolean {
|
||||
const event = new KeyboardEvent('keydown', { key, cancelable: true, bubbles: true, ...init });
|
||||
window.dispatchEvent(event);
|
||||
flushSync();
|
||||
return event.defaultPrevented;
|
||||
}
|
||||
|
||||
/** Body children currently carrying `inert` (see the jsdom caveat above). */
|
||||
function inertBodyChildren(): Element[] {
|
||||
return Array.from(document.body.children).filter((el) => el.hasAttribute('inert'));
|
||||
}
|
||||
|
||||
beforeEach(() => {
|
||||
HTMLElement.prototype.getClientRects = function () {
|
||||
return [{}] as unknown as DOMRectList;
|
||||
};
|
||||
Object.assign(liveProps, {
|
||||
images: [{ id: IMG_A, alt: 'a diagram' }],
|
||||
index: 0,
|
||||
wsSlug: 'ws-one',
|
||||
onClose: () => {},
|
||||
});
|
||||
appRoot = document.body.appendChild(document.createElement('div'));
|
||||
appRoot.id = 'app';
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
while (mounted.length) unmount(mounted.pop()!);
|
||||
document.body.innerHTML = '';
|
||||
__resetViewerBackdropForTests();
|
||||
_resetEscapeStackForTests();
|
||||
HTMLElement.prototype.getClientRects = realGetClientRects;
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
describe('Lightbox — dialog semantics', () => {
|
||||
it('is an aria-modal dialog named after the image', () => {
|
||||
mountViewer();
|
||||
expect(root().getAttribute('role')).toBe('dialog');
|
||||
expect(root().getAttribute('aria-modal')).toBe('true');
|
||||
expect(root().getAttribute('aria-label')).toBe('a diagram');
|
||||
});
|
||||
|
||||
it('falls back to a generic name when the image has no alt', () => {
|
||||
// An unnamed dialog is announced as nothing at all, so the fallback is
|
||||
// part of the contract rather than a nicety.
|
||||
mountViewer({ images: [{ id: IMG_A, alt: '' }] });
|
||||
expect(root().getAttribute('aria-label')).toBe('Attachment viewer');
|
||||
});
|
||||
|
||||
it('gives every control a real accessible name, not a glyph', () => {
|
||||
// The button text is "✕" / "‹" / "›", and `title` does not win over
|
||||
// element content for the accessible name — so without these the controls
|
||||
// are announced as punctuation, and TASK-2436's browser suite (which
|
||||
// addresses surfaces BY NAME) would have nothing to target.
|
||||
mountViewer({
|
||||
images: [
|
||||
{ id: IMG_A, alt: 'first' },
|
||||
{ id: IMG_B, alt: 'second' },
|
||||
],
|
||||
});
|
||||
expect(closeButton().getAttribute('aria-label')).toBe('Close');
|
||||
expect(
|
||||
root().querySelector('.lightbox-nav.prev')?.getAttribute('aria-label')
|
||||
).toBe('Previous image');
|
||||
expect(
|
||||
root().querySelector('.lightbox-nav.next')?.getAttribute('aria-label')
|
||||
).toBe('Next image');
|
||||
});
|
||||
|
||||
it('names itself after the image CURRENTLY shown, not the one it opened on', () => {
|
||||
mountViewer({
|
||||
images: [
|
||||
{ id: IMG_A, alt: 'first' },
|
||||
{ id: IMG_B, alt: 'second' },
|
||||
],
|
||||
});
|
||||
expect(root().getAttribute('aria-label')).toBe('first');
|
||||
root().querySelector<HTMLButtonElement>('.lightbox-nav.next')!.click();
|
||||
flushSync();
|
||||
expect(root().getAttribute('aria-label')).toBe('second');
|
||||
});
|
||||
});
|
||||
|
||||
describe('Lightbox — portal', () => {
|
||||
it('portals to <body> DIRECTLY, not into its mount container', () => {
|
||||
// The structural half of the fixed-overlay contract: with `<body>` as the
|
||||
// parent there is no ancestor left to establish a containing block with
|
||||
// `transform` / `filter` / `contain`, which is the failure mode that
|
||||
// silently traps a `position: fixed` overlay. The geometric half needs a
|
||||
// layout engine and belongs to TASK-2436.
|
||||
mountViewer();
|
||||
expect(root().parentElement).toBe(document.body);
|
||||
expect(appRoot.contains(root())).toBe(false);
|
||||
});
|
||||
|
||||
it('carries the viewer-root class the app-wide guards key off', () => {
|
||||
// `hasForeignEscapeOwner` excludes this class so the route ESC guards
|
||||
// look PAST the viewer to the escape stack. A rename that touched only
|
||||
// the markup would make Escape dead app-wide, hence the shared constant.
|
||||
mountViewer();
|
||||
expect(root().classList.contains(VIEWER_ROOT_CLASS)).toBe(true);
|
||||
});
|
||||
|
||||
it('takes the portaled root back out of <body> on close', () => {
|
||||
const app = mountViewer();
|
||||
expect(roots()).toHaveLength(1);
|
||||
unmount(mounted.splice(mounted.indexOf(app), 1)[0]);
|
||||
flushSync();
|
||||
expect(roots()).toHaveLength(0);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Lightbox — workspace captured at open', () => {
|
||||
it('keeps serving the open-time workspace after the prop changes', () => {
|
||||
mounted.push(mount(Lightbox, { target: appRoot, props: liveProps }));
|
||||
flushSync();
|
||||
expect(imageSrc()).toContain('/workspaces/ws-one/');
|
||||
|
||||
// The pane switches workspace WITHOUT remounting what is above it, so a
|
||||
// live read would rebuild already-captured attachment ids against the new
|
||||
// workspace — a 404 at best, another workspace's blob at worst.
|
||||
liveProps.wsSlug = 'ws-two';
|
||||
flushSync();
|
||||
expect(imageSrc()).toContain('/workspaces/ws-one/');
|
||||
expect(imageSrc()).not.toContain('ws-two');
|
||||
});
|
||||
|
||||
it('still rebuilds the src when the SHOWN IMAGE changes', () => {
|
||||
// The guard above would also pass against a src that never updates at
|
||||
// all, which would be a different bug. This is the counterweight.
|
||||
mountViewer({
|
||||
images: [
|
||||
{ id: IMG_A, alt: 'first' },
|
||||
{ id: IMG_B, alt: 'second' },
|
||||
],
|
||||
});
|
||||
expect(imageSrc()).toContain(IMG_A);
|
||||
expect(press('ArrowRight')).toBe(true);
|
||||
expect(imageSrc()).toContain(IMG_B);
|
||||
expect(imageSrc()).toContain('/workspaces/ws-one/');
|
||||
});
|
||||
});
|
||||
|
||||
describe('Lightbox — focus', () => {
|
||||
it('moves focus to the FIRST tabbable descendant, not the root or any other', () => {
|
||||
// Multi-image on purpose: with three controls (close, prev, next) this
|
||||
// separates "the first tabbable" from "a tabbable" and from "the root".
|
||||
// A single-control fixture would pass for an implementation that took the
|
||||
// LAST candidate.
|
||||
mountViewer({
|
||||
images: [
|
||||
{ id: IMG_A, alt: 'first' },
|
||||
{ id: IMG_B, alt: 'second' },
|
||||
],
|
||||
});
|
||||
const controls = Array.from(root().querySelectorAll('button'));
|
||||
expect(controls).toHaveLength(3);
|
||||
expect(document.activeElement).toBe(controls[0]);
|
||||
expect(document.activeElement).toBe(closeButton());
|
||||
expect(document.activeElement).not.toBe(root());
|
||||
});
|
||||
|
||||
it('returns focus to the invoker on close', () => {
|
||||
const invoker = appRoot.appendChild(document.createElement('button'));
|
||||
const app = mountViewer({ invoker });
|
||||
expect(document.activeElement).toBe(closeButton());
|
||||
|
||||
unmount(mounted.splice(mounted.indexOf(app), 1)[0]);
|
||||
flushSync();
|
||||
expect(document.activeElement).toBe(invoker);
|
||||
});
|
||||
|
||||
it('falls back to <body> when the invoker was detached while the viewer was up', () => {
|
||||
// An editor NodeView is re-rendered on any document change, so the
|
||||
// element that opened the viewer is routinely gone by close time.
|
||||
const invoker = appRoot.appendChild(document.createElement('button'));
|
||||
const app = mountViewer({ invoker });
|
||||
invoker.remove();
|
||||
// `activeElement` alone can't see the `isConnected` check: focusing a
|
||||
// DETACHED element is a no-op in jsdom, so focus would land on <body>
|
||||
// either way. Asserting the call never happens is what pins the check —
|
||||
// and on a real engine an unguarded focus() on a detached node moves
|
||||
// focus to <body> on some engines and nowhere on others.
|
||||
const attempted = vi.spyOn(invoker, 'focus');
|
||||
|
||||
unmount(mounted.splice(mounted.indexOf(app), 1)[0]);
|
||||
flushSync();
|
||||
expect(attempted).not.toHaveBeenCalled();
|
||||
expect(document.activeElement).toBe(document.body);
|
||||
});
|
||||
|
||||
it('falls back to <body> when the invoker is connected but not focusable', () => {
|
||||
// `isConnected` alone is not enough: deletion can leave the node in the
|
||||
// tree but unfocusable (hidden, inerted, or never focusable to begin
|
||||
// with). The restore focuses it and VERIFIES, rather than trusting.
|
||||
//
|
||||
// `activeElement` ALONE cannot see the difference here, and a test that
|
||||
// stopped at it would be vacuous: focus sitting inside the root the
|
||||
// teardown then removes ends up on `<body>` either way. What separates a
|
||||
// verified restore from a trusting one is that the verified path parks
|
||||
// focus DELIBERATELY instead of relying on node-removal fallout — so the
|
||||
// blur is asserted too. (Whether a real engine refuses focus on an inert
|
||||
// or hidden invoker is TASK-2436's; jsdom's `focus()` no-ops on a
|
||||
// non-focusable element, which is the same shape.)
|
||||
const blurred = vi.spyOn(HTMLElement.prototype, 'blur');
|
||||
const invoker = appRoot.appendChild(document.createElement('div'));
|
||||
const app = mountViewer({ invoker });
|
||||
|
||||
unmount(mounted.splice(mounted.indexOf(app), 1)[0]);
|
||||
flushSync();
|
||||
expect(document.activeElement).toBe(document.body);
|
||||
expect(blurred).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('falls back to whatever held focus at open when no invoker is threaded', () => {
|
||||
// The strip and the timeline don't pass an invoker until TASK-2431, and
|
||||
// they keep focus on the clicked tile today. Focus entry is about to move
|
||||
// focus INTO the viewer, so without this capture those two producers would
|
||||
// come out of this commit strictly worse than before it.
|
||||
const tile = appRoot.appendChild(document.createElement('button'));
|
||||
tile.focus();
|
||||
|
||||
const app = mountViewer();
|
||||
expect(document.activeElement).toBe(closeButton());
|
||||
|
||||
unmount(mounted.splice(mounted.indexOf(app), 1)[0]);
|
||||
flushSync();
|
||||
expect(document.activeElement).toBe(tile);
|
||||
});
|
||||
|
||||
it('falls back to <body> when nothing held focus at open either', () => {
|
||||
const app = mountViewer();
|
||||
unmount(mounted.splice(mounted.indexOf(app), 1)[0]);
|
||||
flushSync();
|
||||
expect(document.activeElement).toBe(document.body);
|
||||
});
|
||||
|
||||
it('prefers an explicit invoker over the element that held focus', () => {
|
||||
const tile = appRoot.appendChild(document.createElement('button'));
|
||||
const invoker = appRoot.appendChild(document.createElement('button'));
|
||||
tile.focus();
|
||||
|
||||
const app = mountViewer({ invoker });
|
||||
unmount(mounted.splice(mounted.indexOf(app), 1)[0]);
|
||||
flushSync();
|
||||
expect(document.activeElement).toBe(invoker);
|
||||
});
|
||||
|
||||
it('does NOT yank focus back when something else already owns it', () => {
|
||||
// A producer that moves focus from its own close handler runs BEFORE this
|
||||
// teardown, and a surface opened over the viewer owns focus outright.
|
||||
// Either way the restore must decline rather than move focus a second
|
||||
// time. (`AttachmentViewerHost` was the first case until TASK-2429 moved
|
||||
// the restore here; the guard still covers every other owner.)
|
||||
const invoker = appRoot.appendChild(document.createElement('button'));
|
||||
const elsewhere = appRoot.appendChild(document.createElement('button'));
|
||||
const app = mountViewer({ invoker });
|
||||
|
||||
elsewhere.focus();
|
||||
unmount(mounted.splice(mounted.indexOf(app), 1)[0]);
|
||||
flushSync();
|
||||
expect(document.activeElement).toBe(elsewhere);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Lightbox — Tab trap', () => {
|
||||
it('wraps forward off the last focusable', () => {
|
||||
mountViewer({
|
||||
images: [
|
||||
{ id: IMG_A, alt: 'first' },
|
||||
{ id: IMG_B, alt: 'second' },
|
||||
],
|
||||
});
|
||||
const next = root().querySelector<HTMLButtonElement>('.lightbox-nav.next')!;
|
||||
next.focus();
|
||||
|
||||
expect(press('Tab')).toBe(true);
|
||||
expect(document.activeElement).toBe(closeButton());
|
||||
});
|
||||
|
||||
it('wraps backward off the first focusable', () => {
|
||||
mountViewer({
|
||||
images: [
|
||||
{ id: IMG_A, alt: 'first' },
|
||||
{ id: IMG_B, alt: 'second' },
|
||||
],
|
||||
});
|
||||
closeButton().focus();
|
||||
|
||||
expect(press('Tab', { shiftKey: true })).toBe(true);
|
||||
expect(document.activeElement).toBe(
|
||||
root().querySelector<HTMLButtonElement>('.lightbox-nav.next')
|
||||
);
|
||||
});
|
||||
|
||||
it('pulls focus back to the leading edge when it has escaped the viewer', () => {
|
||||
// The exact target matters: `nextTrapTarget` returns the FIRST focusable
|
||||
// on a forward Tab from outside. Asserting only "somewhere inside" would
|
||||
// pass for an implementation that focused the root instead.
|
||||
const outside = appRoot.appendChild(document.createElement('button'));
|
||||
mountViewer({
|
||||
images: [
|
||||
{ id: IMG_A, alt: 'first' },
|
||||
{ id: IMG_B, alt: 'second' },
|
||||
],
|
||||
});
|
||||
outside.focus();
|
||||
|
||||
expect(press('Tab')).toBe(true);
|
||||
expect(document.activeElement).toBe(closeButton());
|
||||
|
||||
// ...and the trailing edge on a back Tab from outside.
|
||||
outside.focus();
|
||||
expect(press('Tab', { shiftKey: true })).toBe(true);
|
||||
expect(document.activeElement).toBe(
|
||||
root().querySelector('.lightbox-nav.next')
|
||||
);
|
||||
});
|
||||
|
||||
it('leaves a mid-cycle Tab to the browser', () => {
|
||||
// Only the wrap is the trap's business; preventing every Tab would break
|
||||
// the natural order inside the viewer.
|
||||
mountViewer({
|
||||
images: [
|
||||
{ id: IMG_A, alt: 'first' },
|
||||
{ id: IMG_B, alt: 'second' },
|
||||
],
|
||||
});
|
||||
closeButton().focus();
|
||||
expect(press('Tab')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Lightbox — Escape ownership', () => {
|
||||
it('does NOT close on a raw window keydown: the stack is the sole owner', () => {
|
||||
// The local Escape branch was DELETED, not gated. It ignored
|
||||
// `defaultPrevented`, so keeping it alongside the stack gave Escape two
|
||||
// owners and let one press collapse two layers.
|
||||
const onClose = vi.fn();
|
||||
mountViewer({ onClose });
|
||||
|
||||
expect(press('Escape')).toBe(false);
|
||||
expect(onClose).not.toHaveBeenCalled();
|
||||
|
||||
expect(runTopEscape()).toBe(true);
|
||||
expect(onClose).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('outranks the menu layer', () => {
|
||||
// A menu is inert behind the viewer, so the viewer must win the key.
|
||||
//
|
||||
// The menu handler is registered AFTER the viewer deliberately:
|
||||
// `escapeStack` breaks EQUAL-priority ties toward the most recently
|
||||
// registered handler, so registering it first would let the viewer win on
|
||||
// the tie-break alone and the test would pass even at `menu` priority.
|
||||
// Registered last, only a strictly higher priority can win.
|
||||
const onClose = vi.fn();
|
||||
mountViewer({ onClose });
|
||||
const menuClose = vi.fn(() => true);
|
||||
pushEscapeHandler(menuClose, ESCAPE_PRIORITY.menu);
|
||||
|
||||
expect(runTopEscape()).toBe(true);
|
||||
expect(onClose).toHaveBeenCalledTimes(1);
|
||||
expect(menuClose).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('declines Escape once it is no longer the frontmost lease', () => {
|
||||
// The gate that the two-viewer case cannot isolate: with two viewers,
|
||||
// registration order and lease order agree, so `escapeStack`'s
|
||||
// newest-wins tie-break would pick the front one even with the gate
|
||||
// deleted. Taking a lease DIRECTLY puts something above the viewer whose
|
||||
// escape handler is NOT on the stack at all, so lease order and
|
||||
// registration order finally disagree — the viewer must decline, and with
|
||||
// nothing else registered the whole stack must decline with it.
|
||||
const onClose = vi.fn();
|
||||
mountViewer({ onClose });
|
||||
const above = document.body.appendChild(document.createElement('div'));
|
||||
const lease = acquire(above);
|
||||
|
||||
expect(runTopEscape()).toBe(false);
|
||||
expect(onClose).not.toHaveBeenCalled();
|
||||
|
||||
// ...and it takes the key again the moment it is frontmost once more.
|
||||
lease.release();
|
||||
expect(runTopEscape()).toBe(true);
|
||||
expect(onClose).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('ignores a key another control already handled', () => {
|
||||
// The `defaultPrevented` early return. Without it the viewer would page
|
||||
// on an arrow a control underneath (or a layer above) has already
|
||||
// consumed — the exact two-owners-one-press shape the deleted local
|
||||
// Escape branch had.
|
||||
mountViewer({
|
||||
images: [
|
||||
{ id: IMG_A, alt: 'first' },
|
||||
{ id: IMG_B, alt: 'second' },
|
||||
],
|
||||
});
|
||||
expect(imageSrc()).toContain(IMG_A);
|
||||
|
||||
const event = new KeyboardEvent('keydown', {
|
||||
key: 'ArrowRight',
|
||||
cancelable: true,
|
||||
bubbles: true,
|
||||
});
|
||||
event.preventDefault();
|
||||
window.dispatchEvent(event);
|
||||
flushSync();
|
||||
expect(imageSrc()).toContain(IMG_A);
|
||||
});
|
||||
|
||||
it('closes through a ROUTE-SHAPED driver, and closes exactly one layer', () => {
|
||||
// The integration the component depends on, exercised in the shape the
|
||||
// two route handlers actually have: bail on a foreign modal, else run the
|
||||
// stack and preventDefault. Calling `runTopEscape()` directly (as the
|
||||
// tests above do) skips the `hasForeignEscapeOwner()` guard, which is the
|
||||
// half that would silently swallow the viewer's Escape if the viewer were
|
||||
// not excluded from that selector.
|
||||
const paneClose = vi.fn(() => true);
|
||||
pushEscapeHandler(paneClose, ESCAPE_PRIORITY.pane);
|
||||
const onClose = vi.fn();
|
||||
mountViewer({ onClose });
|
||||
|
||||
const routeDriver = (e: KeyboardEvent) => {
|
||||
if (e.key !== 'Escape') return;
|
||||
if (hasForeignEscapeOwner()) return;
|
||||
if (runTopEscape()) e.preventDefault();
|
||||
};
|
||||
window.addEventListener('keydown', routeDriver);
|
||||
try {
|
||||
expect(press('Escape')).toBe(true);
|
||||
} finally {
|
||||
window.removeEventListener('keydown', routeDriver);
|
||||
}
|
||||
|
||||
expect(onClose).toHaveBeenCalledTimes(1);
|
||||
// ONE layer: the pane underneath must not also close on the same press.
|
||||
expect(paneClose).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('unregisters on close, so a later Escape reaches the layer beneath', () => {
|
||||
const paneClose = vi.fn(() => true);
|
||||
pushEscapeHandler(paneClose, ESCAPE_PRIORITY.pane);
|
||||
const app = mountViewer({ onClose: () => {} });
|
||||
|
||||
unmount(mounted.splice(mounted.indexOf(app), 1)[0]);
|
||||
flushSync();
|
||||
expect(runTopEscape()).toBe(true);
|
||||
expect(paneClose).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Lightbox — only the frontmost viewer acts', () => {
|
||||
function mountTwo() {
|
||||
const onCloseBack = vi.fn();
|
||||
const onCloseFront = vi.fn();
|
||||
mountViewer({
|
||||
onClose: onCloseBack,
|
||||
images: [
|
||||
{ id: IMG_A, alt: 'back-first' },
|
||||
{ id: IMG_B, alt: 'back-second' },
|
||||
],
|
||||
});
|
||||
const back = root();
|
||||
mountViewer({
|
||||
onClose: onCloseFront,
|
||||
images: [
|
||||
{ id: IMG_A, alt: 'front-first' },
|
||||
{ id: IMG_B, alt: 'front-second' },
|
||||
],
|
||||
});
|
||||
const front = root();
|
||||
expect(back).not.toBe(front);
|
||||
return { back, front, onCloseBack, onCloseFront };
|
||||
}
|
||||
|
||||
it('closes exactly the front viewer on one Escape', () => {
|
||||
// HONEST SCOPE: this asserts the user-visible contract, but it cannot
|
||||
// isolate the component's `isViewerFrontmost` gate. `escapeStack` breaks
|
||||
// equal-priority ties toward the most recently registered handler, and
|
||||
// registration order and lease order are the same thing here (both are
|
||||
// mount order), so the press would land on the front viewer even with the
|
||||
// gate removed — verified by mutation. The gate is kept because it makes
|
||||
// the ownership rule LOCAL rather than a consequence of another module's
|
||||
// tie-break, and because the arrow / Tab gates on the same predicate ARE
|
||||
// load-bearing (the two tests below fail without them).
|
||||
const { onCloseBack, onCloseFront } = mountTwo();
|
||||
expect(runTopEscape()).toBe(true);
|
||||
expect(onCloseFront).toHaveBeenCalledTimes(1);
|
||||
expect(onCloseBack).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('pages only the front viewer on an arrow key', () => {
|
||||
const { back, front } = mountTwo();
|
||||
expect(press('ArrowRight')).toBe(true);
|
||||
expect(imageSrc(front)).toContain(IMG_B);
|
||||
expect(imageSrc(back)).toContain(IMG_A);
|
||||
});
|
||||
|
||||
it('does not let the BACK viewer steal focus on Tab', () => {
|
||||
// The sharp edge: `nextTrapTarget` deliberately pulls out-of-container
|
||||
// focus INWARD, so a background viewer running the trap would drag focus
|
||||
// out of the viewer in front of it. Handlers are global; the frontmost
|
||||
// check is what stops it.
|
||||
const { back, front } = mountTwo();
|
||||
expect(front.contains(document.activeElement)).toBe(true);
|
||||
|
||||
press('Tab');
|
||||
expect(back.contains(document.activeElement)).toBe(false);
|
||||
expect(front.contains(document.activeElement)).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Lightbox — a native modal opened OVER the viewer', () => {
|
||||
/**
|
||||
* jsdom throws on the `:modal` pseudo-class, so emulate an engine that
|
||||
* supports it — the shape `viewerBackdrop.svelte.test.ts` uses. Both probes
|
||||
* the module makes (`querySelectorAll` and `Element.matches`) are covered.
|
||||
*/
|
||||
function mockOpenModals(modals: Element[]): void {
|
||||
const realQueryAll = document.querySelectorAll.bind(document);
|
||||
vi.spyOn(document, 'querySelectorAll').mockImplementation((selector: string) => {
|
||||
if (selector !== 'dialog:modal') return realQueryAll(selector);
|
||||
return Array.from(realQueryAll('dialog')).filter((d) =>
|
||||
modals.includes(d)
|
||||
) as unknown as NodeListOf<Element>;
|
||||
});
|
||||
const realMatches = Element.prototype.matches;
|
||||
vi.spyOn(Element.prototype, 'matches').mockImplementation(function (
|
||||
this: Element,
|
||||
selector: string
|
||||
) {
|
||||
if (selector !== 'dialog:modal') return realMatches.call(this, selector);
|
||||
return realMatches.call(this, 'dialog') && modals.includes(this);
|
||||
});
|
||||
}
|
||||
|
||||
it('stops trapping Tab while a showModal() dialog is above it', () => {
|
||||
// The frontmost LEASE is not the frontmost SURFACE: a `showModal()` dialog
|
||||
// lives in the top layer, above any body-portaled viewer, and the manager
|
||||
// deliberately leaves it OUT of the inert set so it stays operable. If the
|
||||
// viewer kept trapping, `nextTrapTarget` would pull focus out of that
|
||||
// dialog and back into the viewer underneath it — the inward-redirect
|
||||
// hazard one layer up. Reachable today: the app shell's `?` shortcut opens
|
||||
// the Keyboard Shortcuts modal while a viewer is up (TASK-2430 stops the
|
||||
// shortcut; this stops the viewer fighting the result either way).
|
||||
// The emulation goes in BEFORE the mount: the manager probes `:modal` on
|
||||
// its first reconcile, and jsdom's throw makes it cache "unsupported" for
|
||||
// the rest of the module's life. Mocking afterwards would be ignored — and
|
||||
// the test would then pass for the wrong reason.
|
||||
const dialog = document.body.appendChild(document.createElement('dialog'));
|
||||
const inDialog = dialog.appendChild(document.createElement('button'));
|
||||
mockOpenModals([dialog]);
|
||||
mountViewer({
|
||||
images: [
|
||||
{ id: IMG_A, alt: 'first' },
|
||||
{ id: IMG_B, alt: 'second' },
|
||||
],
|
||||
});
|
||||
inDialog.focus();
|
||||
|
||||
expect(press('Tab')).toBe(false);
|
||||
expect(document.activeElement).toBe(inDialog);
|
||||
});
|
||||
|
||||
it('stops paging on arrows while a showModal() dialog is above it', () => {
|
||||
const dialog = document.body.appendChild(document.createElement('dialog'));
|
||||
mockOpenModals([dialog]);
|
||||
mountViewer({
|
||||
images: [
|
||||
{ id: IMG_A, alt: 'first' },
|
||||
{ id: IMG_B, alt: 'second' },
|
||||
],
|
||||
});
|
||||
|
||||
expect(press('ArrowRight')).toBe(false);
|
||||
expect(imageSrc()).toContain(IMG_A);
|
||||
});
|
||||
|
||||
it('resumes once the dialog closes', () => {
|
||||
// The stand-down must be conditional, not a permanent disable.
|
||||
const dialog = document.body.appendChild(document.createElement('dialog'));
|
||||
mockOpenModals([dialog]);
|
||||
mountViewer({
|
||||
images: [
|
||||
{ id: IMG_A, alt: 'first' },
|
||||
{ id: IMG_B, alt: 'second' },
|
||||
],
|
||||
});
|
||||
expect(press('ArrowRight')).toBe(false);
|
||||
|
||||
mockOpenModals([]);
|
||||
expect(press('ArrowRight')).toBe(true);
|
||||
expect(imageSrc()).toContain(IMG_B);
|
||||
});
|
||||
});
|
||||
|
||||
describe('Lightbox — background inertness (delegated)', () => {
|
||||
it('asks the manager to inert the app shell, and releases it on close', () => {
|
||||
// jsdom has no inertness semantics, so this asserts the manager was
|
||||
// DRIVEN — the attribute on the right body children, gone again after
|
||||
// close. Whether the background is really unreachable is TASK-2436's.
|
||||
const app = mountViewer();
|
||||
expect(inertBodyChildren()).toEqual([appRoot]);
|
||||
|
||||
unmount(mounted.splice(mounted.indexOf(app), 1)[0]);
|
||||
flushSync();
|
||||
expect(inertBodyChildren()).toEqual([]);
|
||||
});
|
||||
|
||||
it('keeps the app inert while a SECOND viewer is still open', () => {
|
||||
// The refcount is the manager's, but the release ordering is this
|
||||
// component's: releasing without the lease stack would un-inert the app
|
||||
// behind a viewer that is still up.
|
||||
const first = mountViewer();
|
||||
const back = root();
|
||||
mountViewer();
|
||||
// Only the FRONT viewer stays interactive: the app shell AND the viewer
|
||||
// beneath it are both inert, which is the stacking the lease order buys.
|
||||
expect(inertBodyChildren()).toEqual([appRoot, back]);
|
||||
|
||||
unmount(mounted.splice(mounted.indexOf(first), 1)[0]);
|
||||
flushSync();
|
||||
expect(inertBodyChildren()).toEqual([appRoot]);
|
||||
});
|
||||
|
||||
it('hands focus to the viewer beneath instead of restoring its own invoker', () => {
|
||||
// The `stackEmpty` half of the teardown: with a viewer still open,
|
||||
// restoring the invoker would yank focus out of the surface the user is
|
||||
// actually looking at. The manager owns the handoff; this component's job
|
||||
// is to STAND DOWN.
|
||||
//
|
||||
// HONEST SCOPE, again: the two defences overlap. The manager's handoff
|
||||
// has already moved focus INTO the viewer beneath by the time the restore
|
||||
// would run, so `restoreFocus`'s own "someone else owns focus" guard
|
||||
// declines even with the `stackEmpty` gate removed — verified by
|
||||
// mutation. What IS asserted is the contract that matters: on closing the
|
||||
// front viewer, focus lands in the one beneath and never on the closed
|
||||
// viewer's invoker.
|
||||
const invokerBack = appRoot.appendChild(document.createElement('button'));
|
||||
const invokerFront = appRoot.appendChild(document.createElement('button'));
|
||||
mountViewer({ invoker: invokerBack });
|
||||
const back = root();
|
||||
const front = mountViewer({ invoker: invokerFront });
|
||||
|
||||
unmount(mounted.splice(mounted.indexOf(front), 1)[0]);
|
||||
flushSync();
|
||||
expect(document.activeElement).not.toBe(invokerFront);
|
||||
expect(back.contains(document.activeElement)).toBe(true);
|
||||
});
|
||||
});
|
||||
@@ -2,6 +2,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
|
||||
import { flushSync, mount, unmount } from 'svelte';
|
||||
import type { AttachmentListItem, AttachmentListResponse } from '$lib/types';
|
||||
import type { UploadedAttachment } from '$lib/attachments/events';
|
||||
import { runTopEscape, _resetEscapeStackForTests } from '$lib/stores/escapeStack';
|
||||
|
||||
// TASK-2383. The strip is mounted OUTSIDE ItemDetail's `{#key itemSlug}`
|
||||
// block, so it PERSISTS across an A→B item switch — the no-{#key} bug class
|
||||
@@ -163,6 +164,9 @@ describe('ItemAttachmentStrip', () => {
|
||||
if (instance) unmount(instance);
|
||||
instance = undefined;
|
||||
target.remove();
|
||||
// The viewer registers on the shared ESC stack (TASK-2429); a case that
|
||||
// leaves one open would otherwise leak a handler into the next test.
|
||||
_resetEscapeStackForTests();
|
||||
});
|
||||
|
||||
function mountStrip(itemId: string | null) {
|
||||
@@ -381,8 +385,17 @@ describe('ItemAttachmentStrip', () => {
|
||||
'img1.png'
|
||||
);
|
||||
|
||||
// Escape is no longer a local `window` listener on the viewer (TASK-2429):
|
||||
// the shared `escapeStack` is its ONE owner, driven by the route host. So
|
||||
// the close is exercised the way the app reaches it — through the stack —
|
||||
// and a raw keydown is asserted to do NOTHING, which is the regression
|
||||
// that would reintroduce the two-owners-one-press bug.
|
||||
window.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape' }));
|
||||
flushSync();
|
||||
expect(document.querySelector('.lightbox-backdrop')).not.toBeNull();
|
||||
|
||||
expect(runTopEscape()).toBe(true);
|
||||
flushSync();
|
||||
expect(document.querySelector('.lightbox-backdrop')).toBeNull();
|
||||
});
|
||||
|
||||
|
||||
@@ -30,6 +30,13 @@ export const ESCAPE_PRIORITY = {
|
||||
// Dropdown menus are the innermost layer — ESC closes an open menu
|
||||
// before it collapses the pane/drawer under it (PLAN-2290 Phase 2).
|
||||
menu: 40,
|
||||
// The body-portaled attachment viewer (PLAN-2392 phase 3a / TASK-2429). It
|
||||
// sits ABOVE the menu layer because it is a real modal: while one is open
|
||||
// the rest of the app — menus included — is inert behind it, so nothing
|
||||
// lower can have a live layer for ESC to reach. Several viewers can be
|
||||
// mounted at once (the strip's and a NodeView's); each declines unless it is
|
||||
// the FRONTMOST backdrop lease, so one press closes exactly the top one.
|
||||
viewer: 50,
|
||||
} as const;
|
||||
|
||||
interface Entry {
|
||||
|
||||
@@ -52,6 +52,7 @@
|
||||
import { resolvePaneReturnTarget } from '$lib/collections/paneFocus';
|
||||
import { createPaneMintSettle, PANE_MINT_SETTLE_MS } from '$lib/collections/paneMintSettle';
|
||||
import { pushEscapeHandler, runTopEscape, topEscapePriority, ESCAPE_PRIORITY } from '$lib/stores/escapeStack';
|
||||
import { hasForeignEscapeOwner } from '$lib/a11y/viewerBackdrop';
|
||||
import { boardKeyNav, type BoardNavColumn, type BoardNavDirection } from '$lib/collections/boardNav';
|
||||
|
||||
type ViewMode = 'list' | 'board' | 'table';
|
||||
@@ -2231,7 +2232,15 @@
|
||||
// EXCLUDED via `:not(.item-pane)` — its ESC is owned by its own escape-
|
||||
// stack handler, and counting it here would swallow ESC-to-close / pop.
|
||||
// The graph drawer isn't a dialog, so the chain is otherwise unblocked.
|
||||
if (document.querySelector('dialog[open], [role="dialog"]:not(.item-pane)')) return;
|
||||
//
|
||||
// Now the SHARED helper (TASK-2429): the attachment viewer is a
|
||||
// body-portaled `role="dialog"` whose ESC is also on the stack, so the
|
||||
// same exclusion it applies to the pane applies to it — without that,
|
||||
// this guard would return here and the viewer's Escape would be dead.
|
||||
// The helper keeps the ARIA branch (BottomSheet / DockedSheet own their
|
||||
// own ESC and are NOT on the stack) and narrows only the native branch
|
||||
// to a feature-detected `dialog:modal`.
|
||||
if (hasForeignEscapeOwner()) return;
|
||||
// A HELD key fires many auto-repeat keydowns (`e.repeat === true`).
|
||||
// Consumed here, BEFORE any layer-close/pop decision, so a hold can
|
||||
// never cascade through the chain — only the initial physical press
|
||||
|
||||
@@ -13,6 +13,7 @@
|
||||
import { inExemptSurface } from '$lib/collections/paneFocus';
|
||||
import { viewport } from '$lib/stores/breakpoint.svelte';
|
||||
import { runTopEscape, topEscapePriority, ESCAPE_PRIORITY } from '$lib/stores/escapeStack';
|
||||
import { hasForeignEscapeOwner } from '$lib/a11y/viewerBackdrop';
|
||||
import type { PaneTarget, ResolvedItemIdentity } from '$lib/types';
|
||||
|
||||
// Full-page pane HOST (PLAN-2154 Phase 2 / Architecture E, bullet 5 /
|
||||
@@ -430,11 +431,12 @@
|
||||
const target = e.target as HTMLElement | null;
|
||||
// Text-editing targets own ESC locally — don't hijack into a layer-close.
|
||||
if (isTextEntryTarget(target)) return;
|
||||
// A native <dialog> / role="dialog" sheet owns its own ESC. The pane's own
|
||||
// mobile overlay is `role="dialog"` too (TASK-2131) but is EXCLUDED via
|
||||
// `:not(.item-pane)` — it's the layer this ESC is meant to close, handled
|
||||
// through the shared escape stack, not a foreign modal to defer to.
|
||||
if (document.querySelector('dialog[open], [role="dialog"]:not(.item-pane)')) return;
|
||||
// A native modal <dialog> / role="dialog" sheet owns its own ESC. The
|
||||
// pane's own mobile overlay is `role="dialog"` too (TASK-2131) and so is
|
||||
// the body-portaled attachment viewer (TASK-2429), but BOTH are EXCLUDED
|
||||
// by the shared helper — they are layers this ESC is meant to close,
|
||||
// handled through the escape stack, not foreign modals to defer to.
|
||||
if (hasForeignEscapeOwner()) return;
|
||||
// A HELD key auto-repeats; only the initial physical press acts.
|
||||
if (e.repeat) {
|
||||
e.preventDefault();
|
||||
|
||||
Reference in New Issue
Block a user