Support fromSavedStateHandle in SafeArgs [136967621]

Fixed

Feature Request

androidx-bug-bounty

Status Update

No update yet.

Description

jo...@gmail.com

created issue #1

Jul 8, 2019 06:35PM

Components used:
androidx.navigation:navigation-safe-args-gradle-plugin:2.0.0
androidx.lifecycle:lifecycle-viewmodel-savedstate:1.0.0-alpha02

Allow SafeArgs to be created `fromSavedStateHandle` as one would from a bundle.

Comments

il...@google.com <il...@google.com> #2Jul 8, 2019 10:01PM

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

jo...@gmail.com <jo...@gmail.com> #3Jul 9, 2019 04:32PM

I am using Safe Args from Navigation and also using SavedStateHandle to pass in arguments from the args bundle to my ViewModel. This gives me access to them through the constructor. Unfortunately I then have to use string keys to get values out of the SavedStateHandle instead of using the generated SafeArgs.

ka...@gmail.com <ka...@gmail.com> #4Jul 7, 2020 07:00AM

Dagger Hilt has been released in alpha. It greatly simplifies SavedStateHandle injection into ViewModel. Soon more and more people would want this feature.

[Deleted User] <[Deleted User]> #5Jul 22, 2020 07:06PM

Currently the combination of Hilt, SavedStateHandle and Safe args is not possible. The closest I've seen is to drop using safe args and just use magic strings. This also not ideal re dependency inversion as now you have to construct a mock SavedStateHandle for unit tests etc.

class ArticleViewModel @ViewModelInject constructor(
        private val repository: ArticleRepository,
        @Assisted savedStateHandle: SavedStateHandle
) {
    // FIXME: oops, magic "articleId" string
    private val articleId: Long = savedStateHandle["articleId"]
            ?: throw IllegalArgumentException("Article ID required")
}

g....@gmail.com <g....@gmail.com> #6Sep 27, 2020 08:21AM

I thought I needed this, but then I realized that I can just create an empty Bundle, store the Fragment's arguments in it as a Bundle (so a Bundle inside a Bundle) and use that to initialize the SavedStateHandle. Then I can just retrieve the arguments from the SavedStateHandle and use the already existing fromBundle method.

What I like about this is that I get to have immutable init arguments, as long as I store the Fragment's arguments with a key that I would never use for other entries.

With a helper function and a base ViewModel class you can hide this entire process and have ViewModels that expose the arguments object as property/member of the ViewModel itself.

ka...@gmail.com <ka...@gmail.com> #7Oct 11, 2020 09:49AM

Could you provide an example of how you achieve that?

g....@gmail.com <g....@gmail.com> #8Oct 11, 2020 11:38AM

What I wrote requires the ability to provide a Bundle to initialize the SavedStateHandle and currently that's not possible with Hilt. You need to wait for this to be fixed: https://github.com/google/dagger/issues/1938 (https://android-review.googlesource.com/c/platform/frameworks/support/+/1427172).

If you do not use Hilt, then what I wrote becomes straightforward. Rather than passing the Fragment's arguments directly, you provide a Bundle with the Fragment's arguments in it:

Bundle().apply { putBundle(SOME_SPECIAL_BUNDLE_KEY, arguments) }

By doing so you'll be able to do the following in your ViewModel:

val fragmentArgs = savedStateHandle[SOME_SPECIAL_BUNDLE_KEY]
val args = YouFragmentArgs.fromBundle(fragmentArgs)

You still need to use a magic key for the Bundle, but it can be a project-wide constant and you only have to pass a single argument. The initialization is also the same for all the ViewModels, so you can easily write helpers to abstract away the process.

al...@linguistica360.com <al...@linguistica360.com> #9Nov 12, 2020 11:39PM

If you are using hilt you can wrap the incoming arguments in fragment like so

open class BaseFragment : Fragment() {

    override fun setArguments(args: Bundle?) {
        if (args != null) {
            super.setArguments(Bundle(args).apply {
                putBundle(BUNDLE_ARGS, args) // Wrap the arguments as BUNDLE_ARGS
            })
        } else {
            super.setArguments(null)
        }
    }
}

and then for the view model, you can inherit from something like that

open class ArgsViewModel(private val savedStateHandle: SavedStateHandle) : ViewModel() {

    val arguments get() = savedStateHandle.get<Bundle>(BUNDLE_ARGS)

    @MainThread
    inline fun <reified Args : NavArgs> navArgs() = NavArgsLazy(Args::class) {
        arguments ?: throw IllegalStateException("ViewModel $this has null arguments")
    }
}

Hope that helps

si...@gmail.com <si...@gmail.com> #10Mar 2, 2021 07:28PM

Should be fixed by https://github.com/androidx/androidx/pull/122

il...@google.com <il...@google.com> Mar 2, 2021 08:08PM

Marked as fixed, reassigned to jb...@google.com.

al...@linguistica360.com <al...@linguistica360.com> #11Mar 3, 2021 10:24PM

Do you know when this will be released?

il...@google.com <il...@google.com> #12Mar 3, 2021 10:27PM

Re #11 - The public hotlist listed at the top of this bug says "androidx-navigation-2.4.0-alpha01" - that's the expected release.

ho...@gmail.com <ho...@gmail.com> #13May 31, 2021 10:31AM

It seems this has been included in the 2.4.0-alpha01 release.

But is there any additional setup required besides updating the navigation dependencies? With the following dependencies on my project

object Navigation {
    private const val version = "2.4.0-alpha01"
    const val compose = "androidx.navigation:navigation-compose:$version"
    const val fragment = "androidx.navigation:navigation-fragment-ktx:$version"
    const val safeArgsPlugin = "androidx.navigation:navigation-safe-args-gradle-plugin:$version"
    const val ui = "androidx.navigation:navigation-ui-ktx:$version"
}

my safeargs don't generate fromSavedStateHandle().

fromBundle() is still the only auto-complete suggestion.

FooFragmentArgs.fromSavedStateHandle(savedStateHandle) is an unresolved reference in my ViewModel.

ok...@gmail.com <ok...@gmail.com> #14Jun 1, 2021 11:38AM

I agree with the previous comment. FooFragmentArgs.fromSavedStateHandle(savedStateHandle) is unresolved reference in my ViewModel too. When I checked on generated FooFragmentArgs class, fromSavedStateHandle function is there but Android Studio can't find that reference. Code compiles successfully.

Android Studio : 4.2.1
com.android.tools.build:gradle:4.2.1
androidx.navigation:navigation-safe-args-gradle-plugin:2.4.0-alpha01

Is there any thing we can do about this?

il...@google.com <il...@google.com> #15Jun 1, 2021 05:02PM

Re #13, #14 - please upgrade to the latest version of Android Studio and file a bug against Android Studio if you are still not seeing this method in the Studio generated light classes.

ho...@gmail.com <ho...@gmail.com> #16Jun 3, 2021 09:07AM

I'm using Arctic Fox | 2020.3.1 Canary 9. I've just updated Navigation to 2.4.0-alpha02 and still seeing the bug, so I've filed a bug against AS. https://issuetracker.google.com/issues/189966590

ho...@gmail.com <ho...@gmail.com> #17Jun 4, 2021 07:15AM

As mentioned in the linked issue, manually downloading the latest version of Android Studio Preview fixes the issue