Fixed
Status Update
Comments
da...@google.com <da...@google.com>
il...@google.com <il...@google.com> #2
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
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
ma...@gmail.com <ma...@gmail.com> #3
<navigation xmlns:android="http://schemas.android.com/apk/res/android "
xmlns:app="http://schemas.android.com/apk/res-auto "
xmlns:tools="http://schemas.android.com/tools "
android:id="@+id/nav_main"
app:startDestination="@id/homeFragment">
<fragment
android:id="@+id/homeFragment"
android:name="com.example.navtest.HomeFragment"
android:label="HomeFragment"
tools:layout="@layout/fragment_home">
<action
android:id="@+id/action_homeFragment_to_nestedFragment"
app:destination="@id/nestedFragment"
app:enterAnim="@anim/nav_default_enter_anim"
app:exitAnim="@anim/nav_default_exit_anim"
app:popEnterAnim="@anim/nav_default_pop_enter_anim"
app:popExitAnim="@anim/nav_default_pop_exit_anim" />
</fragment>
<fragment
android:id="@+id/nestedFragment"
android:name="com.example.navtest.NestedFragment"
android:label="fragment_nested"
tools:layout="@layout/fragment_nested" >
<argument
android:name="foo"
android:defaultValue='"foo"'
app:argType="string" />
</fragment>
<action
android:id="@+id/action_global_nestedFragment"
app:destination="@id/nestedFragment" />
<include app:graph="@navigation/nav_home" />
</navigation>
The compilation error arises after including the nested graph. From this, the error is avoided by either commenting out the global <action> or the <include> tag.
xmlns:app="
xmlns:tools="
android:id="@+id/nav_main"
app:startDestination="@id/homeFragment">
<fragment
android:id="@+id/homeFragment"
android:name="com.example.navtest.HomeFragment"
android:label="HomeFragment"
tools:layout="@layout/fragment_home">
<action
android:id="@+id/action_homeFragment_to_nestedFragment"
app:destination="@id/nestedFragment"
app:enterAnim="@anim/nav_default_enter_anim"
app:exitAnim="@anim/nav_default_exit_anim"
app:popEnterAnim="@anim/nav_default_pop_enter_anim"
app:popExitAnim="@anim/nav_default_pop_exit_anim" />
</fragment>
<fragment
android:id="@+id/nestedFragment"
android:name="com.example.navtest.NestedFragment"
android:label="fragment_nested"
tools:layout="@layout/fragment_nested" >
<argument
android:name="foo"
android:defaultValue='"foo"'
app:argType="string" />
</fragment>
<action
android:id="@+id/action_global_nestedFragment"
app:destination="@id/nestedFragment" />
<include app:graph="@navigation/nav_home" />
</navigation>
The compilation error arises after including the nested graph. From this, the error is avoided by either commenting out the global <action> or the <include> tag.
ap...@google.com <ap...@google.com> #4
Project: platform/frameworks/support
Branch: androidx-master-dev
commit 4855906b13c3adf6bf85833ebf28c3bba74e5969
Author: Daniel Santiago Rivera <danysantiago@google.com>
Date: Wed Sep 26 12:44:33 2018
Detect <include> graphs as an included destination and not a nested one.
Even though included graphs can be considered nested graph, these are
not currently parsed by the plugin and end up being a stub destination.
This causes problems when there is a global action since a class will
be generated for the nested graph containing the global action, but
since the stub destination has no id nor name, the plugin fails.
This change adds a model, 'IncludedDestination' that represents an
included graph detected by the parser and is stored in a different set
within the containing Destination model. The included graph is not
further discovered. However, by treating it different than a nested
destination we avoid the issue described above. Moreover, this change
helps toward later supporting parsing included graphs.
Bug: 116542123
Test: ./gradlew \
:navigation:navigation-safe-args-gradle-plugin:test \
:navigation:navigation-safe-args-generator:test
Change-Id: I9eba5706e0209247a3a599f8e84d56d36b97938e
M navigation/safe-args-generator/src/main/kotlin/androidx/navigation/safe/args/generator/Context.kt
M navigation/safe-args-generator/src/main/kotlin/androidx/navigation/safe/args/generator/NavParser.kt
M navigation/safe-args-generator/src/main/kotlin/androidx/navigation/safe/args/generator/NavParserErrors.kt
M navigation/safe-args-generator/src/main/kotlin/androidx/navigation/safe/args/generator/models/Destination.kt
A navigation/safe-args-generator/src/main/kotlin/androidx/navigation/safe/args/generator/models/IncludedDestination.kt
M navigation/safe-args-generator/src/tests/kotlin/androidx/navigation/safe/args/generator/InvalidXmlTest.kt
M navigation/safe-args-generator/src/tests/kotlin/androidx/navigation/safe/args/generator/NavParserTest.kt
A navigation/safe-args-generator/src/tests/test-data/invalid_xmls/invalid_include_graph_attr.xml
A navigation/safe-args-generator/src/tests/test-data/invalid_xmls/invalid_include_tag.xml
A navigation/safe-args-generator/src/tests/test-data/nested_include_login_test.xml
https://android-review.googlesource.com/769261
https://goto.google.com/android-sha1/4855906b13c3adf6bf85833ebf28c3bba74e5969
Branch: androidx-master-dev
commit 4855906b13c3adf6bf85833ebf28c3bba74e5969
Author: Daniel Santiago Rivera <danysantiago@google.com>
Date: Wed Sep 26 12:44:33 2018
Detect <include> graphs as an included destination and not a nested one.
Even though included graphs can be considered nested graph, these are
not currently parsed by the plugin and end up being a stub destination.
This causes problems when there is a global action since a class will
be generated for the nested graph containing the global action, but
since the stub destination has no id nor name, the plugin fails.
This change adds a model, 'IncludedDestination' that represents an
included graph detected by the parser and is stored in a different set
within the containing Destination model. The included graph is not
further discovered. However, by treating it different than a nested
destination we avoid the issue described above. Moreover, this change
helps toward later supporting parsing included graphs.
Bug: 116542123
Test: ./gradlew \
:navigation:navigation-safe-args-gradle-plugin:test \
:navigation:navigation-safe-args-generator:test
Change-Id: I9eba5706e0209247a3a599f8e84d56d36b97938e
M navigation/safe-args-generator/src/main/kotlin/androidx/navigation/safe/args/generator/Context.kt
M navigation/safe-args-generator/src/main/kotlin/androidx/navigation/safe/args/generator/NavParser.kt
M navigation/safe-args-generator/src/main/kotlin/androidx/navigation/safe/args/generator/NavParserErrors.kt
M navigation/safe-args-generator/src/main/kotlin/androidx/navigation/safe/args/generator/models/Destination.kt
A navigation/safe-args-generator/src/main/kotlin/androidx/navigation/safe/args/generator/models/IncludedDestination.kt
M navigation/safe-args-generator/src/tests/kotlin/androidx/navigation/safe/args/generator/InvalidXmlTest.kt
M navigation/safe-args-generator/src/tests/kotlin/androidx/navigation/safe/args/generator/NavParserTest.kt
A navigation/safe-args-generator/src/tests/test-data/invalid_xmls/invalid_include_graph_attr.xml
A navigation/safe-args-generator/src/tests/test-data/invalid_xmls/invalid_include_tag.xml
A navigation/safe-args-generator/src/tests/test-data/nested_include_login_test.xml
il...@google.com <il...@google.com> #5
This is fixed and will be available in 1.0.0-alpha07
Description
Version used: 1.0.0-alpha06
After updating the safe-args plugin from alpha05 to alpha06, my build fails with the following stacktrace:
org.gradle.api.tasks.TaskExecutionException: Execution failed for task ':app:generateSafeArgsDebug'
Caused by: java.lang.IllegalStateException: Destination with actions must have name
at androidx.navigation.safe.args.generator.NavWriterKt.generateDirectionsJavaFile(NavWriter.kt:419)
at androidx.navigation.safe.args.generator.NavSafeArgsGeneratorKt$generateSafeArgs$1.invoke(NavSafeArgsGenerator.kt:43)
at androidx.navigation.safe.args.generator.NavSafeArgsGeneratorKt$generateSafeArgs$1.invoke(NavSafeArgsGenerator.kt:55)
at androidx.navigation.safe.args.generator.NavSafeArgsGeneratorKt.generateSafeArgs(NavSafeArgsGenerator.kt:58)
at androidx.navigation.safeargs.gradle.ArgumentsGenerationTask.generateArgs(ArgumentsGenerationTask.kt:54)
at androidx.navigation.safeargs.gradle.ArgumentsGenerationTask.doFullTaskAction(ArgumentsGenerationTask.kt:90)
at androidx.navigation.safeargs.gradle.ArgumentsGenerationTask.taskAction$navigation_safe_args_gradle_plugin(ArgumentsGenerationTask.kt:79)
Reverting to alpha05 makes the build complete.
I think this is related to the commit addressing this issue:
My navigation graph has this structure:
<navigation>
<fragment>
<action />
<argument />
</fragment>
<include app:graph="@navigation/nav_graph_two" />
...
<action />
</navigation>
That is, there are global actions, fragment destinations with arguments, fragment destinations with actions and an included nested graph (in a separate xml). Every single destination has a valid name and every root <navigation> has an id (everything works with version alpha05 of the plugin). I might guess this is related with global actions, which expect the parent destination to have a name that is not resolved with the alpha06 update. In fact, commenting out all the global actions makes the generateSafeArgsDebug task pass. My graphs have ids in the form "@+id/nav_graph_one".