Skip to content

Commit 229d6d2

Browse files
Hedger WangFacebook Github Bot 0
Hedger Wang
authored and
Facebook Github Bot 0
committed
Fix NavigationScenesReducer.
Summary: Fix a bug in NavigationScenesReducer that prevents scenes from re-rendering. This happens when jumping the index between routes. The fix is to add an new property `isActive` to `NavigationScene` to indicate the current active scene. Reviewed By: ericvicenti Differential Revision: D3479736 fbshipit-source-id: a71419887acd94ad2fead71596ca46419a88efef
1 parent 9c89221 commit 229d6d2

File tree

5 files changed

+150
-88
lines changed

5 files changed

+150
-88
lines changed

‎Libraries/NavigationExperimental/NavigationPropTypes.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ const layout = PropTypes.shape({
5858
/* NavigationScene */
5959
const scene = PropTypes.shape({
6060
index: PropTypes.number.isRequired,
61+
isActive: PropTypes.bool.isRequired,
6162
isStale: PropTypes.bool.isRequired,
6263
key: PropTypes.string.isRequired,
6364
route: navigationRoute.isRequired,

‎Libraries/NavigationExperimental/NavigationTransitioner.js‎

Lines changed: 7 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,6 @@ const React = require('React');
1919
const StyleSheet = require('StyleSheet');
2020
const View = require('View');
2121

22-
const invariant = require('fbjs/lib/invariant');
23-
2422
import type {
2523
NavigationAnimatedValue,
2624
NavigationLayout,
@@ -53,10 +51,6 @@ const DefaultTransitionSpec = {
5351
easing: Easing.inOut(Easing.ease),
5452
};
5553

56-
function isSceneNotStale(scene: NavigationScene): boolean {
57-
return !scene.isStale;
58-
}
59-
6054
class NavigationTransitioner extends React.Component<any, Props, State> {
6155
_onLayout: (event: any) => void;
6256
_onTransitionEnd: () => void;
@@ -244,21 +238,16 @@ function buildTransitionProps(
244238
position,
245239
progress,
246240
scenes,
247-
scene: findActiveScene(scenes, navigationState.index),
241+
scene: scenes.find(isSceneActive),
248242
};
249243
}
250244

251-
function findActiveScene(
252-
scenes: Array<NavigationScene>,
253-
index: number,
254-
): NavigationScene {
255-
for (let ii = 0, jj = scenes.length; ii < jj; ii++) {
256-
const scene = scenes[ii];
257-
if (!scene.isStale && scene.index === index) {
258-
return scene;
259-
}
260-
}
261-
invariant(false, 'scenes must have an active scene');
245+
function isSceneNotStale(scene: NavigationScene): boolean {
246+
return !scene.isStale;
247+
}
248+
249+
function isSceneActive(scene: NavigationScene): boolean {
250+
return scene.isActive;
262251
}
263252

264253
const styles = StyleSheet.create({

‎Libraries/NavigationExperimental/NavigationTypeDefinition.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ export type NavigationLayout = {
4040

4141
export type NavigationScene = {
4242
index: number,
43+
isActive: boolean,
4344
isStale: boolean,
4445
key: string,
4546
route: NavigationRoute,

‎Libraries/NavigationExperimental/Reducer/NavigationScenesReducer.js‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ function areScenesShallowEqual(
6767
one.key === two.key &&
6868
one.index === two.index &&
6969
one.isStale === two.isStale &&
70+
one.isActive === two.isActive &&
7071
areRoutesShallowEqual(one.route, two.route)
7172
);
7273
}
@@ -116,6 +117,7 @@ function NavigationScenesReducer(
116117
const key = SCENE_KEY_PREFIX + route.key;
117118
const scene = {
118119
index,
120+
isActive: false,
119121
isStale: false,
120122
key,
121123
route,
@@ -144,6 +146,7 @@ function NavigationScenesReducer(
144146
}
145147
staleScenes.set(key, {
146148
index,
149+
isActive: false,
147150
isStale: true,
148151
key,
149152
route,
@@ -170,6 +173,26 @@ function NavigationScenesReducer(
170173

171174
nextScenes.sort(compareScenes);
172175

176+
let activeScenesCount = 0;
177+
nextScenes.forEach((scene, ii) => {
178+
const isActive = !scene.isStale && scene.index === nextState.index;
179+
if (isActive !== scene.isActive) {
180+
nextScenes[ii] = {
181+
...scene,
182+
isActive,
183+
};
184+
}
185+
if (isActive) {
186+
activeScenesCount++;
187+
}
188+
});
189+
190+
invariant(
191+
activeScenesCount === 1,
192+
'there should always be only one scene active, not %s.',
193+
activeScenesCount,
194+
);
195+
173196
if (nextScenes.length !== scenes.length) {
174197
return nextScenes;
175198
}

‎Libraries/NavigationExperimental/Reducer/__tests__/NavigationScenesReducer-test.js‎

Lines changed: 118 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ const NavigationScenesReducer = require('NavigationScenesReducer');
1919
function testTransition(states) {
2020
const routes = states.map(keys => {
2121
return {
22+
index: 0,
2223
routes: keys.map(key => {
2324
return { key };
2425
}),
@@ -44,19 +45,21 @@ describe('NavigationScenesReducer', () => {
4445

4546
expect(scenes).toEqual([
4647
{
47-
'index': 0,
48-
'isStale': false,
49-
'key': 'scene_1',
50-
'route': {
51-
'key': '1'
48+
index: 0,
49+
isActive: true,
50+
isStale: false,
51+
key: 'scene_1',
52+
route: {
53+
key: '1'
5254
},
5355
},
5456
{
55-
'index': 1,
56-
'isStale': false,
57-
'key': 'scene_2',
58-
'route': {
59-
'key': '2'
57+
index: 1,
58+
isActive: false,
59+
isStale: false,
60+
key: 'scene_2',
61+
route: {
62+
key: '2'
6063
},
6164
},
6265
]);
@@ -71,32 +74,52 @@ describe('NavigationScenesReducer', () => {
7174

7275
expect(scenes).toEqual([
7376
{
74-
'index': 0,
75-
'isStale': false,
76-
'key': 'scene_1',
77-
'route': {
78-
'key': '1'
77+
index: 0,
78+
isActive: true,
79+
isStale: false,
80+
key: 'scene_1',
81+
route: {
82+
key: '1'
7983
},
8084
},
8185
{
82-
'index': 1,
83-
'isStale': false,
84-
'key': 'scene_2',
85-
'route': {
86-
'key': '2'
86+
index: 1,
87+
isActive: false,
88+
isStale: false,
89+
key: 'scene_2',
90+
route: {
91+
key: '2'
8792
},
8893
},
8994
{
90-
'index': 2,
91-
'isStale': false,
92-
'key': 'scene_3',
93-
'route': {
94-
'key': '3'
95+
index: 2,
96+
isActive: false,
97+
isStale: false,
98+
key: 'scene_3',
99+
route: {
100+
key: '3'
95101
},
96102
},
97103
]);
98104
});
99105

106+
it('gets active scene when index changes', () => {
107+
const state1 = {
108+
index: 0,
109+
routes: [{key: '1'}, {key: '2'}],
110+
};
111+
112+
const state2 = {
113+
index: 1,
114+
routes: [{key: '1'}, {key: '2'}],
115+
};
116+
117+
const scenes1 = NavigationScenesReducer([], state1, null);
118+
const scenes2 = NavigationScenesReducer(scenes1, state2, state1);
119+
const route = scenes2.find((scene) => scene.isActive).route;
120+
expect(route).toEqual({key: '2'});
121+
});
122+
100123
it('gets same scenes', () => {
101124
const state1 = {
102125
index: 0,
@@ -146,6 +169,22 @@ describe('NavigationScenesReducer', () => {
146169
});
147170

148171

172+
it('gets different scenes when state index changes', () => {
173+
const state1 = {
174+
index: 0,
175+
routes: [{key: '1', x: 1}, {key: '2', x: 2}],
176+
};
177+
178+
const state2 = {
179+
index: 1,
180+
routes: [{key: '1', x: 1}, {key: '2', x: 2}],
181+
};
182+
183+
const scenes1 = NavigationScenesReducer([], state1, null);
184+
const scenes2 = NavigationScenesReducer(scenes1, state2, state1);
185+
expect(scenes1).not.toBe(scenes2);
186+
});
187+
149188
it('pops scenes', () => {
150189
// Transition from ['1', '2', '3'] to ['1', '2'].
151190
const scenes = testTransition([
@@ -155,27 +194,30 @@ describe('NavigationScenesReducer', () => {
155194

156195
expect(scenes).toEqual([
157196
{
158-
'index': 0,
159-
'isStale': false,
160-
'key': 'scene_1',
161-
'route': {
162-
'key': '1'
197+
index: 0,
198+
isActive: true,
199+
isStale: false,
200+
key: 'scene_1',
201+
route: {
202+
key: '1'
163203
},
164204
},
165205
{
166-
'index': 1,
167-
'isStale': false,
168-
'key': 'scene_2',
169-
'route': {
170-
'key': '2'
206+
index: 1,
207+
isActive: false,
208+
isStale: false,
209+
key: 'scene_2',
210+
route: {
211+
key: '2'
171212
},
172213
},
173214
{
174-
'index': 2,
175-
'isStale': true,
176-
'key': 'scene_3',
177-
'route': {
178-
'key': '3'
215+
index: 2,
216+
isActive: false,
217+
isStale: true,
218+
key: 'scene_3',
219+
route: {
220+
key: '3'
179221
},
180222
},
181223
]);
@@ -189,27 +231,30 @@ describe('NavigationScenesReducer', () => {
189231

190232
expect(scenes).toEqual([
191233
{
192-
'index': 0,
193-
'isStale': true,
194-
'key': 'scene_1',
195-
'route': {
196-
'key': '1'
234+
index: 0,
235+
isActive: false,
236+
isStale: true,
237+
key: 'scene_1',
238+
route: {
239+
key: '1'
197240
},
198241
},
199242
{
200-
'index': 0,
201-
'isStale': false,
202-
'key': 'scene_3',
203-
'route': {
204-
'key': '3'
243+
index: 0,
244+
isActive: true,
245+
isStale: false,
246+
key: 'scene_3',
247+
route: {
248+
key: '3'
205249
},
206250
},
207251
{
208-
'index': 1,
209-
'isStale': true,
210-
'key': 'scene_2',
211-
'route': {
212-
'key': '2'
252+
index: 1,
253+
isActive: false,
254+
isStale: true,
255+
key: 'scene_2',
256+
route: {
257+
key: '2'
213258
},
214259
},
215260
]);
@@ -224,27 +269,30 @@ describe('NavigationScenesReducer', () => {
224269

225270
expect(scenes).toEqual([
226271
{
227-
'index': 0,
228-
'isStale': true,
229-
'key': 'scene_1',
230-
'route': {
231-
'key': '1'
272+
index: 0,
273+
isActive: false,
274+
isStale: true,
275+
key: 'scene_1',
276+
route: {
277+
key: '1'
232278
},
233279
},
234280
{
235-
'index': 0,
236-
'isStale': false,
237-
'key': 'scene_2',
238-
'route': {
239-
'key': '2'
281+
index: 0,
282+
isActive: true,
283+
isStale: false,
284+
key: 'scene_2',
285+
route: {
286+
key: '2'
240287
},
241288
},
242289
{
243-
'index': 0,
244-
'isStale': true,
245-
'key': 'scene_3',
246-
'route': {
247-
'key': '3'
290+
index: 0,
291+
isActive: false,
292+
isStale: true,
293+
key: 'scene_3',
294+
route: {
295+
key: '3'
248296
},
249297
},
250298
]);

0 commit comments

Comments
 (0)