Skip to content

Commit 4c001cb

Browse files
authored
[Web] Keep detector-owned DOM props across a full config replace (#4522)
## Description `touchAction`, `userSelect` and `enableContextMenu` are props of `GestureDetector`, not of the gesture config. On web the detector sends them to the handler once, as a partial `updateGestureHandlerConfig`, when the handler attaches. Every full `setGestureHandlerConfig` (mount, and every re-render of the component that owns the gesture) runs `resetConfig` first, which cleared those three fields and, since #4488, refreshed the DOM to `touch-action: none` and `user-select: none`. The detector's prop-diff effect only re-sends when the props change, so nothing repaired it. Before #4488 the fields were already wiped, but the DOM was not refreshed, so it worked by accident. v2 is not affected: the legacy detector copies these props into each gesture's config, so the full set carries them. ## Test plan <details> <summary>Tested on the following code:</summary> ```tsx import React, { useEffect, useRef, useState } from 'react'; import { ScrollView, StyleSheet, Text, View } from 'react-native'; import { GestureDetector, Pressable, usePanGesture, } from 'react-native-gesture-handler'; import { COLORS } from '../../../common'; // Web only. The detector props `touchAction`, `userSelect` and // `enableContextMenu` are sent to the handler once, as a partial update, when // the detector attaches. Every full config set (mount, and each re-render of // the owner) runs `resetConfig` first, which clears them and refreshes the DOM // to the defaults. Expected after mount: the box reports // `touch-action: pan-y` and `user-select: text`, and the page scrolls // vertically over the box. Bug: `none` / `none`, no vertical scroll. type DomStyles = { inlineTouchAction: string; computedTouchAction: string; inlineUserSelect: string; computedUserSelect: string; }; function readDomStyles(node: unknown): DomStyles | null { if (typeof window === 'undefined' || !(node instanceof HTMLElement)) { return null; } const computed = window.getComputedStyle(node); return { inlineTouchAction: node.style.touchAction, computedTouchAction: computed.touchAction, inlineUserSelect: node.style.userSelect, computedUserSelect: computed.userSelect, }; } export default function WebDetectorPropsExample() { const [renders, setRenders] = useState(0); const [propsSet, setPropsSet] = useState(true); const [activations, setActivations] = useState(0); const [domStyles, setDomStyles] = useState<DomStyles | null>(null); const boxRef = useRef<React.ComponentRef<typeof View>>(null); const pan = usePanGesture({ activeOffsetX: [-10, 10], onActivate: () => { setActivations((n) => n + 1); }, }); useEffect(() => { const id = setInterval(() => { setDomStyles(readDomStyles(boxRef.current)); }, 250); return () => clearInterval(id); }, []); // With the props removed the handler falls back to its defaults. const expectedTouchAction = propsSet ? 'pan-y' : 'none'; const expectedUserSelect = propsSet ? 'text' : 'none'; const ok = domStyles?.computedTouchAction === expectedTouchAction && domStyles?.computedUserSelect === expectedUserSelect; return ( <ScrollView contentContainerStyle={styles.content}> <Text style={styles.label} testID="status"> {domStyles == null ? 'reading DOM…' : `touch-action: ${domStyles.computedTouchAction} (inline "${domStyles.inlineTouchAction}"), ` + `user-select: ${domStyles.computedUserSelect} (inline "${domStyles.inlineUserSelect}") ` + `→ ${ok ? 'OK' : 'BUG'}`} </Text> <Text style={styles.label}> renders: {renders}, pan activations: {activations}, detector props:{' '} {propsSet ? 'set (expect pan-y / text)' : 'unset (expect none / none)'} </Text> <Pressable style={styles.button} onPress={() => setRenders((n) => n + 1)} testID="rerender"> <Text style={styles.buttonLabel}>Re-render owner</Text> </Pressable> <Pressable style={styles.button} onPress={() => setPropsSet((v) => !v)} testID="toggleProps"> <Text style={styles.buttonLabel}> {propsSet ? 'Unset detector props' : 'Set detector props'} </Text> </Pressable> <GestureDetector gesture={pan} touchAction={propsSet ? 'pan-y' : undefined} userSelect={propsSet ? 'text' : undefined} enableContextMenu={propsSet ? false : undefined}> <View ref={boxRef} style={styles.box} testID="box"> <Text style={styles.boxLabel}> horizontal pan here, page should still scroll vertically </Text> </View> </GestureDetector> <Text style={styles.label}> scroll down: the page is taller than the screen on purpose </Text> <View style={styles.spacer} /> <Text style={styles.label}>end of page</Text> </ScrollView> ); } const styles = StyleSheet.create({ content: { alignItems: 'center', padding: 16, }, label: { fontSize: 14, color: COLORS.NAVY, marginVertical: 4, textAlign: 'center', }, button: { marginVertical: 6, paddingHorizontal: 20, paddingVertical: 10, borderRadius: 8, backgroundColor: COLORS.PURPLE, }, buttonLabel: { color: 'white', fontSize: 16, }, spacer: { height: 1200, }, box: { marginTop: 12, width: 260, height: 200, borderRadius: 16, backgroundColor: COLORS.GREEN, justifyContent: 'center', alignItems: 'center', padding: 12, }, boxLabel: { color: 'white', fontSize: 16, textAlign: 'center', }, }); ``` </details>
1 parent 5d1c171 commit 4c001cb

5 files changed

Lines changed: 124 additions & 43 deletions

File tree

‎packages/react-native-gesture-handler/src/v3/detectors/HostGestureDetector.web.tsx‎

Lines changed: 40 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@ import type {
99
} from '../../handlers/gestureHandlerCommon';
1010
import RNGestureHandlerModule from '../../RNGestureHandlerModule.web';
1111
import { tagMessage } from '../../utils';
12+
import {
13+
DEFAULT_ENABLE_CONTEXT_MENU,
14+
DEFAULT_TOUCH_ACTION,
15+
DEFAULT_USER_SELECT,
16+
} from '../../web/constants';
1217
import { type PropsRef } from '../../web/interfaces';
1318
import NodeManager from '../../web/tools/NodeManager';
1419
import { useNativeGestureRole } from './useNativeGestureRole';
@@ -57,6 +62,21 @@ type DetectorRefs = {
5762
// Invoked from `NodeManager.observeHandler` once the handler is known to exist. Branches on
5863
// handler kind + actionType to pick the right binding flow. May be called multiple times for
5964
// the same tag (handler re-registration), so each branch must be idempotent.
65+
// The detector owns these props. It always sends concrete values so that
66+
// removing a prop takes the handler back to the default instead of leaving the
67+
// previous value in place (partial updates skip `undefined` keys).
68+
function unpackDOMProps(source: {
69+
userSelect?: UserSelect | undefined;
70+
touchAction?: TouchAction | undefined;
71+
enableContextMenu?: boolean | undefined;
72+
}) {
73+
return {
74+
userSelect: source.userSelect ?? DEFAULT_USER_SELECT,
75+
touchAction: source.touchAction ?? DEFAULT_TOUCH_ACTION,
76+
enableContextMenu: source.enableContextMenu ?? DEFAULT_ENABLE_CONTEXT_MENU,
77+
};
78+
}
79+
6080
function attachReadyHandler(
6181
refs: DetectorRefs,
6282
tag: number,
@@ -99,11 +119,10 @@ function attachReadyHandler(
99119
refs.attachedHandlers.add(tag);
100120
}
101121

102-
RNGestureHandlerModule.updateGestureHandlerConfig(tag, {
103-
userSelect: child.userSelect,
104-
touchAction: child.touchAction,
105-
enableContextMenu: child.enableContextMenu,
106-
});
122+
RNGestureHandlerModule.updateGestureHandlerConfig(
123+
tag,
124+
unpackDOMProps(child)
125+
);
107126
return;
108127
}
109128

@@ -122,11 +141,10 @@ function attachReadyHandler(
122141
refs.attachedHandlers.add(tag);
123142
}
124143

125-
RNGestureHandlerModule.updateGestureHandlerConfig(tag, {
126-
userSelect: refs.propsRef.current.userSelect,
127-
touchAction: refs.propsRef.current.touchAction,
128-
enableContextMenu: refs.propsRef.current.enableContextMenu,
129-
});
144+
RNGestureHandlerModule.updateGestureHandlerConfig(
145+
tag,
146+
unpackDOMProps(refs.propsRef.current)
147+
);
130148
}
131149

132150
function tryAttachNativeHandlersToChildView(refs: DetectorRefs) {
@@ -169,11 +187,10 @@ function tryAttachNativeHandlersToChildView(refs: DetectorRefs) {
169187
refs.viewRef
170188
);
171189
refs.attachedHandlers.add(tag);
172-
RNGestureHandlerModule.updateGestureHandlerConfig(tag, {
173-
userSelect: refs.propsRef.current.userSelect,
174-
touchAction: refs.propsRef.current.touchAction,
175-
enableContextMenu: refs.propsRef.current.enableContextMenu,
176-
});
190+
RNGestureHandlerModule.updateGestureHandlerConfig(
191+
tag,
192+
unpackDOMProps(refs.propsRef.current)
193+
);
177194
}
178195
}
179196

