Skip to content

Commit 33feebb

Browse files
kmichalikkkkafar
andauthored
feat(iOS, Stack v5): Align Stack implementation with RFC 753 (#3774)
Closes software-mansion/react-native-screens-labs#1050 Closes software-mansion/react-native-screens-labs#864 Closes software-mansion/react-native-screens-labs#816 ## Description This PR aligns the current Stack implementation with RFC 753 by adding an operations queue that enables us to defer the application of JS differences in a batch that moves the stack from one consistent state to another. The implementation has been adapted from Android implementation, with key difference being the absence of FragmentManager along with its FragmentOperations. These are handled within RNSStackNavigationController. ## Changes - Introduced pendingPopOperations and pendingPushOperations that form a queue and need not be applied immediately. - Replaced swift controllers with objective-c to remove the need of interoperability: given the amount of props, enums, customizations that will be present in the future, I think it's better to avoid the interop entirely ## Before & after - visual documentation no visual changes ## Test plan Use `Tests.Issue.TestScreenStack` to test various scenarios, verify the state in the logs. Run `Tests.Issue.Test3576` to check a specific scenario that caused problems on the android. ## Checklist - [ ] Ensured that CI passes --------- Co-authored-by: Kacper Kafara <kacperkafara@gmail.com>
1 parent 8748b03 commit 33feebb

18 files changed

+497
-221
lines changed

apps/src/shared/gamma/containers/stack/StackContainer.types.tsx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ export type StackRouteConfig = {
2020
export type StackRoute = StackRouteConfig & {
2121
activityMode: StackScreenProps['activityMode'];
2222
routeKey: StackScreenProps['screenKey'];
23+
isMarkedForDismissal: Boolean, // whether this route is during or after dismissal process
2324
};
2425

2526
/// StackContainer props

apps/src/shared/gamma/containers/stack/reducer.tsx

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ function navigationActionPushHandler(
8181
const stack = state.stack;
8282
const renderedRouteIndex = stack.findIndex(
8383
route =>
84-
route.name === action.routeName && route.activityMode === 'detached',
84+
route.name === action.routeName && route.activityMode === 'detached' && !route.isMarkedForDismissal,
8585
);
8686

8787
if (renderedRouteIndex !== NOT_FOUND_INDEX) {
@@ -161,12 +161,21 @@ function navigationActionPopHandler(
161161
return state;
162162
}
163163

164+
// Pop operation on not-top screen is forbidden and might crash.
165+
const topAttachedRouteIndex = state.stack.findLastIndex(r => r.activityMode === 'attached');
166+
167+
if (topAttachedRouteIndex > routeIndex) {
168+
console.warn(`[Stack] Can not perform pop action on route: ${action.routeKey} - not a top screen`);
169+
return state;
170+
}
171+
164172
const newStack = [...stack];
165173
// NOTE: This modifies existing state, possibly impacting calculations done before new state is updated.
166174
// Consider doing deep copy of the state here.
167175
// EDIT: not sure really whether this is really a problem or not, since the updates are queued
168176
// and the original state won't be immediatelly affected.
169177
route.activityMode = 'detached';
178+
route.isMarkedForDismissal = true;
170179

171180
return stateWithStack(state, newStack);
172181
}
@@ -266,6 +275,7 @@ function createRouteFromConfig(
266275
...config,
267276
activityMode,
268277
routeKey: generateRouteKeyForRouteName(config.name),
278+
isMarkedForDismissal: false,
269279
};
270280
}
271281

@@ -312,7 +322,7 @@ function applyPush(state: StackState, newRoute: StackRoute): StackState {
312322
route => route.activityMode === 'attached',
313323
);
314324

315-
if (lastAttachedIndex === -1) {
325+
if (lastAttachedIndex === NOT_FOUND_INDEX) {
316326
throw new Error(
317327
`[Stack] Invalid stack state: there should be at least one attached route on the stack.`,
318328
);

apps/src/tests/single-feature-tests/stack-v5/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,14 @@ import PreventNativeDismissSingleStack from './prevent-native-dismiss-single-sta
33
import PreventNativeDismissNestedStack from './prevent-native-dismiss-nested-stack';
44
import AnimationAndroid from './test-animation-android';
55
import TestStackHeaderModes from './test-stack-header-modes';
6+
import TestStackSimpleNav from './test-stack-simple-nav';
67

78
const scenarios = {
89
PreventNativeDismissSingleStack,
910
PreventNativeDismissNestedStack,
1011
AnimationAndroid,
1112
TestStackHeaderModes,
13+
TestStackSimpleNav,
1214
};
1315

1416
const StackScenarioGroup: ScenarioGroup<keyof typeof scenarios> = {
Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,97 @@
1+
import React from 'react';
2+
import type { Scenario } from '@apps/tests/shared/helpers';
3+
import { StyleSheet, Text, View } from 'react-native';
4+
import {
5+
StackContainer,
6+
useStackNavigationContext,
7+
} from '@apps/shared/gamma/containers/stack';
8+
import { CenteredLayoutView } from '@apps/shared/CenteredLayoutView';
9+
import Colors from '@apps/shared/styling/Colors';
10+
import { StackNavigationButtons } from '@apps/tests/shared/components/stack-v5/StackNavigationButtons';
11+
12+
const SCENARIO: Scenario = {
13+
name: 'Simple navigation scenario',
14+
key: 'test-stack-simple-nav',
15+
details:
16+
'Test simple push and pop operations',
17+
platforms: ['android', 'ios'],
18+
AppComponent: App,
19+
};
20+
21+
export default SCENARIO;
22+
23+
export function App() {
24+
return (
25+
<StackSetup />
26+
);
27+
}
28+
29+
function StackSetup() {
30+
return (
31+
<StackContainer
32+
routeConfigs={[
33+
{
34+
name: 'Home',
35+
Component: HomeScreen,
36+
options: {},
37+
},
38+
{
39+
name: 'A',
40+
Component: AScreen,
41+
options: {},
42+
},
43+
{
44+
name: 'B',
45+
Component: BScreen,
46+
options: {},
47+
},
48+
]}
49+
/>
50+
);
51+
}
52+
53+
function HomeScreen() {
54+
return (
55+
<CenteredLayoutView style={{ backgroundColor: Colors.BlueLight40 }}>
56+
<RouteInformation routeName="Home" />
57+
<StackNavigationButtons isPopEnabled={false} routeNames={['A', 'B']} />
58+
</CenteredLayoutView>
59+
);
60+
}
61+
62+
function AScreen() {
63+
return (
64+
<CenteredLayoutView style={{ backgroundColor: Colors.YellowLight40 }}>
65+
<RouteInformation routeName="A" />
66+
<StackNavigationButtons isPopEnabled={true} routeNames={['A', 'B']} />
67+
</CenteredLayoutView>
68+
);
69+
}
70+
71+
function BScreen() {
72+
return (
73+
<CenteredLayoutView style={{ backgroundColor: Colors.GreenLight100 }}>
74+
<RouteInformation routeName="B" />
75+
<StackNavigationButtons isPopEnabled={true} routeNames={['A', 'B']} />
76+
</CenteredLayoutView>
77+
);
78+
}
79+
80+
function RouteInformation(props: { routeName: string }) {
81+
const routeKey = useStackNavigationContext().routeKey;
82+
83+
return (
84+
<View>
85+
<Text style={styles.routeInformation}>Name: {props.routeName}</Text>
86+
<Text style={styles.routeInformation}>Key: {routeKey}</Text>
87+
</View>
88+
);
89+
}
90+
91+
const styles = StyleSheet.create({
92+
routeInformation: {
93+
color: 'black',
94+
fontSize: 20,
95+
fontWeight: 'bold',
96+
},
97+
});

ios/gamma/stack/host/RNSStackController.swift

Lines changed: 0 additions & 65 deletions
This file was deleted.

ios/gamma/stack/host/RNSStackHostComponentView.h

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,12 @@
11
#pragma once
22

33
#import "RNSReactBaseView.h"
4-
5-
@class RNSStackController;
6-
@class RNSStackScreenComponentView;
4+
#import "RNSStackScreenComponentView.h"
75

86
NS_ASSUME_NONNULL_BEGIN
97

108
@interface RNSStackHostComponentView : RNSReactBaseView
119

12-
@property (nonatomic, nonnull, strong, readonly) RNSStackController *stackController;
13-
14-
- (nonnull NSMutableArray<RNSStackScreenComponentView *> *)reactSubviews;
15-
1610
@end
1711

1812
#pragma mark - Communication with StackScreen

0 commit comments

Comments
 (0)