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

Remove hero #689

Merged
merged 1 commit into from
Sep 5, 2023
Merged

Remove hero #689

merged 1 commit into from
Sep 5, 2023

Conversation

micahmo
Copy link
Member

@micahmo micahmo commented Aug 26, 2023

Pull Request Description

This PR removes use of the Hero widget, which is what provides the transition animation with images popping in and out of posts. I recognize that this is more of a personal preference, and I think @hjiangsu should approve/deny this specifically, but I think (especially as we've changed other elements in the app), the Hero animation looks clunky in the scenarios where it remains. Of course, I'd be curious to hear what others think!

  1. When a post is read, the animated image is translucent.
  2. The preview has rounded corners, but the animated image does not.
  3. The animation causes the preview to be reloaded.
qemu-system-x86_64_WKQOphCysF.mp4
  1. The animation occurs even when the image is offscreen, causing it to come from nowhere.
qemu-system-x86_64_P4ydsBgjLC.mp4
  1. The animation occurs when navigating back from a community.
qemu-system-x86_64_iL1sxlaEHU.mp4
  1. Maybe one of the most important reasons (obviously not depicted): NSFW images are not blurred during the animation.

Review without whitespace.

Issue Being Fixed

Issue Number: N/A

Screenshots / Recordings

Checklist

  • Did you update CHANGELOG.md?
  • Did you use localized strings where applicable?
  • Did you add semanticLabels where applicable for accessibility?

@micahmo micahmo requested a review from hjiangsu August 26, 2023 21:49
@ajsosa
Copy link
Collaborator

ajsosa commented Aug 27, 2023

Yeah I find the hero animation quite jarring when it animates from off screen. It's definitely a subjective thing but I definitely vote for it to be removed. But yeah we should wait for some input from @hjiangsu.

@machinaeZER0
Copy link
Collaborator

I'd also vote for removing/changing, especially after seeing all the examples/evidence back to back. Hopefully we'll hear back soon!

Copy link
Member

@hjiangsu hjiangsu left a comment

Choose a reason for hiding this comment

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

Yeah, I'm good with us removing this because of the reasons mentioned above! Everything looks good to me!

@hjiangsu hjiangsu merged commit 60a37c4 into thunder-app:develop Sep 5, 2023
@micahmo micahmo deleted the feature/remove-hero branch September 5, 2023 15:26
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.

4 participants