@@ -270,11 +287,10 @@ const HostGestureDetector = (props: GestureHandlerDetectorProps) => {
270287
.difference(claimedByVirtual);
271288

272289
for (const tag of handlersToUpdate) {
273-
RNGestureHandlerModule.updateGestureHandlerConfig(tag, {
274-
userSelect: props.userSelect,
275-
touchAction: props.touchAction,
276-
enableContextMenu: props.enableContextMenu,
277-
});
290+
RNGestureHandlerModule.updateGestureHandlerConfig(
291+
tag,
292+
unpackDOMProps(props)
293+
);
278294
}
279295
}
280296
}, [props, refs]);
@@ -334,11 +350,10 @@ const HostGestureDetector = (props: GestureHandlerDetectorProps) => {
334350
// applied in `attachReadyHandler`, so this is a no-op for them.
335351
for (const tag of subs) {
336352
if (refs.attachedHandlers.has(tag)) {
337-
RNGestureHandlerModule.updateGestureHandlerConfig(tag, {
338-
userSelect: child.userSelect,
339-
touchAction: child.touchAction,
340-
enableContextMenu: child.enableContextMenu,
341-
});
353+
RNGestureHandlerModule.updateGestureHandlerConfig(
354+
tag,
355+
unpackDOMProps(child)
356+
);
342357
}
343358
}
344359
});
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,9 @@
11
export const DEFAULT_TOUCH_SLOP = 15;
22
export const MINIMAL_RECOGNIZABLE_MAGNITUDE = 0.1;
33
export const NATIVE_GESTURE_ROLE_ATTRIBUTE = 'rngh-role';
4+
5+
// Values a detector applies to its DOM element when the corresponding prop is
6+
// not set.
7+
export const DEFAULT_TOUCH_ACTION = 'none';
8+
export const DEFAULT_USER_SELECT = 'none';
9+
export const DEFAULT_ENABLE_CONTEXT_MENU = false;

