-
-
Notifications
You must be signed in to change notification settings - Fork 287
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: Non-aligned button texts and icons on "send anonymous analytics" page #1443
fix: Non-aligned button texts and icons on "send anonymous analytics" page #1443
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #1443 +/- ##
=========================================
Coverage ? 9.15%
=========================================
Files ? 158
Lines ? 6356
Branches ? 0
=========================================
Hits ? 582
Misses ? 5774
Partials ? 0 Continue to review full report at Codecov.
|
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.
Thank you @bhattabhi013 for your PR!
When I said "we should use methods", I just meant that the 2 similar buttons should use the same method, I didn't mean that every Widget
should be turned into a method (1/ that's not a good practice - if really you need to move away the code rather create Widget
class instead and 2/ that's more work for code reviewers).
That said, the method for both buttons is exactly what I had in mind.
The problem is that the result is not exactly what I expected (actually I was not very explicit about it).
I expected something like that:
v Authorize
x Refuse
with v
and x
icons on the same pixel column and A
and R
starting on the same pixel column too. A bit like radio buttons or check boxes.
In your PR they are centered, just like they were before, but the icon is on the left and not on the right anymore...
Thanks for the review @monsieurtanuki, as per the suggestion I will remove extra widgets and revert them back to their original form. I'll also remove extra spaces in |
@bhattabhi013 Let's keep it simple: a |
packages/smooth_app/lib/pages/onboarding/consent_analytics_page.dart
Outdated
Show resolved
Hide resolved
Hi @monsieurtanuki, |
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.
Thank you @bhattabhi013: now the code and the screenshot look very close to what I had in mind!
… page (openfoodfacts#1443) * fix: analytics screen * added extra spaces and elevation * resolved conflicts
* Switched unslecteditemcolor according to theme * fix: Non-aligned button texts and icons on "send anonymous analytics" page (#1443) * fix: analytics screen * added extra spaces and elevation * resolved conflicts * Merge branch 'develop' of https://github.com/abhay1821/smooth-app-1 into develop * removed fitted box
What
Fixes non-aligned button texts and icons on the "send anonymous analytics" page and improve code readability.
Screenshot
Fixes bug(s)
Part of
(please be as granular as possible)