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

Use overloaded AndroidView in MarkdownHelper #767

Merged
merged 4 commits into from
Jul 3, 2023

Conversation

yate
Copy link
Contributor

@yate yate commented Jun 20, 2023

From what I've tested this mostly fixes #730. There's still a bit of a jerk upwards as the images loads in so it's not perfect, but it prevents the endless scroll downwards when hitting an image as shown in the issue.

I'm not entirely sure if anything needs to go in the onReset method, but it's noted in the Jetpack Compose documentation to use this overload with LazyColumns because the view can be reused during recomposition, so it may help with performance as well.

device-2023-06-19-225611.webm

@dessalines
Copy link
Member

The example you linked shows that reset should include view.clear()

@yate
Copy link
Contributor Author

yate commented Jun 20, 2023

I looked more into that example, and that clear() doesn't seem to do anything either

The documentation says the callback "may be used to reset any transient View state like animations or user input.". I don't think that applies here?

@ZJouba
Copy link
Contributor

ZJouba commented Jun 20, 2023

Okay, after taking a look in LayoutInspector, I believe this is caused by the View being reused and the image being discarded.

When the comment is inside the viewport everything looks fine, we can see the markdown helper in the AndroidView
Screenshot 2023-06-20 180827

However, when we scroll the image out of the viewport we see the AndroidView is empty
Screenshot 2023-06-20 181110

Then, I believe when it is scrolled back into view, the image gets reloaded and something forces the scroll to focus on the image. What's weird to me is that the height of the AndroidView remains constant at 504dp so why does the viewport suddenly jump to include the whole image? It's not a matter of layout shift.

@dessalines
Copy link
Member

I'm sure its lazylist weirdness, might be worth trying to mess around with the markwon pictrs coil image sizes.

@ZJouba
Copy link
Contributor

ZJouba commented Jun 21, 2023

Tsk, this is what we need but it's not available for LazyColumn yet https://issuetracker.google.com/issues/172029355

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.

Not sure that this onReset will do anything, but I resolved the conflicts.

@dessalines dessalines enabled auto-merge (squash) July 3, 2023 13:14
@dessalines dessalines disabled auto-merge July 3, 2023 23:59
@dessalines dessalines merged commit 83cf6c9 into LemmyNet:main Jul 3, 2023
MV-GH added a commit to MV-GH/jerboa that referenced this pull request Jul 13, 2023
dessalines pushed a commit that referenced this pull request Jul 14, 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.

Scrolling up past images in the comments jerks you back down
4 participants