‎packages/react-native-gesture-handler/src/web/handlers/GestureHandler.ts‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -959,10 +959,17 @@ export default abstract class GestureHandler implements IGestureHandler {
959959
this.needsPointerData = false;
960960
this.forAnimated = false;
961961
this.forReanimated = false;
962-
this.enableContextMenu = false;
963962
this._activeCursor = undefined;
964-
this._touchAction = undefined;
965-
this._userSelect = undefined;
963+
964+
// On the v3 path these belong to the detector, which sends them (or their
965+
// defaults) through partial updates only. Legacy handlers carry them in the
966+
// full config and still get the reset.
967+
if (!this.usesNativeOrVirtualDetector()) {
968+
this.enableContextMenu = false;
969+
this._touchAction = undefined;
970+
this._userSelect = undefined;
971+
}
972+
966973
this.needsDOMUpdate = true;
967974
}
968975

‎packages/react-native-gesture-handler/src/web/handlers/__tests__/GestureHandler.test.ts‎

Lines changed: 60 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,11 @@
1+
import { ActionType } from '../../../ActionType';
12
import { PointerType } from '../../../PointerType';
23
import { State } from '../../../State';
4+
import {
5+
DEFAULT_ENABLE_CONTEXT_MENU,
6+
DEFAULT_TOUCH_ACTION,
7+
DEFAULT_USER_SELECT,
8+
} from '../../constants';
39
import type { AdaptedEvent, Config } from '../../interfaces';
410
import { EventTypes } from '../../interfaces';
511
import EventManager from '../../tools/EventManager';
@@ -9,7 +15,11 @@ import { GestureHandlerWebDelegate } from '../../tools/GestureHandlerWebDelegate
915
import GestureHandler from '../GestureHandler';
1016
import type IGestureHandler from '../IGestureHandler';
1117

12-
class TestGestureHandler extends GestureHandler {}
18+
class TestGestureHandler extends GestureHandler {
19+
public markAsNativeDetector() {
20+
this.actionType = ActionType.NATIVE_DETECTOR;
21+
}
22+
}
1323

