Skip to content

Commit aaa5c1d

Browse files
fix(components): a field's Esc in the record drawer no longer steps the pane back or restores it from maximized (#28294)
<!-- claude-projects-attribution: {"github_login":"os-tesla"} --> _Requested by **Tesla** · [project thread](https://claude.ai/code/project/chan_01AXsQkLFKRdQCqQ3RTr6Jao?thread=cmsg_01AXsQkLFKRdQCqQ3RTr6Jao9Er3hy5TVuvBiu4EsvhoLJ)_ Fixes #28156 Clause-②: no Metadata-protocol: none ## 维护者速读 - 改了什么:记录抽屉里,在字段内按 Esc(关提及列表、取消单元格编辑、退出动态输入框或放大面)只归该字段:在下层时不再退回上一层,最大化时不再还原。直接按 Esc(焦点不在字段里)仍退一层、仍还原,根层仍关闭抽屉。 - 为什么改:读者在关联记录里按 Esc 只想关掉字段里的东西,却连带丢了当前层级或最大化宽度。Radix 的对话层在 document 捕获阶段先于任何字段收到 Esc,所以卡片写的那一行 `defaultPrevented` 守卫只挡得住放大面那种窗口捕获监听,挡不住普通字段;本 PR 让抽屉用分屏面已经在用的同一条判断。 - 风险与回滚:只改 `@object-ui/components` 的 `navigation-overlay.tsx` 一个处理函数。代价:焦点停在文本框里时 Esc 永远不退层(与分屏面今天一致),要先移开焦点或点 ←。回滚即 revert 本 PR。 - 先看:`ui/packages/components/src/custom/navigation-overlay.tsx` 的 `handleDrawerEscape` 与 `escapeOwnedByTarget`(新增 `pane` 参数);`ui/packages/components/src/__tests__/navigation-overlay-field-escape-28156.test.tsx`。 - 两分钟验证:`pnpm --dir ui exec vitest run packages/components/src/__tests__/navigation-overlay-field-escape-28156.test.tsx`,预期 1 个文件、19 个测试全绿。 - 需要你做的:无,席位落地。 ## What changes **Declared deviation from the card's Wanted.** The card asks for `if (e.defaultPrevented) return;` alone. Measured on `main` 69ce01d against the installed `@radix-ui/react-dismissable-layer` 1.1.19, that guard is not enough: - The layer listens for `keydown` on the document in the **capture** phase (only while it is the highest layer) and calls `onEscapeKeyDown` from there, then reads `defaultPrevented` for its own dismiss. - So it hears `Esc` before any React `onKeyDown` in the record. A probe through the real `NavigationOverlay` drawer with `onBack`: an input whose React handler calls `preventDefault()` logs `onBack` first and then the field (which sees `defaultPrevented` already true); a field marked handled from a **window** capture listener (the shape #28116's enlarged face uses) logs the face first and then `onBack`. - With the one-line guard, the window-capture case stops stepping back and the React case still does. On `main` the composer's mention list (`RichTextCommentInput`), the grid's `InlineEditing` and the data table's cell editor all take `Esc` in a React `onKeyDown`. Seat 2 chose option B, the guard plus the split pane's ownership test (seat ruling, 2026-10-31T02:56Z, in the dispatch thread). **The drawer's `Esc` (`handleDrawerEscape`).** Below the stack's root, or maximized, the drawer's `onEscapeKeyDown` now: 1. returns when the key is already handled (`defaultPrevented`): an enlarged face or anything else that takes the key ahead of the layer; 2. otherwise holds the sheet open (`preventDefault`, so the layer does not dismiss the whole drawer); 3. leaves the key to its target when `escapeOwnedByTarget` says a text field, an editable region, or an open menu, list box or dialog in the record owns it; 4. only then restores from maximized or steps back one level. This is the question the split pane beside a list already asks (`useSplitEscape`), so the drawer and the split pane now answer `Esc` the same way. **`escapeOwnedByTarget(target, pane?)`.** It takes the pane's own dialog and never counts it as an owner. Every target inside the drawer sits inside the sheet's `role="dialog"`, so without this a button in the record would read as owned and a bare `Esc` there would stop stepping back. The split pane passes nothing and reads as before. **Cost.** With focus in a text field, `Esc` never steps the pane back or restores it, as in the split pane today. The reader moves focus out of the field or uses `←`. **Unchanged.** At the root (no `onBack`, not maximized) the drawer passes no handler and the layer dismisses on `Esc` as before; this card does not cover the root. The split pane's listener is untouched. ## Tests New: `ui/packages/components/src/__tests__/navigation-overlay-field-escape-28156.test.tsx` (19), through the real `NavigationOverlay` (its Radix sheet and its split listener). The record body holds the shapes the fields really take the key in: a cell editor (React `onKeyDown`, `preventDefault`), the composer closing its mention list (React `onKeyDown`, nothing marked), a field inside an enlarged face and the face's own `Esc` (window capture listener, marks handled). - Drawer below the root: `Esc` in each of the four keeps the level (`onBack`, `setIsOpen` and `close` never called). CONTROL: a bare `Esc` steps back once and does not close; an `Esc` on a button in the record steps back too. - Drawer maximized: `Esc` in each of the four keeps it maximized and open. CONTROL: a bare `Esc` restores, and only the next one closes the root. - Split pane beside the list, below the root and maximized: the same four keep the level / the maximized width, and a bare `Esc` steps back / restores (CONTROL; these held before this PR and are pinned for parity). <details><summary>Ablations (each through <code>node scripts/ablation-replace.mjs</code>, restore proven blob == HEAD; the test imports the component from source, so no dist leg)</summary> On head e2e6e5e: - A0, the drawer handler back to `main`'s (`e.preventDefault(); handleEscape();`): `Tests 8 failed | 11 passed (19)`, the eight drawer arms (four below the root, four maximized). Controls and split arms green. - A1, the `defaultPrevented` return removed: `2 failed`, the enlarged face's own `Esc`, below the root and maximized. - A2, the ownership test removed: `4 failed`, the cell editor and the composer, below the root and maximized. - A3, `owner !== pane` removed: `1 failed`, the CONTROL that an `Esc` on a button in the record still steps back. </details> ## Acceptance notes - Acceptance item 1 (below the root, field keeps the level; bare `Esc` steps back): pinned in the drawer and in the split pane. - Acceptance item 2 (maximized, field keeps it maximized; bare `Esc` restores): pinned in the drawer and in the split pane. - Acceptance item 3 (against the real `NavigationOverlay`, ablation reds the field case): A0 above. - The root drawer is outside this card: there an `Esc` in a field still dismisses the whole drawer unless something marked it handled ahead of the layer (#28116's face does). Noted, not filed: the card scopes the levels below the root and the maximized pane. ## Verification <details><summary>Tests, type-check and the derived gates on e2e6e5e</summary> Head `e2e6e5e9`, change set vs the merge base `69ce01d7`: 3 paths, 251 changed lines. - `pnpm --dir ui exec vitest run packages/components/` under the verify lock: `Test Files 1 failed | 461 passed | 1 skipped (463)`, `Tests 5 failed | 4422 passed | 24 skipped (4451)`. The one red file was `share-filter-sort-spec-parity.test.ts`, which reads `@objectstack/spec`'s `.d.ts` and met a closure built with `OS_SKIP_DTS=1` (no declarations). After `env -u OS_SKIP_DTS pnpm turbo run build --filter='@object-ui/components^...'` (exit 0) it and the new file pass: `Test Files 2 passed (2)`. - `pnpm --filter @object-ui/components run type-check` (`tsc --noEmit && tsc -p tsconfig.test.json`): exit 0. eslint on the two changed source files: exit 0. `pnpm --dir ui run check:new-line-citations`: `0 new citation(s)`, exit 0. - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`: 34 commands, each run with its exit code captured before any pipe; `--ran` reconcile: `34 derived, 34 run, 0 NOT-MEASURED, 0 UNRUN`. `pnpm --dir ui exec vitest run scripts/`: `Test Files 169 passed | 2 skipped (171)`. - Not run locally, CI's own: the whole `ui/` suite (Test UI), the other type-check lanes, the repo-wide scans. Dogfood not owed (no `!` in the title, no `(narrowing)` arm). ``` node scripts/check-adr-0087-registration.mjs --base origin/main :: exit 0 node scripts/check-adr-0087-registration.mjs --self-test :: exit 0 node scripts/check-changeset-no-major.mjs --base origin/main :: exit 0 node scripts/check-closing-keyword-parity.mjs :: exit 0 node scripts/check-closing-keyword-parity.mjs --self-test :: exit 0 node scripts/check-comment-mask-corpus.mjs :: exit 0 node scripts/check-empty-changeset.mjs --base origin/main :: exit 0 node scripts/check-empty-changeset.mjs --self-test :: exit 0 node scripts/check-sdui-manifest.mjs :: exit 0 node scripts/check-sdui-manifest.mjs --self-test :: exit 0 node scripts/pm/release-rehearsal-clone.mjs --self-test :: exit 0 node scripts/release-pending-publish.mjs --self-test :: exit 0 pnpm --filter @objectstack/spec run check:duration-unit-keys :: exit 0 pnpm check:changeset-gate-self-tests :: exit 0 pnpm check:closed-ceremony-text :: exit 0 pnpm check:cross-package-test-inputs :: exit 0 pnpm check:dispatcher-error-vocabulary :: exit 0 pnpm check:driver-memory-census :: exit 0 pnpm check:engine-double-contract :: exit 0 pnpm check:gitlink-declared :: exit 0 pnpm check:nul-bytes :: exit 0 pnpm check:pm-changeset-deadline-census :: exit 0 pnpm check:published-files :: exit 0 pnpm check:query-options-erasure :: exit 0 pnpm check:refd-timer-probe :: exit 0 pnpm check:skill-identifier-liveness :: exit 0 pnpm check:test-source-alias :: exit 0 pnpm check:tier-file-adoption :: exit 0 pnpm check:type-check-coverage :: exit 0 pnpm check:undeclared-dep-imports :: exit 0 pnpm check:watch-hint-literal :: exit 0 pnpm check:where-matcher :: exit 0 pnpm check:type-check-debt :: exit 0 pnpm --dir ui exec vitest run scripts/ :: exit 0 ``` </details> --- _Generated by [Claude Code](https://claude.ai/code/session_01L9qibwcrcmDwuHqtrvwstB)_ Co-authored-by: Claude <noreply@anthropic.com>
1 parent a8d2fd7 commit aaa5c1d

3 files changed

Lines changed: 245 additions & 6 deletions

File tree

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
'@object-ui/components': patch
3+
---
4+
5+
fix(components): in the record drawer, an `Esc` a field takes stays the field's — below the stack's root it no longer steps the pane back, and maximized it no longer restores the pane
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
The drawer's dialog layer hears `Esc` on the document in the capture phase, before any field in the record does, and its handler stepped back one level (below the root) or restored the pane (maximized) on every `Esc`. So closing a mention list in the composer, cancelling a cell edit, or leaving an enlarged face also lost the reader's level or the maximized width.
11+
12+
- The drawer now asks what the split pane beside a list already asks: an `Esc` already marked handled, or pressed in a text field, an editable region, or an open menu, list box or dialog inside the record, is not the pane's. The pane stays where it is and the field gets the key.
13+
- A bare `Esc`, or one pressed on a control that owns no key (a button in the record), still steps back below the root and still restores from maximized; at the root it still closes the drawer.
14+
- With focus in a text field, `Esc` never steps the pane back, as in the split pane: move focus out of the field, or use `←`.
Lines changed: 209 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,209 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* This source code is licensed under the MIT license found in the
6+
* LICENSE file in the root directory of this source tree.
7+
*/
8+
9+
/**
10+
* objectstack#28156 — an `Esc` a field in the record takes is the field's:
11+
* below the stack's root it does not step the pane back, and maximized it
12+
* does not restore the pane. A bare `Esc` still does both (the controls).
13+
*
14+
* The drawer's Radix dialog layer hears `Esc` on the document in the capture
15+
* phase, ahead of the record's own handlers, so the fields are pinned in the
16+
* shapes they really take the key in:
17+
* - a cell editor cancels in a React `onKeyDown` and marks the key handled;
18+
* - the composer closes its mention list in a React `onKeyDown` and marks
19+
* nothing;
20+
* - an enlarged face, which marks the key handled from a window capture
21+
* listener, ahead of the layer: both its own `Esc` (it closes itself) and
22+
* one pressed in a field inside it.
23+
* The split pane beside a list answers the same, from its document listener.
24+
*/
25+
26+
import * as React from 'react';
27+
import { describe, it, expect, afterEach, beforeEach, vi } from 'vitest';
28+
import { render, screen, cleanup, fireEvent } from '@testing-library/react';
29+
import {
30+
NavigationOverlay,
31+
RECORD_SPLIT_MIN_CONTENT_PX,
32+
type NavigationOverlayProps,
33+
} from '../custom/navigation-overlay';
34+
35+
/** What each field did with the `Esc` it got. */
36+
let fieldLog: string[] = [];
37+
38+
/** The record's body: the fields, the face, and a control that owns no key. */
39+
function RecordBody() {
40+
const faceRef = React.useRef<HTMLDivElement>(null);
41+
React.useEffect(() => {
42+
const onKeyDown = (e: KeyboardEvent) => {
43+
if (e.key !== 'Escape' || !(e.target instanceof Node)) return;
44+
if (!faceRef.current?.contains(e.target)) return;
45+
fieldLog.push('face');
46+
e.preventDefault();
47+
};
48+
window.addEventListener('keydown', onKeyDown, true);
49+
return () => window.removeEventListener('keydown', onKeyDown, true);
50+
}, []);
51+
return (
52+
<div>
53+
<input
54+
aria-label="Cell editor"
55+
onKeyDown={(e) => {
56+
if (e.key !== 'Escape') return;
57+
fieldLog.push('cell editor');
58+
e.preventDefault();
59+
}}
60+
/>
61+
<textarea
62+
aria-label="Composer"
63+
onKeyDown={(e) => {
64+
if (e.key === 'Escape') fieldLog.push('mention list');
65+
}}
66+
/>
67+
<div ref={faceRef} tabIndex={0} aria-label="Enlarged face">
68+
<input aria-label="Face field" />
69+
</div>
70+
<button type="button">Record action</button>
71+
</div>
72+
);
73+
}
74+
75+
const FIELDS: Array<[label: string, logged: string]> = [
76+
['Cell editor', 'cell editor'],
77+
['Composer', 'mention list'],
78+
['Face field', 'face'],
79+
['Enlarged face', 'face'],
80+
];
81+
82+
/** Rendered widths by test id; everything else measures 0 (unmeasured). */
83+
let widths: Record<string, number> = {};
84+
const realRect = HTMLElement.prototype.getBoundingClientRect;
85+
86+
beforeEach(() => {
87+
fieldLog = [];
88+
widths = {};
89+
HTMLElement.prototype.getBoundingClientRect = function (this: HTMLElement) {
90+
const width = widths[this.dataset.testid ?? ''] ?? 0;
91+
return { width, height: 0, top: 0, left: 0, right: width, bottom: 0, x: 0, y: 0, toJSON: () => ({}) } as DOMRect;
92+
};
93+
try { window.localStorage.clear(); } catch { /* private mode */ }
94+
});
95+
afterEach(() => {
96+
cleanup();
97+
HTMLElement.prototype.getBoundingClientRect = realRect;
98+
});
99+
100+
function renderPane(props: Partial<NavigationOverlayProps> = {}) {
101+
const setIsOpen = vi.fn();
102+
const close = vi.fn();
103+
render(
104+
<NavigationOverlay
105+
isOpen
106+
isOverlay
107+
selectedRecord={{ id: 'r1' }}
108+
mode="drawer"
109+
close={close}
110+
setIsOpen={setIsOpen}
111+
title="Lead"
112+
{...props}
113+
>
114+
{() => <RecordBody />}
115+
</NavigationOverlay>,
116+
);
117+
return { setIsOpen, close };
118+
}
119+
120+
/** The split pane beside a list wide enough to keep it there. */
121+
const besideList: Partial<NavigationOverlayProps> = {
122+
mode: 'split',
123+
mainContent: <div>list</div>,
124+
};
125+
126+
const esc = (target: Element | Document = document.body) => fireEvent.keyDown(target, { key: 'Escape' });
127+
const field = (label: string) => screen.getByLabelText(label);
128+
const maximize = () => fireEvent.click(screen.getByTestId('record-pane-maximize'));
129+
const maximizedPane = () => document.querySelector('[data-maximized]');
130+
131+
describe('drawer, below the root: a field keeps its Esc, the level stays', () => {
132+
it.each(FIELDS)('Esc in the %s is the field’s; the drawer neither steps back nor closes', (label, logged) => {
133+
const onBack = vi.fn();
134+
const { setIsOpen, close } = renderPane({ onBack });
135+
esc(field(label));
136+
expect(fieldLog).toEqual([logged]);
137+
expect(onBack).not.toHaveBeenCalled();
138+
expect(setIsOpen).not.toHaveBeenCalled();
139+
expect(close).not.toHaveBeenCalled();
140+
expect(screen.getByRole('dialog', { name: 'Lead' })).toBeTruthy();
141+
});
142+
143+
it('CONTROL: a bare Esc steps back one level, and does not close', () => {
144+
const onBack = vi.fn();
145+
const { setIsOpen } = renderPane({ onBack });
146+
esc();
147+
expect(onBack).toHaveBeenCalledTimes(1);
148+
expect(setIsOpen).not.toHaveBeenCalled();
149+
});
150+
151+
it('CONTROL: an Esc on a control in the record that owns no key steps back too (the sheet’s own dialog owns nothing)', () => {
152+
const onBack = vi.fn();
153+
renderPane({ onBack });
154+
esc(screen.getByRole('button', { name: 'Record action' }));
155+
expect(onBack).toHaveBeenCalledTimes(1);
156+
expect(fieldLog).toEqual([]);
157+
});
158+
});
159+
160+
describe('drawer, maximized: a field keeps its Esc, the pane stays maximized', () => {
161+
it.each(FIELDS)('Esc in the %s is the field’s; the drawer stays maximized and open', (label, logged) => {
162+
const { setIsOpen, close } = renderPane({ maximizable: true });
163+
maximize();
164+
expect(maximizedPane()).not.toBeNull();
165+
esc(field(label));
166+
expect(fieldLog).toEqual([logged]);
167+
expect(maximizedPane()).not.toBeNull();
168+
expect(setIsOpen).not.toHaveBeenCalled();
169+
expect(close).not.toHaveBeenCalled();
170+
});
171+
172+
it('CONTROL: a bare Esc restores, and only the next Esc closes the root', () => {
173+
const { setIsOpen } = renderPane({ maximizable: true });
174+
maximize();
175+
esc();
176+
expect(maximizedPane()).toBeNull();
177+
expect(setIsOpen).not.toHaveBeenCalled();
178+
esc();
179+
expect(setIsOpen).toHaveBeenCalledWith(false);
180+
});
181+
});
182+
183+
describe('split pane beside the list: the same answers', () => {
184+
beforeEach(() => {
185+
widths = { 'record-split-host': RECORD_SPLIT_MIN_CONTENT_PX + 200, 'record-split-list': 600 };
186+
});
187+
188+
it.each(FIELDS)('below the root, Esc in the %s keeps the level; a bare Esc steps back (CONTROL)', (label, logged) => {
189+
const onBack = vi.fn();
190+
renderPane({ ...besideList, onBack });
191+
expect(screen.getByTestId('record-split-pane')).toBeTruthy();
192+
esc(field(label));
193+
expect(fieldLog).toEqual([logged]);
194+
expect(onBack).not.toHaveBeenCalled();
195+
esc();
196+
expect(onBack).toHaveBeenCalledTimes(1);
197+
});
198+
199+
it.each(FIELDS)('maximized, Esc in the %s keeps it maximized; a bare Esc restores (CONTROL)', (label, logged) => {
200+
const { close } = renderPane({ ...besideList, maximizable: true });
201+
maximize();
202+
esc(field(label));
203+
expect(fieldLog).toEqual([logged]);
204+
expect(maximizedPane()).not.toBeNull();
205+
esc();
206+
expect(maximizedPane()).toBeNull();
207+
expect(close).not.toHaveBeenCalled();
208+
});
209+
});

‎ui/packages/components/src/custom/navigation-overlay.tsx‎

Lines changed: 22 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -433,15 +433,17 @@ function useElementWidth(el: HTMLElement | null): number | null {
433433

434434
/**
435435
* Whether an `Esc` belongs to the element it was pressed in rather than to the
436-
* split pane: a text field, or an open menu, list box or dialog (which closes
437-
* itself first and marks the event handled).
436+
* pane: a text field, or an open menu, list box or dialog (which closes itself
437+
* first and marks the event handled). `pane` is the pane's own dialog — the
438+
* drawer's sheet — which never owns the key it is asking about.
438439
*/
439-
function escapeOwnedByTarget(target: EventTarget | null): boolean {
440+
function escapeOwnedByTarget(target: EventTarget | null, pane?: Element | null): boolean {
440441
if (!target || !(target instanceof Element)) return false;
441442
if (target.closest('input, textarea, select, [contenteditable=""], [contenteditable="true"]')) {
442443
return true;
443444
}
444-
return !!target.closest('[role="menu"], [role="listbox"], [role="dialog"], [role="alertdialog"]');
445+
const owner = target.closest('[role="menu"], [role="listbox"], [role="dialog"], [role="alertdialog"]');
446+
return !!owner && owner !== pane;
445447
}
446448

447449
/**
@@ -647,6 +649,18 @@ export const NavigationOverlay: React.FC<NavigationOverlayProps> = ({
647649
else close();
648650
};
649651
useSplitEscape(hasList && face === 'split' && shown, handleEscape);
652+
// The drawer's dialog layer hears `Esc` on the document in the capture
653+
// phase, before any field in the record does, so it asks the split pane's
654+
// question itself: a key already handled, or pressed where a field or a
655+
// menu owns it, is not the pane's. It still holds the sheet open — left
656+
// unprevented, the layer would dismiss the whole drawer on it.
657+
const sheetRef = React.useRef<HTMLDivElement>(null);
658+
const handleDrawerEscape = (e: KeyboardEvent) => {
659+
if (e.defaultPrevented) return;
660+
e.preventDefault();
661+
if (escapeOwnedByTarget(e.target, sheetRef.current)) return;
662+
handleEscape();
663+
};
650664

651665
// Non-overlay modes draw no record (the hook navigates); only the list.
652666
if (!paneMode && !hasList) {
@@ -801,8 +815,10 @@ export const NavigationOverlay: React.FC<NavigationOverlayProps> = ({
801815
onOpenAutoFocus={(e) => e.preventDefault()}
802816
// objectstack#27545 — from maximized, `Esc` restores; objectstack#26679
803817
// — below the root of an in-place stack, `Esc` goes back one level.
804-
// Only the root's `Esc` closes.
805-
onEscapeKeyDown={maximized || onBack ? (e) => { e.preventDefault(); handleEscape(); } : undefined}
818+
// Only the root's `Esc` closes. objectstack#28156 — a field's `Esc`
819+
// stays the field's ({@link escapeOwnedByTarget}).
820+
onEscapeKeyDown={maximized || onBack ? handleDrawerEscape : undefined}
821+
ref={sheetRef}
806822
className={cn(
807823
// Mobile: full width (no inline cap, no max-w from base sheet).
808824
// sm+: honor the pane width via the `--ov-w` CSS var. The host's

0 commit comments

Comments
 (0)