Add support to save non-mutable int in StateSnapshot #1508
Conversation
|
The focus of this draft is to support save & restore After ^, I copied changes from #1494 over to test restoring scroll position, but it's not working, @veyndan can you take a look 🙏? |
| public companion object { | ||
| /** | ||
| * The default [Saver] implementation for [LazyListState]. | ||
| */ | ||
| public val Saver: Saver<LazyListState, *> = Saver( | ||
| save = { it.firstVisibleItemIndex }, | ||
| restore = { | ||
| LazyListState( | ||
| firstVisibleItemIndex = it, | ||
| ) | ||
| } | ||
| ) | ||
| } |
There was a problem hiding this comment.
this is copy-paste from #1494, and will be reverted once this PR goes from draft -> ready for review
|
What doesn't work with the PR? Is it that saving a plain |
this^ |
|
Cool, so if saving a plain |
yep, sounds good to me, will clean up this PR and put it up for review |
jingwei99
force-pushed
the
jingwei.support_int_in_StateSnapshot
branch
from
37a15b8 to
e3ec6ce
Compare
jingwei99
force-pushed
the
jingwei.support_int_in_StateSnapshot
branch
from
e3ec6ce to
c22dd46
Compare
| import kotlin.test.assertTrue | ||
| import kotlinx.serialization.json.JsonPrimitive | ||
|
|
||
| class StateSnapshotTest { |
| assertThat(valuesMap.entries.size).isEqualTo(4) | ||
| assertTrue(valuesMap["key1"]!![0] is MutableState<*>) | ||
| assertThat((valuesMap["key1"]!![0] as MutableState<*>).value).isEqualTo(JsonPrimitive(1)) | ||
|
|
||
| assertThat(valuesMap["key2"]).isEqualTo(listOf(1.0)) | ||
|
|
||
| assertThat(valuesMap["key3"]!![0] is MutableState<*>) | ||
| assertThat((valuesMap["key3"]!![0] as MutableState<*>).value).isEqualTo(JsonPrimitive("str")) | ||
|
|
||
| assertThat(valuesMap["key4"]).isEqualTo(listOf("str")) |
There was a problem hiding this comment.
Could these test cases be simplified with the containsOnly function?
assertThat(valuesMap).containsOnly(
"key1" to listOf(mutableStateOf(JsonPrimitive(1)),
…
)There was a problem hiding this comment.
containsOnly errors out:
expected to contain only:<{"key1"=[MutableState(value=1)@916835004], "key2"=[1.0], "key3"=[MutableState(value="str")@2108297149], "key4"=["str"]}> but was:<{"key1"=[MutableState(value=1)@1112737073], "key2"=[1.0], "key3"=[MutableState(value="str")@1513867245], "key4"=["str"]}> elements not found:<{"key1"=[MutableState(value=1)@916835004], "key3"=[MutableState(value="str")@2108297149]}> extra elements found:<{"key1"=[MutableState(value=1)@1112737073], "key3"=[MutableState(value="str")@1513867245]}>
for the MutableState ones, containsOnly doesn't unwrap and compare the values (it compares the instances)
| assertThat(stateSnapshot.content["key1"]!![0]).isEqualTo(Saveable(true, JsonPrimitive(1))) | ||
| assertThat(stateSnapshot.content["key2"]!![0]).isEqualTo(Saveable(false, JsonPrimitive(1))) | ||
| assertThat(stateSnapshot.content["key3"]!![0]).isEqualTo(Saveable(true, JsonPrimitive("str"))) | ||
| assertThat(stateSnapshot.content["key4"]!![0]).isEqualTo(Saveable(false, JsonPrimitive("str"))) |
There was a problem hiding this comment.
Same idea with these test cases
| if (it.isMutableState) { | ||
| mutableStateOf(it.value) | ||
| } else { | ||
| it.value.fromJsonElement() | ||
| } |
There was a problem hiding this comment.
We could remove the need for the Saveable wrapper class if we wrote an adapter to convert between MutableState and JsonElement. Perhaps it isn't worth the effort though, considering we'll change up this mechanism in #1384.
| } | ||
| return booleanOrNull ?: doubleOrNull ?: error("unexpected type: $this") | ||
| // TODO add other primitive types (double, float, long) when needed | ||
| if (this.isString) { return content } |
There was a problem hiding this comment.
This looks a bit odd having the braces on a single line. Can we revert it, or do if (this.isString) return content?
Tested locally, saving and restoring rememberSaveable of
non-mutableStateworks as expected