1424
class TestEventManager extends EventManager<unknown> {
1525
public registerListeners = jest.fn();
@@ -41,7 +51,7 @@ describe('GestureHandler web config reset', () => {
4151
expect(handler.enabled).toBe(true);
4252
});
4353

44-
test('a config without touchAction refreshes the DOM', () => {
54+
test('a full config replace keeps the detector props and refreshes the DOM', () => {
4555
const delegate = {
4656
onEnabledChange: jest.fn(),
4757
updateDOM: jest.fn(),
@@ -50,14 +60,59 @@ describe('GestureHandler web config reset', () => {
5060
const handler = new TestGestureHandler(
5161
delegate as unknown as GestureHandlerDelegate<unknown, IGestureHandler>
5262
);
53-
handler.setGestureConfig({ enabled: true, touchAction: 'pan-y' });
54-
63+
handler.markAsNativeDetector();
64+
// The detector sends its props once, as a partial update.
65+
handler.updateGestureConfig({
66+
touchAction: 'pan-y',
67+
userSelect: 'text',
68+
enableContextMenu: true,
69+
});
70+
71+
// The owner re-sends the full gesture config, which never carries them.
5572
handler.setGestureConfig({ enabled: true });
5673

57-
expect(handler.touchAction).toBeUndefined();
74+
expect(handler.touchAction).toBe('pan-y');
75+
expect(handler.userSelect).toBe('text');
76+
expect(handler.enableContextMenu).toBe(true);
77+
// The partial update also enabled the handler, so it went through
78+
// `onEnabledChange`; only the full replace refreshes the DOM directly.
79+
expect(delegate.onEnabledChange).toHaveBeenCalledTimes(1);
5880
expect(delegate.updateDOM).toHaveBeenCalledTimes(1);
5981
});
6082

83+
test('the detector resets the props it set by sending the defaults', () => {
84+
const delegate = {
85+
onEnabledChange: jest.fn(),
86+
updateDOM: jest.fn(),
87+
reset: jest.fn(),
88+
};
89+
const handler = new TestGestureHandler(
90+
delegate as unknown as GestureHandlerDelegate<unknown, IGestureHandler>
91+
);
92+
handler.markAsNativeDetector();
93+
handler.setGestureConfig({ enabled: true });
94+
handler.updateGestureConfig({
95+
touchAction: 'pan-y',
96+
userSelect: 'text',
97+
enableContextMenu: true,
98+
});
99+
100+
// The detector re-sends all three whenever one of them changes, with the
101+
// defaults standing in for props that are no longer set.
102+
handler.updateGestureConfig({
103+
touchAction: DEFAULT_TOUCH_ACTION,
104+
userSelect: DEFAULT_USER_SELECT,
105+
enableContextMenu: DEFAULT_ENABLE_CONTEXT_MENU,
106+
});
107+
108+
expect(handler.touchAction).toBe('none');
109+
expect(handler.userSelect).toBe('none');
110+
expect(handler.enableContextMenu).toBe(false);
111+
// The initial full set enabled the handler (`onEnabledChange`); the two
112+
// partial updates refresh the DOM directly.
113+
expect(delegate.updateDOM).toHaveBeenCalledTimes(2);
114+
});
115+
61116
test('the web delegate ignores DOM updates before init', () => {
62117
expect(() => new GestureHandlerWebDelegate().updateDOM()).not.toThrow();
63118
});

‎packages/react-native-gesture-handler/src/web/tools/GestureHandlerWebDelegate.ts‎

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { MouseButton } from '../../handlers/gestureHandlerCommon';
33
import { State } from '../../State';
44
import { tagMessage } from '../../utils';
55
import { SingleGestureName } from '../../v3/types';
6+
import { DEFAULT_TOUCH_ACTION, DEFAULT_USER_SELECT } from '../constants';
67
import type IGestureHandler from '../handlers/IGestureHandler';
78
import type { SVGRef } from '../interfaces';
89
import {
@@ -264,7 +265,7 @@ export class GestureHandlerWebDelegate
264265
this.ensureView(this.view);
265266

266267
const value = this.gestureHandler.enabled
267-
? (userSelect ?? 'none')
268+
? (userSelect ?? DEFAULT_USER_SELECT)
268269
: this.defaultViewStyles.userSelect;
269270

270271
this.setViewStyle('userSelect', value);
@@ -277,24 +278,21 @@ export class GestureHandlerWebDelegate
277278
this.ensureView(this.view);
278279

279280
const value = this.gestureHandler.enabled
280-
? (touchAction ?? 'none')
281+
? (touchAction ?? DEFAULT_TOUCH_ACTION)
281282
: this.defaultViewStyles.touchAction;
282283

283284
this.setViewStyle('touchAction', value);
284285
this.setViewStyle('WebkitTouchCallout', value);
285286
}
286287

287288
private setContextMenu() {
288-
if (!this.gestureHandler.enabled) {
289-
this.removeContextMenuListeners();
290-
return;
291-
}
289+
// Enabling and disabling the context menu use different listeners, so
290+
// drop the current one before attaching the right one.
291+
this.removeContextMenuListeners();
292292

293-
if (!this.wasContextMenuEnabled) {
294-
this.removeContextMenuListeners();
293+
if (this.gestureHandler.enabled) {
294+
this.addContextMenuListeners();
295295
}
296-
297-
this.addContextMenuListeners();
298296
}
299297

300298
onEnabledChange(): void {

0 commit comments

Comments
 (0)