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

feat: #960 - changed the attribute/importance preference display. #1590

Merged
merged 2 commits into from
May 4, 2022

Conversation

monsieurtanuki
Copy link
Contributor

@monsieurtanuki monsieurtanuki commented Apr 17, 2022

Impacted files:

  • attribute_button.dart: changed the whole display.
  • design_constants.dart: added a constant for the default icon size.
  • product_preferences.dart: removed the downgrading of "very important" to "important".
  • pubspec.lock: wtf

What

  • changed the attribute/importance preference display.
  • we still need to find interesting texts for the info dialog of the different importances.

Screenshot

[Edited screenshot after goldens fixes]
Simulator Screen Shot - iPhone 8 Plus - 2022-04-17 at 10 29 17

Part of

… display.

Impacted files:
* `attribute_button.dart`: changed the whole display.
* `design_constants.dart`: added a constant for the default icon size.
* `product_preferences.dart`: added the downgrading of "very important" to "important".
* `pubspec.lock`: wtf
@monsieurtanuki monsieurtanuki requested a review from a team as a code owner April 17, 2022 06:34
@monsieurtanuki monsieurtanuki requested a review from M123-dev April 17, 2022 06:38
Impacted files:
* `attribute_button.dart`: adjustments in order to pass the goldens test.
* `design_constants.dart`: added a constant for the minimum target size (according to goldens).
* `user_preferences_page-blue-dark.png`: impacted by new display.
* `user_preferences_page-blue-light.png`: impacted by new display.
* `user_preferences_page-brown-dark.png`: impacted by new display.
* `user_preferences_page-brown-light.png`: impacted by new display.
* `user_preferences_page-green-dark.png`: impacted by new display.
* `user_preferences_page-green-light.png`: impacted by new display.
@codecov-commenter
Copy link

Codecov Report

Merging #1590 (9f49ab2) into develop (2ea0da3) will increase coverage by 0.13%.
The diff coverage is 37.80%.

@@            Coverage Diff             @@
##           develop   #1590      +/-   ##
==========================================
+ Coverage     8.86%   8.99%   +0.13%     
==========================================
  Files          161     161              
  Lines         6623    6657      +34     
==========================================
+ Hits           587     599      +12     
- Misses        6036    6058      +22     
Impacted Files Coverage Δ
...mooth_app/lib/data_models/product_preferences.dart 27.69% <ø> (-3.74%) ⬇️
...mooth_app/lib/generic_lib/widgets/smooth_card.dart 0.00% <ø> (ø)
...oth_app/lib/pages/onboarding/country_selector.dart 0.00% <0.00%> (ø)
...ges/smooth_app/lib/pages/product/summary_card.dart 0.00% <0.00%> (ø)
...es/smooth_app/lib/pages/scan/ml_kit_scan_page.dart 0.96% <0.00%> (ø)
...kages/smooth_app/lib/widgets/attribute_button.dart 66.10% <60.78%> (-25.90%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update aad0f2a...9f49ab2. Read the comment docs.

@monsieurtanuki
Copy link
Contributor Author

@stephanegigandet ping

@monsieurtanuki
Copy link
Contributor Author

@stephanegigandet ping

@teolemon teolemon added this to the V1 milestone Apr 27, 2022
@monsieurtanuki
Copy link
Contributor Author

@stephanegigandet ping

Copy link
Contributor

@stephanegigandet stephanegigandet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @monsieurtanuki ,

Sorry I missed the notifications.

It looks very good to me, thank you very much!

@monsieurtanuki monsieurtanuki merged commit 21253c1 into openfoodfacts:develop May 4, 2022
@monsieurtanuki
Copy link
Contributor Author

Thank you @stephanegigandet!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants