Fixed
Status Update
No update yet.
Comments
il...@google.com
Project: platform/frameworks/support
Branch: androidx-main
commit c2365dcfeb9064814282304f30f3eb86be71fdc7
Author: Dan Nizri <dniz@google.com>
Date: Wed Dec 20 16:33:08 2023
Fix Popup back handling on Android T/13+ when android:enableOnBackInvokedCallback="true"
Bug: 313702338
Bug: 318537200
Test: tested with android:enableOnBackInvokedCallback="true"
Change-Id: I7c7cef686ed9fbfd2d144c90040b13528f1675ed
M compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/window/AndroidPopup.android.kt
https://android-review.googlesource.com/2886766
Branch: androidx-main
commit c2365dcfeb9064814282304f30f3eb86be71fdc7
Author: Dan Nizri <dniz@google.com>
Date: Wed Dec 20 16:33:08 2023
Fix Popup back handling on Android T/13+ when android:enableOnBackInvokedCallback="true"
Bug: 313702338
Bug: 318537200
Test: tested with android:enableOnBackInvokedCallback="true"
Change-Id: I7c7cef686ed9fbfd2d144c90040b13528f1675ed
M compose/ui/ui/src/androidMain/kotlin/androidx/compose/ui/window/AndroidPopup.android.kt
ec...@gmail.com
Is there remaining work for this feature request. Support for predictive back in Dialogs would allow Material3 to remove some Dialog forks and would be very helpful!
ec...@gmail.com
Yeah, we probably want to investigate new API shapes here to take advantage of predictive back. No work ongoing yet though, needs prioritization
il...@google.com
Pinging here, as we're facing a similar issue on Wear OS, PB support on Dialogs would allow us to simplify/remove some components that handle swipe to dismiss behaviour and consolidate the back gesture throughout the system.
ec...@gmail.com
(recapping my chat with Ian)
The tricky part is that the onDismissRequest is really the final signal and a chance for the app to just say 'nope, I don't want the dialog to dismiss' (since they own the state of 'should the dialog be shown or not' simply by either adding the Dialog to composition or leaving out of composition) - that's the just-in-time problem that the comment talks about. It would be very odd to have any kind of progress animation that doesn't actually lead to the dialog being dismissed when it is committed"
so really it needs to be a more holistic look at how to do dialogs and swap from a binary state (shown/not shown) to something that actually has something in between for cases like predictive back
il...@google.com
Can you include a sample that reproduces your issue?
ec...@gmail.com
Ok. I'm confused. I'll write here, at least for my own investigation.
What I described in #4, that really happens. But that is solved by simply adding defaultValue="@null" in the XML, like people said somewhere else on the webs. By doing that, null is added to the corresponding field in the constructor of the Args class. This thus invalidates #6.
But, the deep link part doesn't like defaultValue="@null" when I try to navigate (through navigation component) with an URI that does not have that argument/parameter. As described in #1, the string parameter defined with defaultValue="@null" gets the string "@null". In short: navigating directly gives that argument with the default value null (from the Args class), but navigating with deepLink gives "@null".
I've also found that optional parameter in deepLinks has been introduced in the change linked herehttps://issuetracker.google.com/issues/133273839#comment3 , which includes the code I referenced in #1.
I can workaround all this by adjusting my code to use empty string as the marker for not present field, and setting defaultValue to "". The editor doesn't like it ("missing value"), but that's another issue.
What I described in #4, that really happens. But that is solved by simply adding defaultValue="@null" in the XML, like people said somewhere else on the webs. By doing that, null is added to the corresponding field in the constructor of the Args class. This thus invalidates #6.
But, the deep link part doesn't like defaultValue="@null" when I try to navigate (through navigation component) with an URI that does not have that argument/parameter. As described in #1, the string parameter defined with defaultValue="@null" gets the string "@null". In short: navigating directly gives that argument with the default value null (from the Args class), but navigating with deepLink gives "@null".
I've also found that optional parameter in deepLinks has been introduced in the change linked here
I can workaround all this by adjusting my code to use empty string as the marker for not present field, and setting defaultValue to "". The editor doesn't like it ("missing value"), but that's another issue.
jb...@google.com
We would like to investigate as well. Do you have a sample project that reproduces the issues that you can upload as a part of this bug?
ec...@gmail.com
Here it goes:
- in order to avoid dealing with initial boilerplate, clonedhttps://github.com/googlecodelabs/android-navigation ;
- uncomented TODO from the codelab steps;
- updated deps, plugins versions, fixed some compiling issue;
- removed code, left Home and DeepLinkFragment only;
- changed app:uri to
"www.example.com/query?param1={param1}¶m2={param2} ";
- added arguments in deepLinkFrangment in mobile_navigation.xml:
<argument
android:name="param1"
android:defaultValue="@null"
app:nullable="true"/>
<argument
android:name="param2"
android:defaultValue="@null"
app:nullable="true"/>
- added action in homeFragment targeting deepLinkFragment
<action
android:id="@+id/deeplink_action"
app:destination="@+id/deeplink_dest" />
- added a button and the following code to HomeFragment, using action generated code (notice that no argument is passed because they default to null):
view.findViewById<Button>(R.id.navigate_direct_button)?.setOnClickListener {
findNavController().navigate(HomeFragmentDirections.deeplinkAction(param1 = "HELLO"))
}
- added a button and the following code to HomeFragment, using Uri (notice that param2 is absent):
view.findViewById<Button>(R.id.navigate_deeplink_button)?.setOnClickListener {
findNavController().navigate(
Uri.parse("example://www.example.com/query?param1=HELLO "))
}
(had to change app:uri from "www.example.com " to "example://www.example.com/.. ." because the first wasn't matching (another issue i guess))
- adjusted DeepLinkFragment to show param1 and param2, from the arguments bundle (could have used by navArgs with same result)
textView.text = arguments?.getString("param1", "") + arguments?.getString("param2", "")
Running the app, when the "navigate with direct action" button is clicked, "HELLO" is shown in the next fragment, as expected. When "navigate with deeplink" is clicked, "HELLO@null" is shown, which is the issue.
- in order to avoid dealing with initial boilerplate, cloned
- uncomented TODO from the codelab steps;
- updated deps, plugins versions, fixed some compiling issue;
- removed code, left Home and DeepLinkFragment only;
- changed app:uri to
"
- added arguments in deepLinkFrangment in mobile_navigation.xml:
<argument
android:name="param1"
android:defaultValue="@null"
app:nullable="true"/>
<argument
android:name="param2"
android:defaultValue="@null"
app:nullable="true"/>
- added action in homeFragment targeting deepLinkFragment
<action
android:id="@+id/deeplink_action"
app:destination="@+id/deeplink_dest" />
- added a button and the following code to HomeFragment, using action generated code (notice that no argument is passed because they default to null):
view.findViewById<Button>(R.id.navigate_direct_button)?.setOnClickListener {
findNavController().navigate(HomeFragmentDirections.deeplinkAction(param1 = "HELLO"))
}
- added a button and the following code to HomeFragment, using Uri (notice that param2 is absent):
view.findViewById<Button>(R.id.navigate_deeplink_button)?.setOnClickListener {
findNavController().navigate(
Uri.parse("example://
}
(had to change app:uri from "
- adjusted DeepLinkFragment to show param1 and param2, from the arguments bundle (could have used by navArgs with same result)
textView.text = arguments?.getString("param1", "") + arguments?.getString("param2", "")
Running the app, when the "navigate with direct action" button is clicked, "HELLO" is shown in the next fragment, as expected. When "navigate with deeplink" is clicked, "HELLO@null" is shown, which is the issue.
b9...@gmail.com
I'm facing this issue as well, default null seems never be null.
The root cause seems in the androidx.navigation.NavDeepLink.java:
// Missing parameter so see if it has a default value or is Nullable
if (argument != null
&& (value == null || value.replaceAll("[{}]", "").equals(argName))) {
if (argument.getDefaultValue() != null) {
value = argument.getDefaultValue().toString();
} else if (argument.isNullable()) {
value = "@null";
}
}
The root cause seems in the androidx.navigation.NavDeepLink.java:
// Missing parameter so see if it has a default value or is Nullable
if (argument != null
&& (value == null || value.replaceAll("[{}]", "").equals(argName))) {
if (argument.getDefaultValue() != null) {
value = argument.getDefaultValue().toString();
} else if (argument.isNullable()) {
value = "@null";
}
}
ra...@gmail.com
+1
arguments?.run {
val args by navArgs<BFragmentArgs>()
//Should be null instead "@null"?
Log.d("ARGS", "Token ${args.token}")
}
arguments?.run {
val args by navArgs<BFragmentArgs>()
//Should be null instead "@null"?
Log.d("ARGS", "Token ${args.token}")
}
ra...@gmail.com
It would be helpful if defaultValue is really null, instead "@null" on deeplink cases like com.sample://fragmentB?token= (that we could receive by mistake from backend/external service)
<argument
android:name="token"
android:defaultValue="@null"
app:argType="string"
app:nullable="true" />
<deepLink
android:id="@+id/cta"
app:uri="com.sample://fragmentB?token={token}" />
<argument
android:name="token"
android:defaultValue="@null"
app:argType="string"
app:nullable="true" />
<deepLink
android:id="@+id/cta"
app:uri="com.sample://fragmentB?token={token}" />
ap...@google.com
Project: platform/frameworks/support
Branch: androidx-master-dev
commit 6e93d1d7ee8ac63451e8822422fb3b1da90fa321
Author: Jeremy Woods <jbwoods@google.com>
Date: Mon Nov 25 15:29:59 2019
Make nullable deep link params default to null
Currently a deep link param that cannot be matched but is marked as
nullable will be stored in the argument bundle with a value of "@null".
This causes the behavior for a missing param to be inconsistent between
a normal navigation and navigation from a deep link.
This change stores null an instance of null in the bundle for the value
instead to ensure we mimic the behavior of non-deep link nullable
params.
Test: adjusted NavDeepLinkTest
BUG: 141613546
Change-Id: I14625699fc9365088e07df9456a05257974a375b
M navigation/navigation-common/src/androidTest/java/androidx/navigation/NavDeepLinkTest.kt
M navigation/navigation-common/src/main/java/androidx/navigation/NavDeepLink.java
https://android-review.googlesource.com/1174705
Branch: androidx-master-dev
commit 6e93d1d7ee8ac63451e8822422fb3b1da90fa321
Author: Jeremy Woods <jbwoods@google.com>
Date: Mon Nov 25 15:29:59 2019
Make nullable deep link params default to null
Currently a deep link param that cannot be matched but is marked as
nullable will be stored in the argument bundle with a value of "@null".
This causes the behavior for a missing param to be inconsistent between
a normal navigation and navigation from a deep link.
This change stores null an instance of null in the bundle for the value
instead to ensure we mimic the behavior of non-deep link nullable
params.
Test: adjusted NavDeepLinkTest
BUG: 141613546
Change-Id: I14625699fc9365088e07df9456a05257974a375b
M navigation/navigation-common/src/androidTest/java/androidx/navigation/NavDeepLinkTest.kt
M navigation/navigation-common/src/main/java/androidx/navigation/NavDeepLink.java
jb...@google.com
This has been fixed internally and will be available in the Navigation 2.2.0-rc03 release.
Description
A parameter of argType string, nullable and defaultValue="@null", gives bundle with "@null" string. We would expect the bundle item to be null, not the string "@null".
From what I could track, there's something strange around NavDeepLink.java#153 and in NavType#parseValue for the string type.