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

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

Closed
WofWca opened this issue Dec 11, 2024 · 4 comments · Fixed by #4431 or #4521
Closed

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

WofWca opened this issue Dec 11, 2024 · 4 comments · Fixed by #4431 or #4521
Assignees
Labels
bug Something isn't working

Comments

@WofWca
Copy link
Collaborator

WofWca commented Dec 11, 2024

  • 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
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 added a commit that referenced this issue Dec 12, 2024
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
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

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
Copy link
Member

nicodh commented Dec 16, 2024

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

WofWca added a commit that referenced this issue Dec 22, 2024
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
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
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
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.
WofWca added a commit that referenced this issue Jan 6, 2025
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.
WofWca added a commit that referenced this issue Jan 6, 2025
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.
@WofWca
Copy link
Collaborator Author

WofWca commented Jan 20, 2025

Bruh. It's still reproducible on 1.51.0, with quoted images. I don't understand how, but somehow upon the initial paint the quote text takes 2 lines, and on the next paint it takes 3 lines:

Image

Here is what it looks like when the image hasn't loaded:
Image

For me it's easier to reproduce when the dev tools are open.

We fixed the width and height, WTF?!

// Remember that width and height need to be set prior to the image
// getting actually loaded by the browser, to avoid content shifts.
// Same goes for messages' `.attachment-content`.
// See https://github.com/deltachat/deltachat-desktop/issues/4404.
//
// Setting just the height is not enough, because there might be text
// displayed beside the image, which might wrap and thus change height.
//
// In addition, really short and wide images could make the message
// overflow the chat container, for non-self-sent messages at least.
height: var(--quote-img-size);
width: var(--quote-img-size);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment