Skip to content

Commit 6b19b6e

Browse files
authored
[drawer] Fix popup flashing fully open for a frame on swipe area re-grab (#5112)
1 parent 50a91e0 commit 6b19b6e

7 files changed

Lines changed: 221 additions & 7 deletions

File tree

packages/react/src/drawer/root/DrawerRoot.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ export function DrawerRoot<Payload = unknown>(props: DrawerRoot.Props<Payload>)
7676
});
7777

7878
const isNestedDrawerOpenRef = React.useRef(false);
79+
const swipeAreaActiveRef = React.useRef(false);
7980

8081
const setActiveSnapPoint = useStableCallback(
8182
(
@@ -172,6 +173,7 @@ export function DrawerRoot<Payload = unknown>(props: DrawerRoot.Props<Payload>)
172173
const contextValue: DrawerRootContext = React.useMemo(
173174
() => ({
174175
swipeDirection,
176+
swipeAreaActiveRef,
175177
snapToSequentialPoints,
176178
snapPoints,
177179
activeSnapPoint: resolvedActiveSnapPoint,
@@ -210,6 +212,7 @@ export function DrawerRoot<Payload = unknown>(props: DrawerRoot.Props<Payload>)
210212
setActiveSnapPoint,
211213
snapPoints,
212214
snapToSequentialPoints,
215+
swipeAreaActiveRef,
213216
swipeDirection,
214217
],
215218
);

packages/react/src/drawer/root/DrawerRootContext.ts

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,11 @@ export interface DrawerNestedSwipeProgressStore {
1313

1414
export interface DrawerRootContext {
1515
swipeDirection: DrawerSwipeDirection;
16+
/**
17+
* Whether `Drawer.SwipeArea` is currently driving an open gesture (writing the popup's
18+
* swipe-movement vars imperatively). The viewport reads this to skip resetting them on open.
19+
*/
20+
swipeAreaActiveRef: React.MutableRefObject<boolean>;
1621
/**
1722
* Whether to disable velocity-based snap skipping.
1823
*/

packages/react/src/drawer/swipe-area/DrawerSwipeArea.test.tsx

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -612,6 +612,101 @@ describe('<Drawer.SwipeArea />', () => {
612612
expect(swipeArea).toHaveAttribute('data-open', '');
613613
});
614614

615+
it.skipIf(isJSDOM)(
616+
'keeps the swipe-area movement on the popup when re-grabbed during close',
617+
async () => {
618+
// Regression guard for the swipe-area re-grab flash. When the swipe area drives the open, the
619+
// popup's `--drawer-swipe-movement-*` are written imperatively by `applySwipeMovement`, but
620+
// the viewport's open-reset effect would otherwise zero them
621+
// (`resetSwipe` -> `syncDragStyles(false)`) on the same commit that flips `open` true,
622+
// flashing the popup fully open for a frame. The shared `swipeAreaActiveRef` must make the
623+
// viewport skip that reset while the swipe area owns the gesture.
624+
//
625+
// The flash only reproduces on a *re-grab*, i.e. when the popup is already mounted as the open
626+
// commit lands. A real exit animation keeps the popup mounted (`mounted` stays true) through
627+
// the close, so the re-grab below drives a fresh open while it is still in the DOM.
628+
globalThis.BASE_UI_ANIMATIONS_DISABLED = false;
629+
630+
const style = `
631+
@keyframes swipe-regrab-exit {
632+
to {
633+
opacity: 0;
634+
}
635+
}
636+
637+
.swipe-regrab-popup {
638+
height: 200px;
639+
}
640+
641+
.swipe-regrab-popup[data-ending-style] {
642+
animation: swipe-regrab-exit 200ms;
643+
}
644+
`;
645+
646+
try {
647+
await render(
648+
<div>
649+
{/* eslint-disable-next-line react/no-danger */}
650+
<style dangerouslySetInnerHTML={{ __html: style }} />
651+
<Drawer.Root>
652+
<Drawer.SwipeArea data-testid="swipe-area" />
653+
<Drawer.Portal>
654+
<Drawer.Viewport>
655+
<Drawer.Popup className="swipe-regrab-popup" data-testid="popup">
656+
<Drawer.Close>Close</Drawer.Close>
657+
</Drawer.Popup>
658+
</Drawer.Viewport>
659+
</Drawer.Portal>
660+
</Drawer.Root>
661+
</div>,
662+
);
663+
664+
const swipeArea = screen.getByTestId('swipe-area');
665+
666+
await swipeUp(swipeArea, 220, 20);
667+
expect(swipeArea).toHaveAttribute('data-open', '');
668+
669+
const popup = screen.getByTestId('popup');
670+
671+
// Begin closing; the exit animation keeps the popup mounted while it plays.
672+
await act(async () => {
673+
screen.getByRole('button', { name: 'Close' }).click();
674+
});
675+
await waitFor(() => {
676+
expect(popup).toHaveAttribute('data-ending-style');
677+
});
678+
679+
// Re-grab while the popup is still mounted mid-exit. A single move that locks the direction,
680+
// re-opens the drawer, and writes the movement (200 - 80 = 120px of remaining travel) on the
681+
// commit that flips `open` back to true.
682+
fireEvent.pointerDown(swipeArea, {
683+
button: 0,
684+
buttons: 1,
685+
pointerId: 1,
686+
clientX: 10,
687+
clientY: 120,
688+
pointerType: 'mouse',
689+
});
690+
await flushMicrotasks();
691+
692+
fireEvent.pointerMove(swipeArea, {
693+
pointerId: 1,
694+
clientX: 10,
695+
clientY: 40,
696+
buttons: 1,
697+
pointerType: 'mouse',
698+
});
699+
await flushMicrotasks();
700+
701+
expect(swipeArea).toHaveAttribute('data-open', '');
702+
// The viewport's open-reset must not have clobbered the swipe area's value back to `0px`.
703+
expect(popup.style.getPropertyValue('--drawer-swipe-movement-y')).toBe('120px');
704+
} finally {
705+
globalThis.BASE_UI_ANIMATIONS_DISABLED = true;
706+
}
707+
},
708+
);
709+
615710
it('opens on a quick flick whose only move is already released (buttons: 0)', async () => {
616711
// On a fast flick — especially on a low-refresh-rate display — the pointer can be lifted before
617712
// the first `pointermove` is sampled, so the single move arrives with `buttons: 0` and no

packages/react/src/drawer/swipe-area/DrawerSwipeArea.tsx

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
'use client';
22
import * as React from 'react';
33
import { useStableCallback } from '@base-ui/utils/useStableCallback';
4+
import { useIsoLayoutEffect } from '@base-ui/utils/useIsoLayoutEffect';
45
import { ownerDocument } from '@base-ui/utils/owner';
56
import { useDialogRootContext } from '../../dialog/root/DialogRootContext';
67
import { useRenderElement } from '../../internals/useRenderElement';
@@ -91,7 +92,7 @@ export const DrawerSwipeArea = React.forwardRef(function DrawerSwipeArea(
9192
} = componentProps;
9293

9394
const { store } = useDialogRootContext();
94-
const { swipeDirection, frontmostHeight } = useDrawerRootContext();
95+
const { swipeDirection, frontmostHeight, swipeAreaActiveRef } = useDrawerRootContext();
9596
const providerContext = useDrawerProviderContext(true);
9697

9798
const [swipeActive, setSwipeActive] = React.useState(false);
@@ -257,6 +258,7 @@ export const DrawerSwipeArea = React.forwardRef(function DrawerSwipeArea(
257258
frontmostHeight: openProgress > 0 ? frontmostHeight : 0,
258259
});
259260
appliedSwipeStylesRef.current = true;
261+
swipeAreaActiveRef.current = true;
260262
}
261263

262264
const clearSwipeStyles = useStableCallback(() => {
@@ -281,6 +283,7 @@ export const DrawerSwipeArea = React.forwardRef(function DrawerSwipeArea(
281283

282284
providerContext?.visualStateStore.set({ swipeProgress: 0, frontmostHeight: 0 });
283285
appliedSwipeStylesRef.current = false;
286+
swipeAreaActiveRef.current = false;
284287
});
285288

286289
function openDrawer(event?: PointerEvent | TouchEvent) {
@@ -397,6 +400,15 @@ export const DrawerSwipeArea = React.forwardRef(function DrawerSwipeArea(
397400
const swipeTouchProps = swipe.getTouchProps();
398401
const resetSwipe = swipe.reset;
399402

403+
// The commit that opens the drawer re-renders the popup, resetting `--swipe-movement-*` to `0px`
404+
// (the viewport isn't swiping). Re-assert after the DOM mutation but before paint. No deps: must
405+
// run on every commit the swipe area participates in.
406+
useIsoLayoutEffect(() => {
407+
if (swipeActive && appliedSwipeStylesRef.current) {
408+
applySwipeMovement();
409+
}
410+
});
411+
400412
React.useEffect(() => {
401413
if (!enabled) {
402414
resetSwipe();

packages/react/src/drawer/viewport/DrawerViewport.tsx

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ export const DrawerViewport = React.forwardRef(function DrawerViewport(
8080
notifyParentSwipeProgressChange,
8181
frontmostHeight,
8282
snapToSequentialPoints,
83+
swipeAreaActiveRef,
8384
} = useDrawerRootContext();
8485
const providerContext = useDrawerProviderContext(true);
8586
const {
@@ -974,10 +975,16 @@ export const DrawerViewport = React.forwardRef(function DrawerViewport(
974975

975976
React.useEffect(() => {
976977
if (open) {
977-
resetSwipe();
978+
// Skip `resetSwipe` while `Drawer.SwipeArea` is driving the open: it zeroes the popup's
979+
// `--swipe-movement-*` (via `syncDragStyles(false)`), flashing it fully open for a frame.
980+
// `clearSwipeRelease` doesn't touch those vars, so always run it to clear any leftover
981+
// release state from a prior dismiss (e.g. when the popup is kept mounted).
982+
if (!swipeAreaActiveRef.current) {
983+
resetSwipe();
984+
}
978985
clearSwipeRelease();
979986
}
980-
}, [clearSwipeRelease, open, resetSwipe]);
987+
}, [clearSwipeRelease, open, resetSwipe, swipeAreaActiveRef]);
981988

982989
React.useEffect(() => {
983990
const backdropElement = backdropRef.current;

packages/react/src/utils/useSwipeDismiss.test.tsx

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -528,6 +528,94 @@ describe('useSwipeDismiss', () => {
528528
expect(element.style.getPropertyValue('--y')).toBe('40px');
529529
});
530530

531+
it('keeps the drag transform on a render that lags behind the swiping state', async () => {
532+
let initialGetDragStyles: (() => React.CSSProperties) | undefined;
533+
534+
function SwipeBoxCaptureStyles() {
535+
const ref = React.useRef<HTMLDivElement>(null);
536+
const swipe = useSwipeDismiss({
537+
enabled: true,
538+
directions: ['down'],
539+
elementRef: ref,
540+
movementCssVars: { x: '--x', y: '--y' },
541+
});
542+
543+
// Capture the latest `getDragStyles` from a render where the `isSwiping` state is still
544+
// false. This stands in for a render that commits during a gesture before `setSwiping(true)`
545+
// has flushed: its output must still mirror `isSwipingRef` so reconciling it onto the DOM
546+
// does not strip the transform the imperative writer set. Gating on `!swipe.swiping` (rather
547+
// than capturing only the very first render) keeps the reference tied to the surviving hook
548+
// instance under StrictMode's mount/unmount/remount.
549+
if (!swipe.swiping) {
550+
initialGetDragStyles = swipe.getDragStyles;
551+
}
552+
553+
return (
554+
<div
555+
data-testid="el"
556+
ref={ref}
557+
style={swipe.getDragStyles()}
558+
{...swipe.getPointerProps()}
559+
/>
560+
);
561+
}
562+
563+
await render(<SwipeBoxCaptureStyles />);
564+
const element = screen.getByTestId('el');
565+
566+
fireEvent.pointerDown(element, {
567+
button: 0,
568+
buttons: 1,
569+
pointerId: 1,
570+
clientX: 0,
571+
clientY: 0,
572+
bubbles: true,
573+
pointerType: 'mouse',
574+
movementX: 0,
575+
movementY: 0,
576+
});
577+
578+
await flushMicrotasks();
579+
580+
// The first move only establishes the drag baseline.
581+
fireEvent.pointerMove(element, {
582+
buttons: 1,
583+
pointerId: 1,
584+
clientX: 0,
585+
clientY: 0,
586+
bubbles: true,
587+
pointerType: 'mouse',
588+
movementX: 0,
589+
movementY: 0,
590+
});
591+
592+
await flushMicrotasks();
593+
594+
fireEvent.pointerMove(element, {
595+
buttons: 1,
596+
pointerId: 1,
597+
clientX: 0,
598+
clientY: 40,
599+
bubbles: true,
600+
pointerType: 'mouse',
601+
movementX: 0,
602+
movementY: 40,
603+
});
604+
605+
await flushMicrotasks();
606+
607+
// The gesture is active: `isSwipingRef` is true and the transform was written imperatively.
608+
expect(element.style.transition).toBe('none');
609+
610+
// The lagging render's `getDragStyles` (captured while the `isSwiping` state was false) must
611+
// still emit `transition: 'none'` and the current drag transform, otherwise React would drop
612+
// the popup to its resting transform for a frame mid-gesture.
613+
expect(initialGetDragStyles).toBeDefined();
614+
const laggingStyles = initialGetDragStyles?.() ?? {};
615+
expect(laggingStyles.transition).toBe('none');
616+
expect(laggingStyles.transform).toMatch(/translate3d\(0px, ?40px, ?0(?:px)?\)/);
617+
});
618+
531619
it('respects custom swipeThreshold', async () => {
532620
const onDismiss = vi.fn();
533621

packages/react/src/utils/useSwipeDismiss.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1057,27 +1057,31 @@ export function useSwipeDismiss(options: UseSwipeDismissOptions): UseSwipeDismis
10571057
});
10581058

10591059
const getDragStyles = React.useCallback((): React.CSSProperties => {
1060+
// Read `isSwipingRef`, not the lagging `isSwiping` state, to match the imperative writer
1061+
// `syncDragStyles`. Otherwise a render that commits before `setSwiping(true)` flushes strips the
1062+
// transform it just wrote, flashing the popup to its resting position for a frame.
1063+
const swiping = isSwipingRef.current;
10601064
const dragOffset = dragOffsetRef.current;
10611065
const initialTransform = initialTransformRef.current;
10621066
const deltaX = dragOffset.x - initialTransform.x;
10631067
const deltaY = dragOffset.y - initialTransform.y;
10641068

1065-
if (!isSwiping && deltaX === 0 && deltaY === 0 && !dragDismissed) {
1069+
if (!swiping && deltaX === 0 && deltaY === 0 && !dragDismissed) {
10661070
return {
10671071
[movementCssVars.x]: '0px',
10681072
[movementCssVars.y]: '0px',
10691073
} as React.CSSProperties;
10701074
}
10711075

10721076
return {
1073-
transition: isSwiping ? 'none' : undefined,
1077+
transition: swiping ? 'none' : undefined,
10741078
// While swiping, freeze the element at its current visual transform so it doesn't snap to the
10751079
// end position.
1076-
transform: isSwiping ? getDragTransform(dragOffset, initialTransform.scale) : undefined,
1080+
transform: swiping ? getDragTransform(dragOffset, initialTransform.scale) : undefined,
10771081
[movementCssVars.x]: `${deltaX}px`,
10781082
[movementCssVars.y]: `${deltaY}px`,
10791083
} as React.CSSProperties;
1080-
}, [dragDismissed, isSwiping, movementCssVars]);
1084+
}, [dragDismissed, movementCssVars]);
10811085

10821086
const getPointerProps = React.useCallback(() => {
10831087
if (!enabled) {

0 commit comments

Comments
 (0)