Status Update
No update yet.
Comments
il...@google.com
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
sk...@gmail.com
I simplified it too much, I am sorry. It's happening when using a custom nav type.
The contract does not mention it:
/**
* Serialize a value of this NavType into a String.
*
* By default it returns value of [kotlin.toString] or null if value passed in is null.
*
* This method can be override for custom serialization implementation on types such
* custom NavType classes.
*
* @param value a value representing this NavType to be serialized into a String
* @return serialized String value of [value]
*/
and TBH expecting this from NavType seems to be a code smell (non-symmetric, as can be seen in StringType).
il...@google.com
That is working as intended then.
So is your actual issue that you didn't know that serializeAsValue needed to return a valid Uri encoded string?
sk...@gmail.com
So is your actual issue that you didn't know that serializeAsValue needed to return a valid Uri encoded string?
Yes. Also, I realized this from Crashlytics. Maybe a lint would be useful or something to realize this earlier.
il...@google.com
cl...@google.com
As first step, adding comment to highlight the fact that outputs should be encoded. The lint can be added as a follow up, as part of other safe arg lints to be added.
ap...@google.com
Branch: androidx-main
commit 752e82e468e577c2fc891faae7b815501d6d3504
Author: Clara Fok <clarafok@google.com>
Date: Thu Jun 06 11:03:34 2024
Add comment on serializeAsValue/Values
Test: TH
Bug: 344943214
Relnote: "Add comment on NavType.serializeAsValue and serializeAsValues to highlight that the final outputs should be Uri encoded."
Change-Id: Ida6bd848fb1e3d8e26277eff0f214e1d40907063
M navigation/navigation-common/src/main/java/androidx/navigation/CollectionNavType.kt
M navigation/navigation-common/src/main/java/androidx/navigation/NavType.kt
cl...@google.com
The added comments will be available in navigation 2.8.0-beta03.
jb...@google.com
pr...@google.com
The following release(s) address this bug.It is possible this bug has only been partially addressed:
androidx.navigation:navigation-common:2.8.0-beta03
ig...@forzafootball.com
I believe the real issue has not been addressed.
The very idea of implementing type-safe navigation is to abstract details away (like dealing with uri creation and parsing), thus decreasing runtime issues by making developers work with types, which is great, BUT for some reason type abstraction still leaks with uri-based backend details, and still obligates devs to write boilerplate. For example there could be
enum class Category {
@SerialName("started") Started,
@SerialName("ended_unexpectedly") EndedUnexpectedly
}
which could be used as a primitive within app, come from network of whatever and one would expect to safely use it as a property in route data class bc there is first-party support for enum serialization.
Despite all this assumptions app would fail, when EndedUnexpectedly is used, but won't fail if Started is used bc of _, which is quite weird and moreover crash won't tell anything related to this specifically, only generic "can't find destination".
Forcing developers taking care of this is clearly the same issue we had before type-safe nav and which it was supposed to fix.
Description
Component used: Navigation
Version used: 2.8.0-beta02
Devices/Android versions reproduced on: emulator API 34
Basically,
RouteBuilder.Filled<T>does not do any URL escaping. This causes runtime crashes during navigation with a non-escaped value the navigation library cannot find a target destination (as there is no argument match).This is unexpected as I'm using the abstraction for objects, with no URLs involved.