Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/bottom-sheet-ime-escape-guard.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@astryxdesign/core': patch
---

[fix] BottomSheet: a standalone sheet no longer dismisses when a CJK user presses Escape to cancel an in-progress IME composition. The browser fires that keydown before `compositionend`, so an Escape handler reading a bare `event.key` misread the composition cancel as a dismissal command and closed the sheet — losing whatever had been typed into a `purpose="form"` field inside it. The handler now early-returns on `isImeKeyEvent`, the same guard `Dialog` and `BottomSheetSwitcher` already carry, and claims the key first so the browser raises no close request of its own.

@ernestt
123 changes: 123 additions & 0 deletions packages/core/src/BottomSheet/BottomSheet.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,106 @@ describe('BottomSheet', () => {
expect(onOpenChange).toHaveBeenCalledWith(false);
});

it('ignores Escape while an IME composition is active', () => {
const onOpenChange = vi.fn();
render(
<BottomSheet isOpen onOpenChange={onOpenChange} label="Filters">
<input aria-label="Search" />
</BottomSheet>,
);
const dialog = screen.getByRole('dialog');

fireEvent.keyDown(dialog, {key: 'Escape', isComposing: true});
fireEvent.keyDown(dialog, {key: 'Escape', keyCode: 229});

expect(onOpenChange).not.toHaveBeenCalled();
});

it('claims the composing Escape so no native close request follows', () => {
const onOpenChange = vi.fn();
render(
<BottomSheet isOpen onOpenChange={onOpenChange} label="Filters">
<input aria-label="Search" />
</BottomSheet>,
);
const dialog = screen.getByRole('dialog');

// An unclaimed Escape lets the browser raise its own close request, which
// arrives as `cancel` and dismisses the sheet on the same keypress. The
// guard has to swallow the composing Escape, not merely skip dismissal.
const wasNotClaimed = fireEvent.keyDown(dialog, {
key: 'Escape',
isComposing: true,
});

expect(wasNotClaimed).toBe(false);
expect(onOpenChange).not.toHaveBeenCalled();
});

it('keeps a form sheet open when Escape cancels a field composition', () => {
const onOpenChange = vi.fn();
render(
<BottomSheet
isOpen
purpose="form"
onOpenChange={onOpenChange}
label="Edit profile">
<input aria-label="Name" />
</BottomSheet>,
);

// The reported path: the keydown starts at the composing field and bubbles
// to the sheet's <dialog>, so the guard has to survive the trip.
fireEvent.keyDown(screen.getByRole('textbox', {name: 'Name'}), {
key: 'Escape',
isComposing: true,
});

expect(onOpenChange).not.toHaveBeenCalled();
});

it('still dismisses on the Escape after a composition is cancelled', () => {
const onOpenChange = vi.fn();
render(
<BottomSheet isOpen onOpenChange={onOpenChange} label="Filters">
<input aria-label="Search" />
</BottomSheet>,
);
const dialog = screen.getByRole('dialog');

fireEvent.keyDown(dialog, {key: 'Escape', isComposing: true});
expect(onOpenChange).not.toHaveBeenCalled();

fireEvent.keyDown(dialog, {key: 'Escape'});

expect(onOpenChange).toHaveBeenCalledWith(false);
});

it('leaves the IME the composition keys that are not Escape', () => {
const onOpenChange = vi.fn();
render(
<BottomSheet isOpen onOpenChange={onOpenChange} label="Filters">
<input aria-label="Search" />
</BottomSheet>,
);
const dialog = screen.getByRole('dialog');

// Enter commits a candidate and the arrows walk the candidate window. The
// sheet must claim neither, or the IME loses keys it owns.
const enterUnclaimed = fireEvent.keyDown(dialog, {
key: 'Enter',
isComposing: true,
});
const arrowUnclaimed = fireEvent.keyDown(dialog, {
key: 'ArrowDown',
isComposing: true,
});

expect(enterUnclaimed).toBe(true);
expect(arrowUnclaimed).toBe(true);
expect(onOpenChange).not.toHaveBeenCalled();
});

it('requests close when the scrim (dialog element itself) is clicked', () => {
const onOpenChange = vi.fn();
render(
Expand Down Expand Up @@ -474,6 +574,29 @@ describe('BottomSheet', () => {
expect(onOpenChange).toHaveBeenCalledWith(false);
});

it('ignores Escape while an IME composition is active', () => {
const onOpenChange = vi.fn();
render(
<BottomSheet
isOpen
hasScrim={false}
onOpenChange={onOpenChange}
label="Filters">
<input aria-label="Search" />
</BottomSheet>,
);

// Escape is this sheet's only keyboard route out — there is no native
// close request behind it — so the guard is all that stands between a
// composing CJK user and a dismissed sheet.
fireEvent.keyDown(screen.getByRole('dialog'), {
key: 'Escape',
isComposing: true,
});

expect(onOpenChange).not.toHaveBeenCalled();
});

it('still dismisses on a downward swipe past the threshold', () => {
const onOpenChange = vi.fn();
render(
Expand Down
21 changes: 17 additions & 4 deletions packages/core/src/BottomSheet/BottomSheet.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import * as stylex from '@stylexjs/stylex';
import type {BaseProps} from '../BaseProps';
import type {DialogPurpose} from '../Dialog';
import {colorVars, durationVars, easeVars} from '../theme/tokens.stylex';
import {useDevWarning, useScrollLock} from '../hooks';
import {isImeKeyEvent, useDevWarning, useScrollLock} from '../hooks';
import {
BottomSheetPanel,
type BottomSheetPanelMotion,
Expand Down Expand Up @@ -298,17 +298,30 @@ function StandaloneBottomSheet({

const handleCancel = useCallback(
(event: React.SyntheticEvent<HTMLDialogElement>) => {
// No IME guard here: `cancel` is a plain Event carrying no composition
// state, and handleKeyDown claims a composing Escape before the browser
// can raise the close request that would arrive here.
event.preventDefault();
dismissOnEscape();
},
[dismissOnEscape],
);
const handleKeyDown = useCallback(
(event: React.KeyboardEvent<HTMLDialogElement>) => {
if (event.key === 'Escape') {
event.preventDefault();
dismissOnEscape();
if (event.key !== 'Escape') {
return;
}
// Claim the key before reading it: an unclaimed Escape lets the browser
// raise its own close request, which lands on handleCancel and dismisses
// on the same keypress.
event.preventDefault();
// An IME fires this keydown to cancel an in-progress composition, ahead
// of compositionend. It is a composition cancel, not a dismissal command
// — see utils/ime; Dialog and BottomSheetSwitcher guard the same way.
if (isImeKeyEvent(event.nativeEvent)) {
return;
}
dismissOnEscape();
},
[dismissOnEscape],
);
Expand Down
Loading