Skip to content

Commit 88c9bbd

Browse files
Fix incorrect back navigation to ForYou instead of Topic screen (#1866)
When navigating ForYou → Topic → switch tab → back, the user landed on the ForYou screen instead of the Topic screen. The root cause was that toEntries() flattened all tab sub-stacks into a single entries list, causing ListDetailSceneStrategy to mis-render after cross-tab back. - Change toEntries() to only return entries from the current tab's sub-stack while still decorating all stacks for state preservation - Add BackHandler for cross-tab back navigation since NavDisplay now only sees within-tab entries - Add unit test for the sub-stack preservation scenario - Un-ignore navigationBar_multipleBackStackInterests test - Add back-button variant instrumented test
1 parent ade8065 commit 88c9bbd

File tree

4 files changed

+63
-8
lines changed
  • app/src
    • androidTest/kotlin/com/google/samples/apps/nowinandroid/ui
    • main/kotlin/com/google/samples/apps/nowinandroid/ui
  • core/navigation/src

4 files changed

+63
-8
lines changed

app/src/androidTest/kotlin/com/google/samples/apps/nowinandroid/ui/NavigationTest.kt

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ import dagger.hilt.android.testing.HiltAndroidTest
4646
import kotlinx.coroutines.flow.first
4747
import kotlinx.coroutines.runBlocking
4848
import org.junit.Before
49-
import org.junit.Ignore
5049
import org.junit.Rule
5150
import org.junit.Test
5251
import javax.inject.Inject
@@ -255,9 +254,6 @@ class NavigationTest {
255254
}
256255
}
257256

258-
// TODO decide if backStack should preserve previous stacks when navigating back to home tab (ForYou)
259-
// https://github.com/android/nowinandroid/issues/1937
260-
@Ignore
261257
@Test
262258
fun navigationBar_multipleBackStackInterests() {
263259
composeTestRule.apply {
@@ -283,6 +279,32 @@ class NavigationTest {
283279
}
284280
}
285281

282+
@Test
283+
fun navigationBar_backFromTabPreservesSubStack() {
284+
composeTestRule.apply {
285+
onNodeWithText(interests).performClick()
286+
287+
// Select a topic
288+
val topic = runBlocking {
289+
topicsRepository.getTopics().first().sortedBy(Topic::name).last()
290+
}
291+
onNodeWithTag(LIST_PANE_TEST_TAG).performScrollToNode(hasText(topic.name))
292+
onNodeWithText(topic.name).performClick()
293+
294+
// Verify the topic is shown
295+
onNodeWithTag("topic:${topic.id}").assertIsDisplayed()
296+
297+
// Switch to another tab
298+
onNodeWithText(saved).performClick()
299+
300+
// Press back to return to previous tab
301+
Espresso.pressBack()
302+
303+
// Verify the topic detail is still shown (sub-stack preserved)
304+
onNodeWithTag("topic:${topic.id}").assertExists()
305+
}
306+
}
307+
286308
@Test
287309
fun navigatingToTopicFromForYou_showsTopicDetails() {
288310
composeTestRule.apply {

app/src/main/kotlin/com/google/samples/apps/nowinandroid/ui/NiaApp.kt

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
package com.google.samples.apps.nowinandroid.ui
1818

19+
import androidx.activity.compose.BackHandler
1920
import androidx.compose.foundation.layout.Box
2021
import androidx.compose.foundation.layout.Column
2122
import androidx.compose.foundation.layout.WindowInsets
@@ -264,8 +265,16 @@ internal fun NiaApp(
264265
searchEntry(navigator)
265266
}
266267

268+
val navigationState = appState.navigationState
269+
BackHandler(
270+
enabled = navigationState.currentKey == navigationState.currentTopLevelKey &&
271+
navigationState.currentTopLevelKey != navigationState.startKey,
272+
) {
273+
navigator.goBack()
274+
}
275+
267276
NavDisplay(
268-
entries = appState.navigationState.toEntries(entryProvider),
277+
entries = navigationState.toEntries(entryProvider),
269278
sceneStrategy = listDetailStrategy,
270279
onBack = { navigator.goBack() },
271280
)

core/navigation/src/main/kotlin/com/google/samples/apps/nowinandroid/core/navigation/NavigationState.kt

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,5 @@ fun NavigationState.toEntries(
9696
)
9797
}
9898

99-
return topLevelStack
100-
.flatMap { decoratedEntries[it] ?: emptyList() }
101-
.toMutableStateList()
99+
return (decoratedEntries[currentTopLevelKey] ?: emptyList()).toMutableStateList()
102100
}

core/navigation/src/test/kotlin/com/google/samples/apps/nowinandroid/core/navigation/NavigatorTest.kt

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -247,6 +247,32 @@ class NavigatorTest {
247247
assertThat(navigationState.currentTopLevelKey).isEqualTo(TestFirstTopLevelKey)
248248
}
249249

250+
@Test
251+
fun testSubStackPreservedAfterTabSwitchAndBack() {
252+
// Navigate to sub-page in first tab (ForYou → Topic)
253+
navigator.navigate(TestKeyFirst)
254+
255+
assertThat(navigationState.currentKey).isEqualTo(TestKeyFirst)
256+
assertThat(navigationState.currentTopLevelKey).isEqualTo(TestFirstTopLevelKey)
257+
258+
// Switch to second tab (e.g. Bookmarks)
259+
navigator.navigate(TestSecondTopLevelKey)
260+
261+
assertThat(navigationState.currentKey).isEqualTo(TestSecondTopLevelKey)
262+
assertThat(navigationState.currentTopLevelKey).isEqualTo(TestSecondTopLevelKey)
263+
264+
// Press back from second tab
265+
navigator.goBack()
266+
267+
// Should return to first tab with sub-stack preserved
268+
assertThat(navigationState.currentTopLevelKey).isEqualTo(TestFirstTopLevelKey)
269+
assertThat(navigationState.currentKey).isEqualTo(TestKeyFirst)
270+
assertThat(navigationState.currentSubStack).containsExactly(
271+
TestFirstTopLevelKey,
272+
TestKeyFirst,
273+
).inOrder()
274+
}
275+
250276
@Test
251277
fun throwOnEmptyBackStack() {
252278
assertFailsWith<IllegalStateException> {

0 commit comments

Comments
 (0)