Skip to content

Commit d194d8b

Browse files
LESANFmeta-codesync[bot]
authored andcommitted
Stop remounting the app when toggling Touchables in the Element Inspector (#58705)
Summary: Pressing **Touchables** in the Element Inspector remounts the whole app. All component state is lost, including navigation state, so the app jumps back to its first screen, away from the screen whose press targets you wanted to see. While Inspect is on, touches are intercepted, so you can't navigate back either. The remount is how the toggle is applied. `PressabilityDebug` keeps the flag in a module variable that React can't observe, so `Inspector` asks `AppContainer` to change the `key` of the root view: ```js const setTouchTargeting = (val: boolean) => { PressabilityDebug.setEnabled(val); onRequestRerenderApp(); // AppContainer: setKey(k => k + 1) }; ``` This has been in place since d5c1de7 (2016). The only related report I found, #26354, was closed with a workaround. This change: - makes `PressabilityDebug` notify subscribers when the flag changes, and adds `useIsEnabled()`, which reads it with `useSyncExternalStore` - uses it in `PressabilityDebugView`, which `Pressable` and every `Touchable*` already render in `__DEV__`, so those components don't change - moves the magenta color of pressable `Text` into `PressableText` / `PressableVirtualText`, so `Text` that isn't pressable gets no extra hook - uses it in the Inspector panel for the state of the Touchables button - removes `onRequestRerenderApp`, and the root `key` in `AppContainer` that only existed for it In production `useIsEnabled` is `isEnabled`, so no hooks are added. No public API changes. One difference: `Text` with both `onPress` and `disabled` no longer turns magenta, since it isn't pressable. ## Changelog: [GENERAL] [FIXED] - Toggling Touchables in the Element Inspector no longer remounts the app and resets its state Pull Request resolved: #58705 Test Plan: | Before | After | | --- | --- | | <video src="https://github.com/user-attachments/assets/bb8b819b-256d-4ecf-9504-da974af3ad31" width="260"></video> | <video src="https://github.com/user-attachments/assets/23048e00-e252-467b-85d6-9503cfd10e56" width="260"></video> | Before, the app remounts and goes back to its first screen. After, it stays on the screen and the press targets are outlined. Recorded in an Expo SDK 57 app (React Native 0.86.3, New Architecture) with this change applied as a patch: open a screen other than the first one, then Dev Menu → Toggle Element Inspector → **Touchables**. The new Fantom test `PressabilityDebug-itest.js` checks that the press target outline appears and disappears without remounting, that pressable `Text` (top-level and nested) turns magenta, and that disabled `Text` does not. All four tests fail without this change. - `yarn fantom PressabilityDebug-itest` → 4 passed - `yarn fantom` → 182 suites passed, 22 skipped, 0 failed - `yarn test packages/react-native` → 146 suites, 5232 tests passed - `yarn flow-check` → 0 errors - `yarn lint`, `yarn format-check-javascript` → clean - `yarn build-types` → `ReactNativeApi.d.ts` unchanged Reviewed By: javache Differential Revision: D122144366 Pulled By: fabriziocucci fbshipit-source-id: 8e6a204a7ca3814ad0a34d09211f7e27985f7b34
1 parent f1d97ff commit d194d8b

5 files changed

Lines changed: 196 additions & 22 deletions

File tree

‎packages/react-native/Libraries/Pressability/PressabilityDebug.js‎

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,8 +36,9 @@ type Props = Readonly<{
3636
*
3737
*/
3838
export function PressabilityDebugView(props: Props): React.Node {
39+
const enabled = useIsEnabled();
3940
if (__DEV__) {
40-
if (isEnabled()) {
41+
if (enabled) {
4142
const normalizedColor = normalizeColor(props.color);
4243
if (typeof normalizedColor !== 'number') {
4344
return null;
@@ -70,6 +71,7 @@ export function PressabilityDebugView(props: Props): React.Node {
7071
}
7172

7273
let isDebugEnabled = false;
74+
const listeners: Set<() => void> = new Set();
7375

7476
export function isEnabled(): boolean {
7577
if (__DEV__) {
@@ -80,6 +82,30 @@ export function isEnabled(): boolean {
8082

8183
export function setEnabled(value: boolean): void {
8284
if (__DEV__) {
85+
if (isDebugEnabled === value) {
86+
return;
87+
}
8388
isDebugEnabled = value;
89+
listeners.forEach(listener => listener());
8490
}
8591
}
92+
93+
function subscribe(listener: () => void): () => void {
94+
listeners.add(listener);
95+
return () => {
96+
listeners.delete(listener);
97+
};
98+
}
99+
100+
function useIsEnabledDev(): boolean {
101+
return React.useSyncExternalStore(subscribe, isEnabled);
102+
}
103+
104+
/**
105+
* Like `isEnabled`, but re-renders the calling component when the value
106+
* changes, so toggling it does not require remounting the app. Outside of
107+
* `__DEV__` it always returns `false` and uses no hooks.
108+
*/
109+
export const useIsEnabled: () => boolean = __DEV__
110+
? useIsEnabledDev
111+
: () => false;
Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
/**
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @flow strict-local
8+
* @format
9+
* @oncall react_native
10+
*/
11+
12+
import '@react-native/fantom/src/setUpDefaultReactNativeEnvironment';
13+
14+
import * as PressabilityDebug from '../PressabilityDebug';
15+
import * as Fantom from '@react-native/fantom';
16+
import * as React from 'react';
17+
import {useEffect} from 'react';
18+
import {Pressable, Text} from 'react-native';
19+
20+
describe('PressabilityDebug', () => {
21+
beforeEach(() => {
22+
Fantom.runTask(() => {
23+
PressabilityDebug.setEnabled(false);
24+
});
25+
});
26+
27+
afterEach(() => {
28+
Fantom.runTask(() => {
29+
PressabilityDebug.setEnabled(false);
30+
});
31+
});
32+
33+
it('shows press targets when enabled, without remounting', () => {
34+
const root = Fantom.createRoot();
35+
let mountCount = 0;
36+
37+
function Screen() {
38+
useEffect(() => {
39+
mountCount++;
40+
}, []);
41+
return <Pressable style={{height: 10}} />;
42+
}
43+
44+
Fantom.runTask(() => {
45+
root.render(<Screen />);
46+
});
47+
48+
expect(root.getRenderedOutput({props: []}).toJSX()).toEqual(<rn-view />);
49+
50+
Fantom.runTask(() => {
51+
PressabilityDebug.setEnabled(true);
52+
});
53+
54+
expect(root.getRenderedOutput({props: []}).toJSX()).toEqual(
55+
<rn-view>
56+
<rn-view />
57+
</rn-view>,
58+
);
59+
60+
Fantom.runTask(() => {
61+
PressabilityDebug.setEnabled(false);
62+
});
63+
64+
expect(root.getRenderedOutput({props: []}).toJSX()).toEqual(<rn-view />);
65+
expect(mountCount).toBe(1);
66+
});
67+
68+
it('colors pressable text when enabled', () => {
69+
const root = Fantom.createRoot();
70+
71+
Fantom.runTask(() => {
72+
root.render(<Text onPress={() => {}}>text</Text>);
73+
});
74+
75+
expect(
76+
root.getRenderedOutput({props: ['foregroundColor']}).toJSX(),
77+
).toEqual(
78+
<rn-paragraph foregroundColor="rgba(0, 0, 0, 0)">text</rn-paragraph>,
79+
);
80+
81+
Fantom.runTask(() => {
82+
PressabilityDebug.setEnabled(true);
83+
});
84+
85+
expect(
86+
root.getRenderedOutput({props: ['foregroundColor']}).toJSX(),
87+
).toEqual(
88+
<rn-paragraph foregroundColor="rgba(255, 0, 255, 1)">text</rn-paragraph>,
89+
);
90+
});
91+
92+
it('colors nested pressable text when enabled', () => {
93+
const root = Fantom.createRoot();
94+
95+
Fantom.runTask(() => {
96+
root.render(
97+
<Text>
98+
<Text onPress={() => {}}>nested</Text>
99+
</Text>,
100+
);
101+
});
102+
103+
Fantom.runTask(() => {
104+
PressabilityDebug.setEnabled(true);
105+
});
106+
107+
expect(
108+
root.getRenderedOutput({props: ['foregroundColor']}).toJSX(),
109+
).toEqual(
110+
<rn-paragraph foregroundColor="rgba(0, 0, 0, 0)">
111+
<rn-text foregroundColor="rgba(255, 0, 255, 1)">nested</rn-text>
112+
</rn-paragraph>,
113+
);
114+
});
115+
116+
it('does not color disabled text', () => {
117+
const root = Fantom.createRoot();
118+
119+
Fantom.runTask(() => {
120+
PressabilityDebug.setEnabled(true);
121+
});
122+
123+
Fantom.runTask(() => {
124+
root.render(
125+
<Text disabled onPress={() => {}}>
126+
text
127+
</Text>,
128+
);
129+
});
130+
131+
expect(
132+
root.getRenderedOutput({props: ['foregroundColor']}).toJSX(),
133+
).toEqual(
134+
<rn-paragraph foregroundColor="rgba(0, 0, 0, 0)">text</rn-paragraph>,
135+
);
136+
});
137+
});

‎packages/react-native/Libraries/ReactNative/AppContainer-dev.js‎

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ import {RootTagContext, createRootTag} from './RootTag';
2727
import * as React from 'react';
2828
import {useRef} from 'react';
2929

30-
const {useEffect, useState, useCallback} = React;
30+
const {useEffect, useState} = React;
3131

3232
const reactDevToolsHook: ReactDevToolsGlobalHook = (window as $FlowFixMe)
3333
.__REACT_DEVTOOLS_GLOBAL_HOOK__;
@@ -51,15 +51,13 @@ type ExternalInspection = {
5151

5252
type InspectorDeferredProps = {
5353
inspectedViewRef: InspectedViewRef,
54-
onInspectedViewRerenderRequest: () => void,
5554
reactDevToolsAgent?: ReactDevToolsAgent,
5655
devMenuInspectorOpen: boolean,
5756
externalInspection: ExternalInspection,
5857
};
5958

6059
const InspectorDeferred = ({
6160
inspectedViewRef,
62-
onInspectedViewRerenderRequest,
6361
reactDevToolsAgent,
6462
devMenuInspectorOpen,
6563
externalInspection,
@@ -72,7 +70,6 @@ const InspectorDeferred = ({
7270
return (
7371
<Inspector
7472
inspectedViewRef={inspectedViewRef}
75-
onRequestRerenderApp={onInspectedViewRerenderRequest}
7673
reactDevToolsAgent={reactDevToolsAgent}
7774
devMenuInspectorOpen={devMenuInspectorOpen}
7875
externalInspection={externalInspection}
@@ -118,7 +115,6 @@ const AppContainer = ({
118115
debuggingOverlayRef,
119116
);
120117

121-
const [key, setKey] = useState(0);
122118
const [shouldRenderInspector, setShouldRenderInspector] = useState(false);
123119
const [reactDevToolsAgent, setReactDevToolsAgent] =
124120
useState<ReactDevToolsAgent | void>(reactDevToolsHook?.reactDevtoolsAgent);
@@ -157,7 +153,6 @@ const AppContainer = ({
157153
<View
158154
collapsable={reactDevToolsAgent == null && !shouldRenderInspector}
159155
pointerEvents="box-none"
160-
key={key}
161156
style={rootViewStyle || styles.container}
162157
ref={innerViewRef}>
163158
{children}
@@ -172,11 +167,6 @@ const AppContainer = ({
172167
);
173168
}
174169

175-
const onInspectedViewRerenderRequest = useCallback(
176-
() => setKey(k => k + 1),
177-
[],
178-
);
179-
180170
return (
181171
<RootTagContext.Provider value={createRootTag(rootTag)}>
182172
<View
@@ -198,7 +188,6 @@ const AppContainer = ({
198188
externalInspection.externalInspectingEnabled) && (
199189
<InspectorDeferred
200190
inspectedViewRef={innerViewRef}
201-
onInspectedViewRerenderRequest={onInspectedViewRerenderRequest}
202191
reactDevToolsAgent={reactDevToolsAgent}
203192
devMenuInspectorOpen={shouldRenderInspector}
204193
externalInspection={externalInspection}

‎packages/react-native/Libraries/Text/Text.js‎

Lines changed: 29 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -166,11 +166,6 @@ const TextImpl: component(
166166
selectionColor != null ? processColor(selectionColor) : undefined;
167167

168168
let _style = style;
169-
if (__DEV__) {
170-
if (PressabilityDebug.isEnabled() && onPress != null) {
171-
_style = [style, {color: 'magenta'}];
172-
}
173-
}
174169

175170
let _numberOfLines = numberOfLines;
176171
if (_numberOfLines != null && !(_numberOfLines >= 0)) {
@@ -471,6 +466,24 @@ function useTextPressability({
471466
);
472467
}
473468

469+
function usePressabilityDebugStyleDev(
470+
style: ?TextStyleProp,
471+
onPress: ?(event: GestureResponderEvent) => unknown,
472+
): ?TextStyleProp {
473+
const isDebugEnabled = PressabilityDebug.useIsEnabled();
474+
return isDebugEnabled && onPress != null
475+
? [style, {color: 'magenta'}]
476+
: style;
477+
}
478+
479+
/**
480+
* Colors pressable text when press targets are shown by the Inspector.
481+
* Outside of `__DEV__` it returns the style unchanged and uses no hooks.
482+
*/
483+
const usePressabilityDebugStyle: typeof usePressabilityDebugStyleDev = __DEV__
484+
? usePressabilityDebugStyleDev
485+
: style => style;
486+
474487
/**
475488
* Wrap the NativeVirtualText component and initialize pressability.
476489
*
@@ -485,10 +498,15 @@ component PressableVirtualText(
485498
const [isHighlighted, eventHandlersForText] = useTextPressability(
486499
textPressabilityProps,
487500
);
501+
const style = usePressabilityDebugStyle(
502+
textProps.style,
503+
textPressabilityProps.onPress,
504+
);
488505

489506
return (
490507
<NativeVirtualText
491508
{...textProps}
509+
style={style}
492510
{...eventHandlersForText}
493511
isHighlighted={isHighlighted}
494512
isPressable={true}
@@ -513,12 +531,18 @@ component PressableText(
513531
textPressabilityProps,
514532
);
515533

534+
const style = usePressabilityDebugStyle(
535+
textProps.style,
536+
textPressabilityProps.onPress,
537+
);
538+
516539
const NativeComponent =
517540
selectable === true ? NativeSelectableText : NativeText;
518541

519542
return (
520543
<NativeComponent
521544
{...textProps}
545+
style={style}
522546
{...eventHandlersForText}
523547
isHighlighted={isHighlighted}
524548
isPressable={true}

‎packages/react-native/src/private/devsupport/devmenu/elementinspector/Inspector.js‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -56,15 +56,13 @@ type ExternalInspection = {
5656

5757
type Props = {
5858
inspectedViewRef: InspectedViewRef,
59-
onRequestRerenderApp: () => void,
6059
reactDevToolsAgent?: ReactDevToolsAgent,
6160
devMenuInspectorOpen: boolean,
6261
externalInspection: ExternalInspection,
6362
};
6463

6564
function Inspector({
6665
inspectedViewRef,
67-
onRequestRerenderApp,
6866
reactDevToolsAgent,
6967
devMenuInspectorOpen,
7068
externalInspection,
@@ -79,6 +77,7 @@ function Inspector({
7977
const [selectionIndex, setSelectionIndex] = useState<?number>(null);
8078
const [elementsHierarchy, setElementsHierarchy] =
8179
useState<?ElementsHierarchy>(null);
80+
const touchTargeting = PressabilityDebug.useIsEnabled();
8281

8382
// Derive inspecting state: external inspection forces it on, otherwise use local state
8483
const isInspecting = externalInspectingEnabled || inspectingEnabled;
@@ -160,7 +159,6 @@ function Inspector({
160159

161160
const setTouchTargeting = (val: boolean) => {
162161
PressabilityDebug.setEnabled(val);
163-
onRequestRerenderApp();
164162
};
165163

166164
const panelContainerStyle =
@@ -188,7 +186,7 @@ function Inspector({
188186
hierarchy={elementsHierarchy}
189187
selection={selectionIndex}
190188
setSelection={setSelection}
191-
touchTargeting={PressabilityDebug.isEnabled()}
189+
touchTargeting={touchTargeting}
192190
setTouchTargeting={setTouchTargeting}
193191
/>
194192
</SafeAreaView>

0 commit comments

Comments
 (0)