Skip to content
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

Move unExpandedComments and commentsWithToggledActionBar from PostActivity to PostViewModel #1068

Merged
merged 7 commits into from
Jul 21, 2023

Conversation

twizmwazin
Copy link
Contributor

@twizmwazin twizmwazin commented Jul 20, 2023

Right now, if you've collapsed some comments or toggled their action bars and then navigate to a new page, when you return back to that post the comment state is reset because it is tied to the composable rather than the view model. Moving this state to the view model fixes this issue.

Is there other state where we would want to do this as well?

Fixes #643

@MV-GH
Copy link
Collaborator

MV-GH commented Jul 20, 2023

fix the formatting, would this also fix #643?

@twizmwazin
Copy link
Contributor Author

Most likely, yes. I'll test it.

@MV-GH
Copy link
Collaborator

MV-GH commented Jul 20, 2023

Would this also work for saving the current scroll position? #952

@twizmwazin
Copy link
Contributor Author

Any state that we want to tie to the lifetime of the ViewModel this method should work for. So, if the HomeViewModel isn't destroyed and recreated when navigating on the navbar between tabs, then this method should work. A case where this wouldn't work is when a user backs out of a post and then re-opens the post from the home or community view, because that would cause the view model to be destroyed and then recreated.

@MV-GH
Copy link
Collaborator

MV-GH commented Jul 20, 2023

I see, in that case i don't think it will work though.

Copy link
Member

@dessalines dessalines left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested a few scenarios:

action works
Navigating from post -> a user or community page Yes
Navigating from post to a create / edit comment Yes
Navigating from post back to home, then same post No

I'd say even with those two improvements, this is good.

@MV-GH MV-GH enabled auto-merge (squash) July 20, 2023 18:19
@twizmwazin
Copy link
Contributor Author

If we want to fix "Navigating from post back to home, then same post", then we would probably need to move the postviewmodel reference to the home/community activity instead of the post activity, and then when a user opens a post, re-use that viewmodel if it is the same as the previously opened post, otherwise replace it. I can give this a shot later.

@MV-GH MV-GH merged commit abec22b into LemmyNet:main Jul 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Comments Uncollapse Upon Reply
3 participants