Commit c72539d
authored
feat(Android, Stack v5): allow for nested container pop via JS (#3612)
## Description
Previously it has not been possible to pop nested container via JS.
When there was only a single screen left in the JS stack state, the
reducer would deny removing it and would just ignore the operation
printing a warning that pop operation requires at least two screens.
Currently, in such scenario the parent `StackContainer` (JS component)
will be asked to pop whole screen that hosts the nested container.
## Changes
This is achieved by changes in stack container state reducer.
Previously it operated directly on stack, but now I need to somehow
communicate that an action has not been performed directly on the
container and must be forwarded to parent container. We can
not use the `parentNavigation` directly in the reducer, because:
1. reducer should be pure,
2. we can not dispatch an action directly to the parent container during
render (and this is when the state reducing takes place), React
treats it as an error.
Therefore, I see two possibilities here. Either we move all the
`StackContainer` state to some centralized place (similarly to what
react-navigation does with `NavigationContainer`) or we stay with
current approach (each `StackContainer` owns its state) but we emit
this intent as an side effect.
Right now I went with second option. Reducer, when it find it makes
sense, not only updates the stack state, but also produces side effects
to be performed. Currently, there is only single side effect:
`PopContainerStackNavigationEffect`, but I found it low cost to make it
a bit more generic.
Reducer produces the side effect, which is later consumed by the new
`useParentNavigationEffect` hook.
Important thing to note in this implementation is the referential
stability of the effects array. The state object id changes every time
`state.stack` or `state.effects` change. However, `state.effects` object
id stays the same as long as there is no change to prevent excessive
rendering.
Given a nested stack, with only a single screen in the nested stack,
the flow is as follows:
1. `navigation.pop` is dispatched to the nested stack, this triggers
render.
2. Reducer detects that it has only a single screen & can not perform
the action, therefore it produces new state with new effects array
containing the `PopContainerStackNavigationEffect`.
3. The `useParentNavigationEffect` schedules the `useEffect`, which is
executed after this render.
4. The `useEffect` is executed triggering render of the outer container
and consuming the effect (trigger render of the inner container).
5. New render starts from the outer container -> it performs the
navigation action, detaching the screen that hosts nested container
6. During the same render as in 5., nested container clears its
`state.effects`.
7. After the dismiss completes, `pop-completed` operation is dispatched
by event listeners & hosting screen & nested container are cleared from
the state (they no longer render).
> [!note]
> Thing to note here, is that the nested stack's only screen, when it's
> being dismissed it sends `onDismissed(isNativeDismiss=true)` event,
> because it is dismissed & it's activityMode has never been changed to
> `detached`. I don't think this is causing any problems right now,
> but it is definitely something to be aware of.
What happens if there is only a single screen in outer stack & it
hosts the nested stack? Nothing. Renders are triggered, but there is no
one to handle that operation.
## Visual documentation
| before | after |
| -- | -- |
| <video
src=https://github.com/user-attachments/assets/836181fb-4f94-488f-93bd-27a58a3e2f20
alt="before" /> | <video
src=https://github.com/user-attachments/assets/84997062-07d6-4a48-878b-08d04e3891be
alt="after" /> |
## Test plan
I'm using `TestStackNesting`.
Just as the video shows.
## Checklist
- [x] Included code example that can be used to test this change.
- [x] Updated / created local changelog entries in relevant test files.
- [x] For visual changes, included screenshots / GIFs / recordings
documenting the change.
- [x] For API changes, updated relevant public types.
- [x] Ensured that CI passes (all but Android lint - I'll fix it
separately one day)1 parent 5a6f842 commit c72539d
6 files changed
Lines changed: 224 additions & 82 deletions
Lines changed: 34 additions & 27 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3 | 3 | | |
4 | 4 | | |
5 | 5 | | |
| 6 | + | |
6 | 7 | | |
7 | | - | |
8 | 8 | | |
9 | 9 | | |
10 | | - | |
| 10 | + | |
11 | 11 | | |
12 | 12 | | |
13 | 13 | | |
| |||
19 | 19 | | |
20 | 20 | | |
21 | 21 | | |
| 22 | + | |
22 | 23 | | |
23 | 24 | | |
24 | 25 | | |
25 | 26 | | |
26 | | - | |
27 | | - | |
| 27 | + | |
| 28 | + | |
28 | 29 | | |
29 | 30 | | |
30 | 31 | | |
31 | 32 | | |
32 | | - | |
| 33 | + | |
33 | 34 | | |
34 | 35 | | |
35 | 36 | | |
36 | 37 | | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
37 | 42 | | |
38 | 43 | | |
39 | 44 | | |
| |||
55 | 60 | | |
56 | 61 | | |
57 | 62 | | |
58 | | - | |
59 | | - | |
60 | | - | |
61 | | - | |
62 | | - | |
63 | | - | |
64 | | - | |
65 | | - | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
66 | 72 | | |
67 | | - | |
68 | | - | |
69 | | - | |
70 | | - | |
71 | | - | |
72 | | - | |
73 | | - | |
74 | | - | |
75 | | - | |
76 | | - | |
77 | | - | |
78 | | - | |
79 | | - | |
80 | | - | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
81 | 88 | | |
82 | 89 | | |
83 | 90 | | |
| |||
Lines changed: 29 additions & 4 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
3 | 3 | | |
| 4 | + | |
| 5 | + | |
4 | 6 | | |
5 | 7 | | |
6 | 8 | | |
| |||
12 | 14 | | |
13 | 15 | | |
14 | 16 | | |
15 | | - | |
16 | | - | |
17 | | - | |
18 | | - | |
19 | 17 | | |
20 | 18 | | |
21 | 19 | | |
22 | 20 | | |
23 | 21 | | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
24 | 28 | | |
25 | 29 | | |
26 | 30 | | |
27 | 31 | | |
28 | 32 | | |
29 | 33 | | |
| 34 | + | |
30 | 35 | | |
31 | 36 | | |
32 | 37 | | |
| |||
35 | 40 | | |
36 | 41 | | |
37 | 42 | | |
| 43 | + | |
38 | 44 | | |
39 | 45 | | |
40 | 46 | | |
41 | 47 | | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
42 | 59 | | |
43 | 60 | | |
44 | 61 | | |
| |||
74 | 91 | | |
75 | 92 | | |
76 | 93 | | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
77 | 101 | | |
78 | 102 | | |
79 | 103 | | |
| |||
89 | 113 | | |
90 | 114 | | |
91 | 115 | | |
| 116 | + | |
92 | 117 | | |
Lines changed: 39 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
Lines changed: 7 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2 | 2 | | |
3 | 3 | | |
4 | 4 | | |
| 5 | + | |
5 | 6 | | |
6 | 7 | | |
7 | 8 | | |
| |||
74 | 75 | | |
75 | 76 | | |
76 | 77 | | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
77 | 82 | | |
78 | 83 | | |
79 | 84 | | |
| |||
82 | 87 | | |
83 | 88 | | |
84 | 89 | | |
| 90 | + | |
85 | 91 | | |
86 | 92 | | |
87 | 93 | | |
| |||
90 | 96 | | |
91 | 97 | | |
92 | 98 | | |
| 99 | + | |
93 | 100 | | |
94 | 101 | | |
95 | 102 | | |
| |||
0 commit comments