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

Fix an issue with comment scrolling offset #1639

Merged
merged 1 commit into from
Dec 28, 2024

Conversation

micahmo
Copy link
Member

@micahmo micahmo commented Dec 13, 2024

Pull Request Description

This is a quick fix related to comment scrolling in the new post page. It looks like my change in #1630 inadvertently allowed the comments to be scrolled to the very top of the display, due to the extended SafeArea, which is now partly covered by a Container. This fix adds an offset to animateToItem to account for the status bar.

However, this turned out to be tricky, because as a child widget of SafeArea(top: false), accessing the status bar height with MediaQuery.of(context).padding.top always returns 0. Therefore I had to capture the height outside the SafeArea and pass it in. I also had to remove the offset when the top bar is pinned. Now it works well!

Issue Being Fixed

Issue Number: N/A

Screenshots / Recordings

Before

comment_scroll_before.mp4

After

comment_scroll_after.mp4

Checklist

  • If a new package was added, did you ensure it uses an appropriate license and is actively maintained?
  • Did you use localized strings (and added appropriate descriptions) where applicable?
  • Did you add semanticLabels where applicable for accessibility?

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.

LGTM!

@hjiangsu hjiangsu merged commit f90f711 into thunder-app:develop Dec 28, 2024
1 check passed
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.

2 participants