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: 4674 - new UI for main image page; fallback image only for product icon #4738

Merged
merged 2 commits into from
Oct 28, 2023

Conversation

monsieurtanuki
Copy link
Contributor

What

  • This is the first PR for New feature: create a screen to view all photos from a product #4674.
  • Other PRs will follow, that would include adding somehow the "OTHER" pictures.
  • The page where the UI changed is accessible from "edit product" page / "pictures".
  • Here we do not use fallback pictures anymore - only the picture that matches the specified language, in order to avoid confusion.
  • There's one single exception to that new "no fallback" rule: the product thumbnail.

Screenshot

fr it en
Screenshot_1698390683 Screenshot_1698390668 Screenshot_1698390698

Impact on soon-to-be-removed carousel:

fr it
Screenshot_1698394192 Screenshot_1698394162

Part of

Impacted files:

  • add_new_product_helper.dart: minor refactoring
  • add_new_product_page.dart: minor refactoring
  • image_field_extension.dart: minor refactoring
  • product_cards_helper.dart: removed references to fallback image - now we stick to the specified language; minor refactoring
  • product_image_carousel.dart: minor refactoring
  • product_image_gallery_view.dart: added a language selector; new UI
  • product_image_swipeable_view.dart: added an optional language parameter; simplified the code now relying only on ImageField
  • product_image_viewer.dart: minor refactoring
  • smooth_list_tile_card.dart: minor refactoring
  • smooth_product_image.dart: now we explicitly use the default image if needed

…uct icon

Impacted files:
* `add_new_product_helper.dart`: minor refactoring
* `add_new_product_page.dart`: minor refactoring
* `image_field_extension.dart`: minor refactoring
* `product_cards_helper.dart`: removed references to fallback image - now we stick to the specified language; minor refactoring
* `product_image_carousel.dart`: minor refactoring
* `product_image_gallery_view.dart`: added a language selector; new UI
* `product_image_swipeable_view.dart`: added an optional language parameter; simplified the code now relying only on `ImageField`
* `product_image_viewer.dart`: minor refactoring
* `smooth_list_tile_card.dart`: minor refactoring
* `smooth_product_image.dart`: now we explicitly use the default image if needed
@monsieurtanuki monsieurtanuki requested a review from a team as a code owner October 27, 2023 08:24
@github-actions github-actions bot added 🥫 Product page Product addition The easier it is to add a product and get Nutri-Score, Eco-Score, the happier the users. labels Oct 27, 2023
@codecov-commenter
Copy link

Codecov Report

Merging #4738 (b4314bb) into develop (2d90250) will decrease coverage by 0.04%.
Report is 12 commits behind head on develop.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           develop   #4738      +/-   ##
==========================================
- Coverage     9.90%   9.87%   -0.04%     
==========================================
  Files          310     310              
  Lines        15807   15846      +39     
==========================================
- Hits          1566    1565       -1     
- Misses       14241   14281      +40     
Files Coverage Δ
...lib/generic_lib/widgets/smooth_list_tile_card.dart 0.00% <ø> (ø)
.../smooth_app/lib/helpers/image_field_extension.dart 0.00% <ø> (ø)
..._app/lib/pages/product/add_new_product_helper.dart 0.00% <ø> (ø)
...th_app/lib/pages/product/add_new_product_page.dart 0.00% <ø> (ø)
...th_app/lib/pages/product/product_image_viewer.dart 0.00% <ø> (ø)
...ib/cards/product_cards/product_image_carousel.dart 0.00% <0.00%> (ø)
.../lib/generic_lib/widgets/smooth_product_image.dart 0.00% <0.00%> (ø)
...ib/pages/product/product_image_swipeable_view.dart 0.00% <0.00%> (ø)
...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%> (ø)

... and 4 files with indirect coverage changes

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

@g123k
Copy link
Collaborator

g123k commented Oct 27, 2023

Thanks for your PR @monsieurtanuki :)

Just so I know where your PR stops, in terms of UI:
-> Will you do the accessibility thing?
-> … change the placeholder?
-> Add the title for the section?

@monsieurtanuki
Copy link
Contributor Author

Thanks for your PR @monsieurtanuki :)

Just so I know where your PR stops, in terms of UI: -> Will you do the accessibility thing? -> … change the placeholder? -> Add the title for the section?

@g123k The PR is "as is".
Let's work in an interative way: after it's merged let's fine-tune the display. There are already significant model changes in this PR. We can focus on UI details in another PR.

Copy link
Collaborator

@g123k g123k left a comment

Choose a reason for hiding this comment

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

Everything looks good to me.
I think that for "Other photos", you will be forced to use a GridView (since you have a variable number of products). And I wonder if it will not be simpler to directly use it for this part

@monsieurtanuki monsieurtanuki merged commit efb0483 into openfoodfacts:develop Oct 28, 2023
6 checks passed
@monsieurtanuki
Copy link
Contributor Author

Everything looks good to me. I think that for "Other photos", you will be forced to use a GridView (since you have a variable number of products). And I wonder if it will not be simpler to directly use it for this part

Thank you @g123k for your review!
We don't need a GridView, a horizontal ListView.builder is good enough (cf. the carousel).
That said, there are still some details to clarify. But let's comment that on #4674 instead of that merged PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Product addition The easier it is to add a product and get Nutri-Score, Eco-Score, the happier the users. 🥫 Product page
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants