-
Notifications
You must be signed in to change notification settings - Fork 15
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: persist scroll state #170
feat: persist scroll state #170
Conversation
@abGit9 Thanks for the PR! Do you think it is important to keep the scroll state across multiple app sessions? The comment I added to your previous PR was mainly about keeping the scroll position when switching between grid and normal layout, not across multiple app sessions. |
Hi @DennisBauer. So preserving the scroll position during a single app session, especially when toggling between grid and standard layouts, is certainly more essential and fundamental than between app sessions. However, do you think that maintaining the scroll state across multiple app sessions, although not as critical, provides a nice touch of user experience enhancement when the app is relaunched? I actually got the idea from you during this PR #156 (comment) |
@abGit9 I like the idea, but code-wise it looks quite heavy and introduces overhead that could lead to more bugs. So I'm not entirely convinced of the need to persist the scroll state across multiple app sessions. I can't really think of other apps that persist scroll states, even for those with long lists. So I would rather just persist the scroll state within a single app session. |
Sure, that's fine. Across sessions is not required. Within sessions it is certainly more essential. I'll go ahead and reset the branch to e4fd488. Thanks. |
b217b00
to
e4fd488
Compare
@@ -57,6 +58,10 @@ fun RecurringExpenseOverview( | |||
contentPadding: PaddingValues = PaddingValues(0.dp), | |||
) { | |||
val fadeDuration: Int = integerResource(id = R.integer.overview_list_grid_toggle_fade_anim_duration) | |||
|
|||
val listState = rememberLazyStaggeredGridState() | |||
val gridState = rememberLazyStaggeredGridState() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need two different states? I'm also fine with it, just asking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just tested it myself and I think it is easier with two states 👍
e4fd488
to
2477947
Compare
Thank you! |
This pull request makes the following changes:
Addresses : #159 (comment)
Addresses issue #117
Maintains scroll position within user sessions.