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.

Sign up for GitHub

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

Jump to bottom

chat "scrolls up" right after switching to it, because images load and shift height #4404

Open
WofWca opened this issue Dec 11, 2024 · 3 comments · May be fixed by #4431
Open

chat "scrolls up" right after switching to it, because images load and shift height #4404

WofWca opened this issue Dec 11, 2024 · 3 comments · May be fixed by #4431
Labels
bug Something isn't working

Comments

@WofWca
Copy link
Collaborator

WofWca commented Dec 11, 2024 •
edited
Loading

  • Operating System (Linux/Mac/Windows/iOS/Android): Windows
  • Delta Chat Version: 1.48.0
  • Expected behavior: Chat loads with the last message at the very bottom
  • Actual behavior: First the chat is scrolled all the way to the bottom, but after a few milliseconds "scrolls up"
  • Steps to reproduce the problem:
    1. Open a chat where one of the last messages is an image
  • Screenshots:
  • Logs:

@maxphilippov suggested what is likely to be the solution here #4315 (comment), with an extra comment from me #4315 (comment).

Update: the issue per se has been fixed in #4407, which sort of reverts the MR that introduced it (#4315), but we might want to come back to dynamic image sizes.
@WofWca WofWca added the bug Something isn't working label Dec 11, 2024
@WofWca
Copy link
Collaborator Author

WofWca commented Dec 12, 2024

Turns out this was introduced recently, in #4315

WofWca added a commit that referenced this issue Dec 12, 2024
@WofWca
fix: chat not being scrolled to bottom (images)
5bb3710 
Closes #4404.

The bug was introduced in db83279.
Prior to that all attachments had a fixed height of 200px.

This should also eliminate layout shifting when scrolling the chat,
and when new messages arrive.

An alternative solution that would still have layout shifting,
but at least keep the chat "anchored" to bottom would be
#3753 (comment),
but it's not supported well by browsers.
@WofWca WofWca mentioned this issue Dec 12, 2024
Closed
WofWca added a commit that referenced this issue Dec 12, 2024
@WofWca @maxphilippov
fix: chat not being scrolled to bottom (images)
af0c8a4 
Closes #4404.

The bug was introduced in db83279.
Prior to that all attachments had a fixed height of 200px.

This should also eliminate layout shifting when scrolling the chat,
and when new messages arrive.

An alternative solution that would still have layout shifting,
but at least keep the chat "anchored" to bottom would be
#3753 (comment),
but it's not supported well by browsers.

Co-authored-by: Max Philippov <[email protected]>
@WofWca WofWca self-assigned this Dec 12, 2024
WofWca added a commit that referenced this issue Dec 12, 2024
@WofWca @maxphilippov
fix: chat not being scrolled to bottom (images)
1d2508f 
Closes #4404.

The bug was introduced in db83279.
Prior to that all attachments had a fixed height of 200px.

This should also eliminate layout shifting when scrolling the chat,
and when new messages arrive.

An alternative solution that would still have layout shifting,
but at least keep the chat "anchored" to bottom would be
#3753 (comment),
but it's not supported well by browsers.

Co-authored-by: Max Philippov <[email protected]>
@WofWca
Copy link
Collaborator Author

WofWca commented Dec 12, 2024 •
edited
Loading

Ok, looks like setting either height attribute or CSS height: to intrinsic size, as I tried in #4406 doesn't work, see #4406 (comment).

I think we need a different approach, perhaps utilizing the aspect-ratio CSS property and turning the container into a flex container, and removing object-fit.

Completely disregarding actual message size and only looking at its aspect ratio is probably not the way to go for tiny images.
Maybe we can set max-height and max-width to the intrinsic size.

We should think in terms of "we need to set the height and width of the image, and not rely on the browser to figure out the dimensions based on the image's intrinsic size.

Also, looking at Signal's implementation could be worth it.

@nicodh nicodh mentioned this issue Dec 12, 2024
Merged
@WofWca WofWca removed their assignment Dec 14, 2024
@Simon-Laux Simon-Laux mentioned this issue Dec 14, 2024
Open
4 tasks
@nicodh
Copy link
Contributor

nicodh commented Dec 16, 2024 •
edited
Loading

Is this still an issue with current main for anyone?
@Jikstra ?

WofWca added a commit that referenced this issue Dec 22, 2024
@WofWca
fix: chat "scrolling up" because of imgs loading
3f8b90b 
This is a follow-up to #4407.
This partially addresses #4404.

The bug was introduced in #3999.
Pior to that we used to have fixed width on quoted images as well.
This commmit practically reverts that MR, but also adds
`object-fit` and `object-position`, and is now in line with
message attachment images.
@WofWca WofWca linked a pull request Dec 22, 2024 that will close this issue
Open
WofWca added a commit that referenced this issue Dec 22, 2024
@WofWca
fix: chat "scrolling up" because of imgs loading
410127a 
This is a follow-up to #4407.
This partially addresses #4404.

The bug was introduced in #3999.
Pior to that we used to have fixed width on quoted images as well.
This commmit practically reverts that MR, but also adds
`object-fit` and `object-position`, and is now in line with
message attachment images.
WofWca added a commit that referenced this issue Dec 22, 2024
@WofWca
fix: chat "scrolling up" because of imgs loading
0b05ac9 
This is a follow-up to #4407.
This partially addresses #4404.

The bug was introduced in #3999.
Pior to that we used to have fixed width on quoted images as well.
This commmit practically reverts that MR, but also adds
`object-fit` and `object-position`, and is now in line with
message attachment images.
WofWca added a commit that referenced this issue Dec 22, 2024
@WofWca
fix: chat "scrolling up" because of imgs loading
ada6bf5 
This, together with #4407,
should close #4404.

The bug was introduced in #3999.
Pior to that we used to have fixed width on quoted images as well.
This commmit practically reverts that MR, but also adds
`object-fit` and `object-position`, and is now in line with
message attachment images.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
2 participants
@nicodh @WofWca