-
Notifications
You must be signed in to change notification settings - Fork 241
fix: Reduced the extra white space around save and transfer #1421
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: Reduced the extra white space around save and transfer #1421
Conversation
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.
Sorry @samruddhi-Rahegaonkar, you have reached your weekly rate limit for Sourcery. Please try again later
3633bd8
to
a7fee45
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.
Please test this on different screen sizes and ensure that the buttons do not get moved off-screen.
@nope3472 please review |
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 looks fine on my testing device just confirm on other screen sizes
@mariobehling @hpdang @hpdang This is rendering correctly for all screen sizes. |
@sourcery-ai review |
Reviewer's GuideThis PR streamlines the HomeScreen button area by removing redundant containers and padding, flattening the layout, and adjusting spacing and sizing to eliminate extra whitespace around the Save and Transfer buttons. 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:
- There’s a lot of duplicated styling and onTap logic for the Save/Transfer buttons—consider extracting them into a reusable widget or helper method to make the build method more concise and maintainable.
- Instead of wrapping Containers with GestureDetector, using Flutter’s built-in Button widgets (ElevatedButton/TextButton) with custom ButtonStyle will improve accessibility and give you consistent ripple effects out of the box.
- Relying on a fixed height (height: 350.h) for the TabBarView can cause layout issues on different screen sizes—consider using Flexible/Expanded or letting the content size itself dynamically instead.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There’s a lot of duplicated styling and onTap logic for the Save/Transfer buttons—consider extracting them into a reusable widget or helper method to make the build method more concise and maintainable.
- Instead of wrapping Containers with GestureDetector, using Flutter’s built-in Button widgets (ElevatedButton/TextButton) with custom ButtonStyle will improve accessibility and give you consistent ripple effects out of the box.
- Relying on a fixed height (height: 350.h) for the TabBarView can cause layout issues on different screen sizes—consider using Flexible/Expanded or letting the content size itself dynamically instead.
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Build StatusBuild successful. APKs to test: https://github.com/fossasia/badgemagic-app/actions/runs/17370904873/artifacts/3895962899. 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.
Please address sourcery-ai reviews specifically about screen sizes.
@samruddhi-Rahegaonkar can you get this one ready to merge? |
yes. by today only. |
@hpdang @mariobehling the height scales proportionally on different screens (small devices, tablets, large phones). |
this is ready to merge. |
This feature worked for me. |
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.
Ok, merging this, but we need to continuously test this on more different screens.
…#1421) Co-authored-by: Mario Behling <[email protected]>
Fixes #1419
Changes
Screenshots / Recordings
Checklist:
constants.dart
without hard coding any value.Summary by Sourcery
Remove extra whitespace around Save and Transfer buttons by flattening their layout and adjusting parent container settings.
Bug Fixes:
Enhancements: