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

Hero animation for images should run only to/from lightbox, not between message lists #930

Open
gnprice opened this issue Sep 5, 2024 · 5 comments · May be fixed by #1348
Open

Hero animation for images should run only to/from lightbox, not between message lists #930

gnprice opened this issue Sep 5, 2024 · 5 comments · May be fixed by #1348
Assignees
Labels
a-lightbox The lightbox / image-viewer screen a-msglist The message-list screen, except what's label:a-content help wanted
Milestone

Comments

@gnprice
Copy link
Member

gnprice commented Sep 5, 2024

When a message has an image and you tap the image to see it in the lightbox, we use a Hero widget to cause the image to animate from its old position and size in the message list to its new position and size in the lightbox, and to do the same in reverse when you navigate back.

Currently, though, our implementation of this causes those "hero" animations to happen also when you navigate from one message list to another that happens to contain the same image-bearing message. That doesn't make so much sense, and tends to look glitchy. It can look extra glitchy when the message is off screen either before or after the navigation (or even both), as spotted by @alexmv:
https://chat.zulip.org/#narrow/stream/48-mobile/topic/Images.20have.20the.20wrong.20z-index/near/1932012

So we should fix that.

I think the way to implement this will be to augment the tag parameter we pass to Hero so that it identifies not only the message and the image within the message, but the message list the message is shown in.

@gnprice gnprice added a-msglist The message-list screen, except what's label:a-content a-lightbox The lightbox / image-viewer screen labels Sep 5, 2024
@gnprice gnprice added this to the Post-launch milestone Sep 5, 2024
@shumail05
Copy link

i want to work on this issue , please assign this to me.

@Adityakk9031
Copy link

can try to resolve this issue

Adityakk9031 added a commit to Adityakk9031/zulip-flutter that referenced this issue Dec 5, 2024
resolve the hero animation
@chrisbobbe
Copy link
Collaborator

sher999 added a commit to sher999/zulip-flutter that referenced this issue Dec 11, 2024
sher999 added a commit to sher999/zulip-flutter that referenced this issue Dec 13, 2024
… the LightBoxHeroTag

and adding narrow on lightbox_test and content_test
sher999 added a commit to sher999/zulip-flutter that referenced this issue Dec 13, 2024
sher999 added a commit to sher999/zulip-flutter that referenced this issue Dec 26, 2024
lakshya1goel added a commit to lakshya1goel/zulip-flutter that referenced this issue Feb 12, 2025
@lakshya1goel
Copy link
Contributor

Please assign this issue to me, I have started working on this. Created a draft PR as well need to add tests.

Implementation Video

Before

WhatsApp.Video.2025-02-12.at.5.18.39.PM.mp4

After

WhatsApp.Video.2025-02-12.at.5.18.35.PM.mp4

lakshya1goel added a commit to lakshya1goel/zulip-flutter that referenced this issue Feb 15, 2025
@lakshya1goel
Copy link
Contributor

PR #1348 is ready for review. PTAL, Thanks!

lakshya1goel added a commit to lakshya1goel/zulip-flutter that referenced this issue Feb 18, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-lightbox The lightbox / image-viewer screen a-msglist The message-list screen, except what's label:a-content help wanted
Projects
Status: No status
Development

Successfully merging a pull request may close this issue.

5 participants