Ensure visible entries is populated correctly [225394514]

Fixed

Bug

Status Update

No update yet.

Description

jb...@google.com

created issue #1

Mar 18, 2022 08:35PM

The visibleEntries stateFlow that is currently offered as an experimental API on NavController is populated based on the current lifecycle state of the entry. This means that it considers the hostLifecycleState of the entry when determining whether entries should be part of the list.

Because of this, when the navController is first created, depending on the lifecycle state when you call setGraph, it is possible for the visibleEntries to be empty until after the first navigate call. Also, if the hostLifecycle state is forced down, as in the case of using nested NavHost with different NavControllers in navigation-compose, entries are immediately removed from the list of visible entries although they might still be visible because of animations.

We should visible visible entries so that is always contains any entry that actually should be considered visible.

Comments

ap...@google.com <ap...@google.com> #2Mar 28, 2022 10:00PM

Project: platform/frameworks/support
Branch: androidx-master-dev

commit c60f33e229e31ab328ef6b59dab63b264954831c
Author: Alexandre Elias <aelias@google.com>
Date: Fri Jul 10 16:23:09 2020

Semantics no-op cleanups

Partly in response to lmr's broad code review, I did a pass of
superficial API/implementation cleanup. The main changes are:

- I changed each Boolean SemanticsProperty where false is equivalent to
not being present to take "Unit" instead. This is conceptually
clearer: it avoids questions like "can I cancel out a semantics from a
merged child by setting it to false?" Because "property = Unit" looks
weird, I also changed the style of these to "property()".

- I moved the Semantics id generator closer to where it's used, in
SemanticsModifierCore. I made it internal and an AtomicInt.
(Note that integer ids are heavily used in the Android
AccessibilityNodeInfo APIs so I can't simply remove them entirely.)

- I deleted dead code. Some examples include SemanticsHintOverrides,
a public API not connected to anything, and SemanticsPropertyKey
merge() open method which is never called. (In both cases I have
a different plan in mind for accessibility.)

Fixes: 145951226
Fixes: 145955412
Test: existing tests
Relnote: "Single-value semantics properties now use a calling style.
For example, 'semantics { hidden = true }' is now written as:
'semantics { hidden() }'."

Change-Id: Ic1afd12ea22c926babc9662f1804d80b33aa0cfc

M ui/integration-tests/benchmark/src/androidTest/java/androidx/ui/benchmark/test/LayoutNodeModifierBenchmark.kt
M ui/ui-core/api/0.1.0-dev15.txt
M ui/ui-core/api/current.txt
M ui/ui-core/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-core/api/public_plus_experimental_current.txt
M ui/ui-core/api/restricted_0.1.0-dev15.txt
M ui/ui-core/api/restricted_current.txt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/graphics/vector/VectorTest.kt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/semantics/SemanticsTests.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidActuals.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeView.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeViewAccessibilityDelegateCompat.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidPopup.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/Expect.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsConfiguration.kt
D ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsHintOverrides.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsModifier.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsNode.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsOwner.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsWrapper.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/semantics/SemanticsProperties.kt
M ui/ui-foundation/api/0.1.0-dev15.txt
M ui/ui-foundation/api/current.txt
M ui/ui-foundation/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-foundation/api/public_plus_experimental_current.txt
M ui/ui-foundation/api/restricted_0.1.0-dev15.txt
M ui/ui-foundation/api/restricted_current.txt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Clickable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Dialog.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Scroller.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Selectable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Toggleable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/semantics/FoundationSemanticsProperties.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ButtonTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CardTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CheckboxScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/RadioButtonScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ScaffoldTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SnackbarTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SurfaceTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ripple/RippleIndicationTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/textfield/TextFieldScreenshotTest.kt
M ui/ui-material/src/main/java/androidx/ui/material/AppBar.kt
M ui/ui-material/src/main/java/androidx/ui/material/TextFieldImpl.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/AssertsTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/CallSemanticsActionTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ErrorMessagesTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/FindersTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/PrintToStringTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ScrollToTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/TextActionsTest.kt
M ui/ui-test/src/main/java/androidx/ui/test/Actions.kt
M ui/ui-test/src/main/java/androidx/ui/test/Filters.kt
M ui/ui-text/api/0.1.0-dev15.txt
M ui/ui-text/api/current.txt
M ui/ui-text/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-text/api/public_plus_experimental_current.txt
M ui/ui-text/api/restricted_0.1.0-dev15.txt
M ui/ui-text/api/restricted_current.txt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/CoreTextField.kt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/TextSemanticsProperties.kt

https://android-review.googlesource.com/1360099

ap...@google.com <ap...@google.com> #3Mar 28, 2022 10:00PM

Project: platform/frameworks/support
Branch: androidx-main

commit 4e8c0ba4829f25f2aab3daa89b6b7f8feaf1eb25
Author: Jeremy Woods <jbwoods@google.com>
Date: Wed Mar 23 16:09:20 2022

Base visible entries off max lifecycle

Instead of having visible entries based off of the entry current
lifecycle which considers the host lifecycle, we should base them on the
maxLifecycle.

This means in the case of nested NavHost, the visible entries of the
inner NavHost will remain the same even if the outer state goes below
STARTED.

RelNote: "The `visibleEntries` `StateFlow provided by `NavController` is now based on the entry max Lifecycle state instead of the current Lifecycle state. This means that even if the host lifecycle of the navController goes below STARTED, the list of visible entires will remain the same."
Test: added test
Bug: 225394514

Change-Id: I9e2a810f261e9dc1e61417b168427b3bf597fbd5

M navigation/navigation-runtime/src/main/java/androidx/navigation/NavController.kt
M navigation/navigation-runtime/src/androidTest/java/androidx/navigation/NavBackStackEntryLifecycleTest.kt

https://android-review.googlesource.com/2039224

ap...@google.com <ap...@google.com> #4Mar 29, 2022 06:02PM

Project: platform/frameworks/support
Branch: androidx-main

commit 1cb5a423aa271f78c7b8c843f49e51f976909c1d
Author: Jeremy Woods <jbwoods@google.com>
Date: Wed Mar 23 17:18:06 2022

Use visibleEntries in NavHost

Instead of all the custom code we currently use in NavHost to determine
which destinations to compose, we can just use visibleEntries directly
which has already sorted those destinations for us.

Moved the custom APIs over to DialogHost to fix that separately as the
DialogHost does not have access to a NavController.

RelNote: "NavHost now depend on `visibleEntries` from the NavController
to determine which entries to compose. This means that when using nested
NavHost the inner NavHost should now properly animate out."
Test: all existing tests pass
Bug: 225394514

Change-Id: I4ba2b542821154a76566f9c02cf7a95599a30f14

M navigation/navigation-compose/src/main/java/androidx/navigation/compose/NavHost.kt
M navigation/navigation-compose/src/main/java/androidx/navigation/compose/DialogHost.kt
M navigation/navigation-compose/src/androidTest/java/androidx/navigation/compose/NavHostTest.kt

https://android-review.googlesource.com/2039231

jb...@google.com <jb...@google.com> #5Mar 29, 2022 09:38PM

Marked as fixed.

This has been fixed internally and will be available in the Navigation 2.5.0-alpha05 release.

Issue 225394514

Description

Issue summary

Comments

ap...@google.com <ap...@google.com> #2Mar 28, 2022 10:00PM

ap...@google.com <ap...@google.com> #3Mar 28, 2022 10:00PM

ap...@google.com <ap...@google.com> #4Mar 29, 2022 06:02PM

jb...@google.com <jb...@google.com> #5Mar 29, 2022 09:38PM

Add comment

Issue metadata