voyager icon indicating copy to clipboard operation
voyager copied to clipboard

rememberSaveable doesn't work since 1.0.0-beta08

Open Grouen opened this issue 3 years ago • 8 comments

To reproduce this issue, just enable "Dont't keep activites" and use this test screen.

class TestScreen : Screen {
    @Composable
    override fun Content() {
        var text by rememberSaveable {
            mutableStateOf("")
        }

        Column {
            TextField(value = text, onValueChange = { text = it })
        }
    }
}

rememberSaveable must work because some components used it under the hood to restore scroll position and other things.

Grouen avatar Nov 29 '21 02:11 Grouen

As I can see the value is in rememberSaveable but something wrong in default key calculation when we use it in Screen.Content. If I set key manually, this works fine, but it's still a problem.

Grouen avatar Nov 29 '21 03:11 Grouen

Sorry for taking too long. Just tried to reproduce with your TestScreen and "Don't keep activities" enabled, seems the bug was solved in past releases.

Feel free to reopen the issue if still happens with you.

adrielcafe avatar Apr 30 '22 16:04 adrielcafe

Back to voyager 1.0 RC6 and I'm facing this too.

I'm logging the key in an AndroidScreen and it's properly restored after activity kill.

But rememberSaveable by default does not work, passing a key does work and the value is properly restored.

So

    var curPage by rememberSaveable(key = "Test") {
        mutableStateOf(0)
    }

Will properly restore any changed value

But

    var curPage by rememberSaveable {
        mutableStateOf(0)
    }

will not

This is painful as many Compose rememberXXXX use internal rememberSaveable and we can't pass key there like rememberPagerState and others like rememberLauncherForActivityResult ...

Can this be reopened ? @adrielcafe @DevSrSouza

Tolriq avatar May 15 '23 15:05 Tolriq

Bump

Tolriq avatar May 19 '23 07:05 Tolriq

We will investigate

DevSrSouza avatar May 20 '23 12:05 DevSrSouza

Unable to reproduce it here.

I add a simple BasicNavigationScreen and it seems that everything is being proper restaured.

var memoryState by rememberSaveable {
    mutableStateOf(0)
}
Text("Memory State: $memoryState", modifier = Modifier.clickable { memoryState++ })

adb shell am kill cafe.adriel.voyager.sample

Can you share more context and more examples @Tolriq ?

DevSrSouza avatar May 20 '23 12:05 DevSrSouza

I'm deeper in the hierarchy and using Compose 1.5 but basically I'm having AndroidScreen in different modules.

Like :

class ProviderLocalDeviceAddScreen : AndroidScreen() {

    override val key: ScreenKey
        get() = "ProviderLocalDeviceAdd${super.key}"

    @Composable
    override fun Content() {
        val viewModel = getScreenModel<ProviderLocalDeviceAddScreenViewModel>()
        ProviderLocalDeviceAddScreenContent(viewModel)
    }

    @Module
    @InstallIn(SingletonComponent::class)
    internal class Navigation {

        @Provides
        @IntoMap
        @NavigationDestinationKey(ProviderLocalDeviceAddDestination::class)
        fun provideDestination(): OptionalScreenProvider<*> {
            return OptionalScreenProvider<ProviderLocalDeviceAddDestination> {
                ProviderLocalDeviceAddScreen()
            }
        }
    }
}

In the @Composable ProviderLocalDeviceAddScreenContent I have something simple like :

    val launcher = rememberLauncherForActivityResult(AdvancedOpenDocumentTree()) {
        scope.launch {
            viewModel.handleResult(it)
        }
    }

    var mediaMode by rememberSaveable {
        mutableStateOf(0)
    }
   Text("Memory State: $memoryState", modifier = Modifier.clickable { mediaMode ++ })
   Text("Launch", modifier = Modifier.clickable {  launcher.launch(null) })

With the dev option do not keep activity enabled.

On return from the SAF selection, the mediaMode will be reset to 0 and not restored.

If I change to

    var mediaMode by rememberSaveable(key = "toto") {
        mutableStateOf(0)
    }

Then it properly works. (But we can't do that for most of the Compose internal stuff.

I have absolutely no idea where to start to debug the ComposeHashKey generation issue.

Edit: And as said in previous, the Android screen key is properly restored and the same when logged.

Tolriq avatar May 20 '23 13:05 Tolriq

Ok so I did more tests and seems the issue is about FadeTransition @DevSrSouza FadeTransition(navigator) to navigator.lastItem.Content() fix this. (But I'd love to have transitions :)

Tolriq avatar May 20 '23 13:05 Tolriq