-
-
Notifications
You must be signed in to change notification settings - Fork 302
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: Language selector + ingredients (text field + outdated) #6170
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #6170 +/- ##
==========================================
- Coverage 9.54% 6.33% -3.22%
==========================================
Files 325 451 +126
Lines 16411 25984 +9573
==========================================
+ Hits 1567 1645 +78
- Misses 14844 24339 +9495 ☔ View full report in Codecov by Sentry. |
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.
I would have possibly added the image icon (instead of the camera which implies an action)
The penultimate one is interesting, and we could badge it with a clock if obsolete |
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.
Hi @g123k!
LGTM
builder: (BuildContext context) { | ||
return SmoothModalSheet( | ||
title: appLocalizations.product_image_outdated_explanations_title, | ||
prefixIndicator: true, |
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.
I would somehow put the outdated icon here.
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.
For now, the header of a modal sheet can't have an icon.
But now that we have modal sheets for Help, I think I will add a better layout for the content (including that kind of image)
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.
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.
In the "new" design, modal sheets only have this indicator.
Hence, this is a boolean.
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.
That's how the "new" design may be improved, using as an indicator: a dot, nothing, or a specified icon.
Hi!
This PR brings 3 minor changes:
Ingredients/packaging
Photo Gallery