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: #3065 - using Robotoff question imageUrl if available #3178

Merged
merged 1 commit into from
Oct 19, 2022

Conversation

monsieurtanuki
Copy link
Contributor

Impacted files:

  • Podfile.lock (app): wtf
  • Podfile.lock (smooth_app): wtf
  • product_image_carousel.dart: added optional imageUrl that displays it instead of the standard all pictures
  • question_card.dart: added Robotoff question imageUrl if available

What

  • In Robotoff questions, there is an optional imageUrl field that we didn't use at all in Smoothie.
  • When the field is populated, it is the best field that could help the user answer the question, so it should be displayed with the higher priority (instead of being ignored).
  • This PR displays the standard carousel of images when the question imageUrl field is not populated (same as before), or only the question imageUrl field if populated. If needed by the user, all pictures are still available if you click on the single picture.

Open questions:

  • Should we display the question imageUrl AND the rest of the images - can be problematic when the imageUrl is the front image, like stuttering (would need deduplicating)
  • Should we display a bigger image instead of the carousel?
  • Are the question imageUrl always populated?

Screenshot

before after
Capture d’écran 2022-10-19 à 13 25 25 Capture d’écran 2022-10-19 à 13 24 46
Capture d’écran 2022-10-19 à 13 21 45 Capture d’écran 2022-10-19 à 13 22 38

Fixes bug(s)

Impacted files:
* `Podfile.lock` (`app`): wtf
* `Podfile.lock` (`smooth_app`): wtf
* `product_image_carousel.dart`: added optional imageUrl that displays it instead of the standard all pictures
* `question_card.dart`: added Robotoff question imageUrl if available
@codecov-commenter
Copy link

Codecov Report

Merging #3178 (c267beb) into develop (3b2a12f) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           develop   #3178      +/-   ##
==========================================
- Coverage     6.18%   6.17%   -0.01%     
==========================================
  Files          252     252              
  Lines        12491   12496       +5     
==========================================
  Hits           772     772              
- Misses       11719   11724       +5     
Impacted Files Coverage Δ
...ib/cards/product_cards/product_image_carousel.dart 0.00% <0.00%> (ø)
...ooth_app/lib/pages/hunger_games/question_card.dart 0.00% <0.00%> (ø)

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

@monsieurtanuki
Copy link
Contributor Author

Thank you @M123-dev for your review!
Not 100% convinced by the visual aspect though... Well, that would be a new issue.

@monsieurtanuki monsieurtanuki merged commit 086ff45 into openfoodfacts:develop Oct 19, 2022
@M123-dev M123-dev added this to the v4 milestone Nov 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

On Robotoff question, display the image that triggered insight creation
4 participants