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: #3238 - removed "other" pictures in gallery (keep just the main 4) #3241

Merged
merged 2 commits into from
Nov 3, 2022

Conversation

monsieurtanuki
Copy link
Contributor

Impacted files:

  • product_cards_helper.dart: added an optional parameter in order to ignore "other" pictures
  • product_image_gallery_view.dart: removed the "other" pictures

What

  • There was an endless refresh of this page, due to "other" pictures being loaded on the side.
  • Actually those "other" pictures are not very relevant in the app, because they have limited features.
  • I suggested to ignore the "other" pictures: that simplifies the code, the UX/UI, and fixes the bug.
  • No problem to put the "other" pictures back later, probably in another page, definitely in another issue/PR

Screenshot

Capture d’écran 2022-10-31 à 20 06 42

Fixes bug(s)

…ust the main 4)

Impacted files:
* `product_cards_helper.dart`: added an optional parameter in order to ignore "other" pictures
* `product_image_gallery_view.dart`: removed the "other" pictures
@codecov-commenter
Copy link

Codecov Report

Merging #3241 (2d6d82a) into develop (14cfed4) will increase coverage by 0.04%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           develop   #3241      +/-   ##
==========================================
+ Coverage     7.66%   7.71%   +0.04%     
==========================================
  Files          249     248       -1     
  Lines        12286   12211      -75     
==========================================
  Hits           942     942              
+ Misses       11344   11269      -75     
Impacted Files Coverage Δ
...s/smooth_app/lib/helpers/product_cards_helper.dart 0.00% <0.00%> (ø)
.../lib/pages/product/product_image_gallery_view.dart 0.00% <0.00%> (ø)
..._lib/widgets/images/smooth_images_sliver_grid.dart

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

@teolemon
Copy link
Member

The image manager should definitely be back at some point, possibly elsewhere.
The list of other images is useful for:

  • moderation (spot bad images)
  • selecting images from server as front/ingredient/nutrition/packaging

Copy link
Member

@AshAman999 AshAman999 left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@monsieurtanuki
Copy link
Contributor Author

Thank you @AshAman999 for your review!
I've created #3245 about the "other" images that I need to dismiss here temporarily because they don't really belong in this page (not the same purpose) and because it's an additional exotic problem to solve regarding the background task/refresh/upload.

@monsieurtanuki monsieurtanuki merged commit f65c169 into openfoodfacts:develop Nov 3, 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.

Endless rebuild in product_image_gallery_view.dart
5 participants