-
Notifications
You must be signed in to change notification settings - Fork 240
fix: Added visible Scrollbar on right of animation tab #1426
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
Conversation
Reviewer's GuideThis PR enhances the animation tab by introducing a persistent, thicker, rounded scrollbar, and refines the HomeScreen layout by tightening its column sizing, enlarging the TabBarView height, and streamlining the Save/Transfer button implementations. Class diagram for updated TransitionTab widgetclassDiagram
class TransitionTab {
+State createState()
}
class _TransitionTabState {
-ScrollController _scrollController
+build(context)
}
TransitionTab --> _TransitionTabState
Class diagram for HomeScreen Save/Transfer button logic changesclassDiagram
class _HomeScreenState {
+build(context)
-Save button logic updated
-Transfer button logic updated
}
File-Level Changes
Assessment against linked issues
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- Don’t forget to dispose the ScrollController in TransitionTab’s state to avoid memory leaks.
- There’s a lot of repeated styling and logic around your “Save” and “Transfer” buttons—consider extracting a reusable widget or helper to keep the code DRY.
- You might switch from GestureDetector + Container to Flutter’s built-in Button widgets (TextButton/ElevatedButton) for better theming, accessibility, and built-in feedback.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Don’t forget to dispose the ScrollController in TransitionTab’s state to avoid memory leaks.
- There’s a lot of repeated styling and logic around your “Save” and “Transfer” buttons—consider extracting a reusable widget or helper to keep the code DRY.
- You might switch from GestureDetector + Container to Flutter’s built-in Button widgets (TextButton/ElevatedButton) for better theming, accessibility, and built-in feedback.
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
f2b54c3
to
4e4a8c4
Compare
fix: disposed the scrollcontroller
2a6e502
to
db8c871
Compare
Build StatusBuild successful. APKs to test: https://github.com/fossasia/badgemagic-app/actions/runs/17460351640/artifacts/3925705584. Screenshots |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@samruddhi-Rahegaonkar i think the size of the sidebar is too big
I thought i will be easily visible to the user. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you change this to a thinner scroll bar, please? The main reason to have the scroll bar is to have a visual indication for the user. It is not in the same way important that the user can tap on the scroll bar.
ee4c921
to
700fe57
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected would be that the spacing to the scroll bar is the same as the spacing between the tiles. This should be even meaning on both sides. You might have to adjust the size of the tiles itself to make this work nicely. Please test this on different screen resolutions.
![]() How about this @mariobehling |
d810a76
to
1cddf19
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.

hey @samruddhi-Rahegaonkar i see this change in UI please check for the last 2 animations
yeah i know. Is it good or keep it as it is ? |
The last image in the comment is not displayed.
|
@samruddhi-Rahegaonkar Please share the broken image again and do not forget to address the sourcery-ai comments. Thanks |
Okay! |
Please update this branch. And when there are 12 cards/animations, the app should all in one screen like this PR #1421 |
Fixes #1422
Changes
Screenshots / Recordings
Before:


After:
Checklist:
constants.dart
without hard coding any value.Summary by Sourcery
Add a visible, always-on scrollbar to the animation transition tab, enable vertical scrolling for its contents, adjust the tab panel height and inner column sizing, and streamline the Save/Transfer button widget structure.
Bug Fixes:
Enhancements: