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/layer-dismiss-gesture.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@astryxdesign/core': patch
---

[fix] Popup triggers no longer fight the browser's own light dismiss: pressing the button of an open Selector, MultiSelector, ComplexSelector, DropdownMenu or Popover closes it once instead of closing and reopening, and a clear or status button sitting on the trigger no longer dismisses the popup it belongs to

@cixzhang
30 changes: 30 additions & 0 deletions packages/core/src/ComplexSelector/ComplexSelector.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -417,4 +417,34 @@ describe('ComplexSelector popup theme target', () => {
document.querySelector('.astryx-complex-selector-popup'),
).not.toBeNull();
});

it('stays closed when the trigger click follows its own light dismiss (#5004)', async () => {
const user = userEvent.setup();
render(
<ComplexSelector label="Fruit blend" value="Apple" triggerLabel="Apple">
{() => <button type="button">Done</button>}
</ComplexSelector>,
);
const trigger = screen.getByRole('button', {name: 'Fruit blend'});
await user.click(trigger);
expect(trigger).toHaveAttribute('aria-expanded', 'true');

// The browser dismissed the popup on pointerup and queued the toggle. When
// that event lands before the click — WebKit, or any engine under load —
// the click used to read a closed popup and reopen it.
const popover = document.querySelector('[popover]') as HTMLElement;
act(() => {
popover.dispatchEvent(
Object.assign(new Event('toggle'), {
oldState: 'open',
newState: 'closed',
}),
);
});
// Synchronously: the click falls inside the one gesture the guard covers,
// as it does in a browser a few milliseconds behind the dismissal.
fireEvent.click(trigger);

expect(trigger).toHaveAttribute('aria-expanded', 'false');
});
});
4 changes: 1 addition & 3 deletions packages/core/src/ComplexSelector/ComplexSelector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -373,14 +373,12 @@ export function ComplexSelector<Value>({
.join(' ') || undefined;

const triggerRef = useRef<HTMLButtonElement>(null);
const lastHideTimeRef = useRef(0);

const [isPending, startTransition] = useTransition();
const [optimisticValue, setOptimisticValue] = useOptimistic(value);
const isBusy = isLoading || isPending;

const handlePopoverHide = useCallback(() => {
lastHideTimeRef.current = Date.now();
triggerRef.current?.focus();
}, []);

Expand All @@ -395,7 +393,7 @@ export function ComplexSelector<Value>({
const isOpen = popover.isOpen;

const handleTriggerClick = useCallback(() => {
if (isDisabled || Date.now() - lastHideTimeRef.current < 50) {
if (isDisabled) {
return;
}
if (popover.isOpen) {
Expand Down
52 changes: 42 additions & 10 deletions packages/core/src/DropdownMenu/DropdownMenu.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
*/

import {describe, it, expect, vi, beforeEach} from 'vitest';
import {render, screen, fireEvent, waitFor} from '@testing-library/react';
import {render, screen, fireEvent, waitFor, act} from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import {useState} from 'react';
import {DropdownMenu} from './DropdownMenu';
Expand Down Expand Up @@ -315,24 +315,56 @@ describe('DropdownMenu', () => {
});

describe('DropdownMenu light-dismiss race', () => {
it('does not re-open the menu when a click follows a hide within the guard window', () => {
// Reproduces the iOS Safari race: pointerdown fires light-dismiss before
// the subsequent click on the trigger; without the guard, the click would
// immediately re-open the menu in the same tap.
function openMenu() {
render(
<DropdownMenu
button={{label: 'Actions'}}
items={[{label: 'Edit'}]}
data-testid="astryx-dropdown-menu"
/>,
);

const trigger = screen.getByTestId('astryx-dropdown-menu');
fireEvent.click(trigger); // open
fireEvent.click(trigger); // close (stamps guard)
fireEvent.click(trigger); // would re-open without guard
fireEvent.pointerDown(trigger);
fireEvent.click(trigger);
expect(HTMLElement.prototype.showPopover).toHaveBeenCalledTimes(1);
expect(HTMLElement.prototype.hidePopover).toHaveBeenCalledTimes(1);
return trigger;
}

/**
* The browser dismisses the menu on pointerup and queues the `toggle` event;
* on the engines that lose the race it reaches React before the trigger's
* own click, which then reads a closed menu.
*/
function lightDismiss() {
const popover = document.querySelector('[popover]') as HTMLElement;
act(() => {
popover.dispatchEvent(
Object.assign(new Event('toggle'), {
oldState: 'open',
newState: 'closed',
}),
);
});
}

it('does not re-open when the trigger click follows its own light dismiss', () => {
const trigger = openMenu();

lightDismiss();
fireEvent.click(trigger);

expect(HTMLElement.prototype.showPopover).toHaveBeenCalledTimes(1);
});

it('re-opens on a press of its own after a light dismiss', () => {
const trigger = openMenu();

lightDismiss();
fireEvent.click(trigger);
fireEvent.pointerDown(trigger);
fireEvent.click(trigger);

expect(HTMLElement.prototype.showPopover).toHaveBeenCalledTimes(2);
});
});

Expand Down
13 changes: 2 additions & 11 deletions packages/core/src/DropdownMenu/DropdownMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -261,14 +261,8 @@ export function DropdownMenu({
const isControlled = controlledIsOpen !== undefined;
const isOpen = isControlled ? controlledIsOpen : internalIsOpen;

// Track when the menu was last hidden so a near-simultaneous trigger
// click — e.g. on iOS Safari where pointerdown fires light-dismiss
// before the trigger's click event — can't immediately re-open it.
const lastHideTimeRef = useRef(0);

// Close menu + return focus to trigger
const handleLayerHide = useCallback(() => {
lastHideTimeRef.current = Date.now();
onOpenChange?.(false);
if (!isControlled) {
setInternalIsOpen(false);
Expand Down Expand Up @@ -422,11 +416,8 @@ export function DropdownMenu({

const handleButtonClick = useCallback(
(e: React.MouseEvent<HTMLButtonElement>) => {
// If the menu was just closed by light dismiss (e.g. iOS Safari fires
// pointerdown → hide before the trigger's click), the click would
// otherwise immediately re-open it. Short-circuit within the guard
// window.
if (Date.now() - lastHideTimeRef.current < 50) {
// The click that light-dismissed the menu is not a request to reopen it.
if (popover.wasJustDismissed()) {
return;
}
onClick?.();
Expand Down
11 changes: 11 additions & 0 deletions packages/core/src/Field/InputClearButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,13 @@ const styles = stylex.create({
export interface InputClearButtonProps {
label: string;
onClick: (e: React.MouseEvent<HTMLButtonElement>) => void;
/**
* Pointer and capture-phase click handlers, for inputs that render this
* button beside an open layer: spread `keepOpenProps` so pressing the clear
* button does not light-dismiss the layer it sits next to.
*/
onPointerDown?: React.PointerEventHandler<HTMLElement>;
onClickCapture?: React.MouseEventHandler<HTMLElement>;
xstyle?: stylex.StyleXStyles;
/**
* Extra class(es) for the clear glyph itself, merged onto the shared
Expand All @@ -83,6 +90,8 @@ export interface InputClearButtonProps {
export function InputClearButton({
label,
onClick,
onPointerDown,
onClickCapture,
xstyle,
iconClassName,
}: InputClearButtonProps): ReactNode {
Expand All @@ -107,6 +116,8 @@ export function InputClearButton({
/>
}
onClick={onClick}
onPointerDown={onPointerDown}
onClickCapture={onClickCapture}
isIconOnly
xstyle={[styles.button, xstyle]}
/>
Expand Down
45 changes: 45 additions & 0 deletions packages/core/src/Layer/gestureCounter.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
// Copyright (c) Meta Platforms, Inc. and affiliates.

'use client';

/**
* @file gestureCounter.ts
* @input Listens for pointerdown and keydown on the document
* @output Exports currentGesture, a counter identifying the user gesture in
* flight
* @position Internal to Layer; used by useLayer to tell a click that belongs
* to a dismissing press from a fresh one
*
* A browser light-dismiss and the trigger's own click come from ONE press, and
* which of them React sees first is a race. Comparing timestamps against a
* window guesses; counting gestures does not. The counter advances on every
* new press or keystroke, so "the click from the gesture that dismissed the
* layer" is exactly "the click while the counter still reads what it read at
* the dismissal", no matter how long the main thread was blocked in between.
*/

let gesture = 0;
let isListening = false;

function advance() {
gesture += 1;
}

function listen() {
if (isListening || typeof document === 'undefined') {
return;
}
isListening = true;
// Capture phase: the count must advance before any handler reads it.
document.addEventListener('pointerdown', advance, true);
document.addEventListener('keydown', advance, true);
}

/**
* Identifies the user gesture in flight. Two reads returning the same value
* happened within one press (or one keystroke).
*/
export function currentGesture(): number {
listen();
return gesture;
}
Loading
Loading