Skip to content

Commit 997d44c

Browse files
authored
fix(iOS): pressables in header not working when hosting screen has modal presentation (#2915)
## Description Fixes #2842 `RNSModalScreen` sends non-zero contentOffsetY to its shadow node. This is wrong, because its content in HostTree is not offset by any means, as there is no associated navigation bar. I do not know, why we do sent non-zero header height there. I guess that this case was overlooked in #2028. Above behaviour leads to discrepancies between HostTree & ShadowTree positions of descendant views, causing problems with pressables. ## Changes <!-- Please describe things you've changed here, make a **high level** overview, if change is simple you can omit this section. For example: - Updated `about.md` docs --> <!-- ## Screenshots / GIFs Here you can add screenshots / GIFs documenting your change. You can add before / after section if you're changing some behavior. ### Before ### After --> ## Test code and steps to reproduce <!-- Please include code that can be used to test this change and short description how this example should work. This snippet should be as minimal as possible and ready to be pasted into editor (don't exclude exports or remove "not important" parts of reproduction example) --> Added `Test2915` to test exactly this case. ## Checklist - [x] Included code example that can be used to test this change - [x] Ensured that CI passes
1 parent 69f9913 commit 997d44c

File tree

4 files changed

+138
-4
lines changed

4 files changed

+138
-4
lines changed
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import { device, expect, element, by } from 'detox';
2+
import { describeIfiOS } from '../e2e-utils';
3+
4+
describeIfiOS('Test2842 - pressables in modal', () => {
5+
beforeAll(async () => {
6+
await device.reloadReactNative();
7+
});
8+
9+
it('Test2842 should exist', async () => {
10+
await waitFor(element(by.id('root-screen-tests-Test2842')))
11+
.toBeVisible()
12+
.whileElement(by.id('root-screen-examples-scrollview'))
13+
.scroll(600, 'down', NaN, 0.85);
14+
15+
await expect(element(by.id('root-screen-tests-Test2842'))).toBeVisible();
16+
await element(by.id('root-screen-tests-Test2842')).tap();
17+
});
18+
19+
it('Modal should open', async () => {
20+
const openModalButtonElement = element(by.id('home-button-open-modal'));
21+
await expect(openModalButtonElement).toExist();
22+
await openModalButtonElement.tap();
23+
24+
// Verify that the modal has opened by checking that the "close" button is visible
25+
const closeModalButtonElement = element(by.id('modal-button-close'));
26+
await expect(closeModalButtonElement).toBeVisible();
27+
});
28+
29+
it('HeaderRight subview should be visible', async () => {
30+
const headerRightElement = element(by.id('subview-headerright'));
31+
await expect(headerRightElement).toBeVisible(100);
32+
});
33+
34+
it('HeaderRight should be pressable', async () => {
35+
const headerRightElement = element(by.id('subview-headerright'));
36+
await headerRightElement.tap();
37+
38+
// These are rendered under the modal, therefore they are not visible at the first glance.
39+
await expect(element(by.text('1. onPressIn'))).toExist();
40+
await expect(element(by.text('2. onPress'))).toExist();
41+
await expect(element(by.text('3. onPressOut'))).toExist();
42+
});
43+
44+
it('HeaderRight should not lose focus on swipe', async () => {
45+
const headerRightElement = element(by.id('subview-headerright'));
46+
47+
await headerRightElement.swipe('right', 'slow', 0.15, 0.1, 0.5);
48+
49+
// If the element has lost focus, it wouldn't fire onPress.
50+
// Note that when swiping the event order is different - but this is RN behaviour.
51+
await expect(element(by.text('4. onPressIn'))).toExist();
52+
await expect(element(by.text('5. onPressOut'))).toExist();
53+
await expect(element(by.text('6. onPress'))).toExist();
54+
});
55+
});

apps/src/tests/Test2842.tsx

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
import { NavigationContainer, ParamListBase } from '@react-navigation/native';
2+
import { NativeStackNavigationProp, createNativeStackNavigator } from '@react-navigation/native-stack';
3+
import React from 'react';
4+
import { styles } from '../shared/styles';
5+
import { Button, Text, View } from 'react-native';
6+
import { Rectangle } from '../shared/Rectangle';
7+
import PressableWithFeedback from '../shared/PressableWithFeedback';
8+
import { ToastProvider, useToast } from '../shared';
9+
10+
interface StackParamList extends ParamListBase {
11+
Home: undefined;
12+
Modal: undefined;
13+
}
14+
15+
interface StackNavigationProps {
16+
navigation: NativeStackNavigationProp<StackParamList>;
17+
}
18+
19+
const Stack = createNativeStackNavigator<StackParamList>();
20+
21+
function Home({ navigation }: StackNavigationProps) {
22+
return (
23+
<View style={[styles.flexContainer, { backgroundColor: 'darkorange' }]}>
24+
<Text>Home</Text>
25+
<Button title="Open modal" onPress={() => navigation.navigate('Modal')} testID="home-button-open-modal" />
26+
</View>
27+
);
28+
}
29+
30+
function Modal({ navigation }: StackNavigationProps) {
31+
return (
32+
<View style={[styles.flexContainer, { backgroundColor: 'lightblue' }]}>
33+
<Text>Modal</Text>
34+
<Button title="Close modal" onPress={() => navigation.pop()} testID="modal-button-close" />
35+
</View>
36+
);
37+
}
38+
39+
function HeaderRight() {
40+
const toast = useToast();
41+
42+
return (
43+
<PressableWithFeedback
44+
onPress={() => {
45+
toast.push({ backgroundColor: 'blue', message: 'onPress' });
46+
}}
47+
onPressIn={() => {
48+
toast.push({ backgroundColor: 'blue', message: 'onPressIn' });
49+
}}
50+
onPressOut={() => {
51+
toast.push({ backgroundColor: 'blue', message: 'onPressOut' });
52+
}}
53+
>
54+
<Rectangle width={128} height={36} color={'lightgreen'} style={{ borderRadius: 16 }} testID="subview-headerright" />
55+
</PressableWithFeedback>
56+
);
57+
}
58+
59+
function App() {
60+
return (
61+
<ToastProvider>
62+
<NavigationContainer>
63+
<Stack.Navigator>
64+
<Stack.Screen name="Home" component={Home} />
65+
<Stack.Screen name="Modal" component={Modal} options={{
66+
presentation: 'modal',
67+
headerRight: HeaderRight,
68+
}} />
69+
</Stack.Navigator>
70+
</NavigationContainer>
71+
</ToastProvider>
72+
);
73+
}
74+
75+
export default App;

apps/src/tests/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,7 @@ export { default as Test2789 } from './Test2789';
131131
export { default as Test2809 } from './Test2809';
132132
export { default as Test2811 } from './Test2811';
133133
export { default as Test2819 } from './Test2819';
134+
export { default as Test2842 } from './Test2842'; // [E2E created](iOS): issue is related to iOS
134135
export { default as Test2855 } from './Test2855';
135136
export { default as Test2877 } from './Test2877'; // [E2E created](iOS): issue is related to formSheet on iOS
136137
export { default as Test2895 } from './Test2895';

ios/RNSScreen.mm

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
#import <UIKit/UIKit.h>
22

3+
#import "RNSModalScreen.h"
34
#import "RNSScreen.h"
45
#import "RNSScreenContainer.h"
56
#import "RNSScreenContentWrapper.h"
@@ -154,12 +155,14 @@ - (void)updateBounds
154155
if (_state != nullptr) {
155156
RNSScreenStackHeaderConfig *config = [self findHeaderConfig];
156157

157-
// in large title, ScrollView handles the offset of content so we cannot set it here also
158-
// TODO: Why is it assumed in comment above, that large title uses scrollview here? What if only SafeAreaView is
158+
// * in large title, ScrollView handles the offset of content so we cannot set it here also
159+
// * TODO: Why is it assumed in comment above, that large title uses scrollview here? What if only SafeAreaView is
159160
// used?
160-
// When config.translucent == true, we currently use `edgesForExtendedLayout` and the screen is laid out under the
161+
// * When config.translucent == true, we currently use `edgesForExtendedLayout` and the screen is laid out under the
161162
// navigation bar, therefore there is no need to set content offset in shadow tree.
162-
const CGFloat effectiveContentOffsetY = config.largeTitle || config.translucent
163+
// * When this view is the modal root controller (presented in separate view hierarchy) it does not have navigation
164+
// bar! We send non-zero size to JS, for some reason. TODO: this needs to be investigated.
165+
const CGFloat effectiveContentOffsetY = config.largeTitle || config.translucent || self.isPresentedAsNativeModal
163166
? 0
164167
: [_controller calculateHeaderHeightIsModal:self.isPresentedAsNativeModal];
165168

0 commit comments

Comments
 (0)