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

fix: miniature of ingredients blocks the text #2964

Merged
merged 6 commits into from
Sep 9, 2022

Conversation

M123-dev
Copy link
Member

@M123-dev M123-dev commented Sep 7, 2022

What

@M123-dev M123-dev requested a review from a team as a code owner September 7, 2022 16:56
@github-actions github-actions bot added Product addition The easier it is to add a product and get Nutri-Score, Eco-Score, the happier the users. 🥫 Product page labels Sep 7, 2022
@github-actions github-actions bot added 🤳 MLKit 🤳🥫 Scan We need to be able to scan on low-end, old devices, even with a bad camera, connexion… labels Sep 7, 2022
@codecov-commenter
Copy link

Codecov Report

Merging #2964 (9ec0ca4) into develop (283cda6) will decrease coverage by 0.31%.
The diff coverage is 0.45%.

@@            Coverage Diff             @@
##           develop   #2964      +/-   ##
==========================================
- Coverage     6.94%   6.63%   -0.32%     
==========================================
  Files          229     236       +7     
  Lines        11155   11731     +576     
==========================================
+ Hits           775     778       +3     
- Misses       10380   10953     +573     
Impacted Files Coverage Δ
...s/smooth_app/lib/data_models/user_preferences.dart 9.37% <0.00%> (-0.52%) ⬇️
...e_panel/knowledge_panels/knowledge_panel_card.dart 0.00% <0.00%> (ø)
...knowledge_panels/knowledge_panel_element_card.dart 0.00% <0.00%> (ø)
...nowledge_panels/knowledge_panel_expanded_card.dart 0.00% <0.00%> (ø)
...l/knowledge_panels/knowledge_panel_group_card.dart 0.00% <ø> (ø)
...e_panel/knowledge_panels/knowledge_panel_page.dart 0.00% <0.00%> (ø)
.../lib/knowledge_panel/knowledge_panels_builder.dart 0.00% <0.00%> (ø)
packages/smooth_app/lib/pages/crop_helper.dart 0.00% <0.00%> (ø)
packages/smooth_app/lib/pages/image_crop_page.dart 2.12% <0.00%> (+0.20%) ⬆️
...ages/onboarding/knowledge_panel_page_template.dart 0.00% <0.00%> (ø)
... and 18 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Contributor

@monsieurtanuki monsieurtanuki left a comment

Choose a reason for hiding this comment

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

Hi @M123-dev!
Not very convinced: of course it probably solves the issue (a screenshot would be nice), but the Image is therefore probably a bit little.

Possible improvement: use the RRect like in the product items in lists.
Not sure if it's much bigger, but at least it's vaguely consistent with the rest of the app.

Comment on lines 192 to 196
SizedBox(
height: MINIMUM_TOUCH_SIZE,
width: MINIMUM_TOUCH_SIZE,
child: Image.file(image, fit: BoxFit.cover),
),
Copy link
Contributor

Choose a reason for hiding this comment

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

That's not that big. Probably ridiculous for images in height.

@M123-dev
Copy link
Member Author

M123-dev commented Sep 9, 2022

Thanks for the review @monsieurtanuki, I added the RRect and changed the size back to 50 as this will be changed anyways by #2843

@M123-dev M123-dev merged commit c30e109 into develop Sep 9, 2022
@M123-dev M123-dev deleted the fix-miniature-of-ingredients-blocks-the-text branch September 9, 2022 23:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤳 MLKit Product addition The easier it is to add a product and get Nutri-Score, Eco-Score, the happier the users. 🥫 Product page 🤳🥫 Scan We need to be able to scan on low-end, old devices, even with a bad camera, connexion…
Development

Successfully merging this pull request may close these issues.

Product Addition: miniature of ingredients blocks the text
4 participants