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: Add a gallery of all the images present on the server (v2) #2801

Merged
merged 52 commits into from
Sep 4, 2022

Conversation

VaiTon
Copy link
Member

@VaiTon VaiTon commented Aug 17, 2022

What

  • Image list gallery v2

Supersedes

2022-09-02.23-57-37.mp4

VaiTon and others added 24 commits August 3, 2022 21:30
# Conflicts:
#	packages/smooth_app/lib/pages/product/product_image_gallery_view.dart
…dart

Co-authored-by: monsieurtanuki <fabrice_fontaine@hotmail.com>
# Conflicts:
#	packages/smooth_app/lib/pages/product/edit_product_page.dart
Co-authored-by: monsieurtanuki <fabrice_fontaine@hotmail.com>
@VaiTon VaiTon self-assigned this Aug 17, 2022
VaiTon and others added 3 commits September 3, 2022 12:49
Co-Authored-By: monsieurtanuki <11576431+monsieurtanuki@users.noreply.github.com>
Co-Authored-By: monsieurtanuki <11576431+monsieurtanuki@users.noreply.github.com>
@VaiTon VaiTon linked an issue Sep 3, 2022 that may be closed by this pull request
@teolemon
Copy link
Member

teolemon commented Sep 4, 2022

@VaiTon in the next PR, we could add image flagging: https://github.com/openfoodfacts/hunger-games/pull/172/files

@VaiTon
Copy link
Member Author

VaiTon commented Sep 4, 2022

@monsieurtanuki I've fixed the file names problem with the url hash. Please tell me your opinion

@monsieurtanuki
Copy link
Contributor

@monsieurtanuki I've fixed the file names problem with the url hash. Please tell me your opinion

@VaiTon That's better, but a hash is not unique. My suggestion is.

@VaiTon
Copy link
Member Author

VaiTon commented Sep 4, 2022

@monsieurtanuki The hash should be unique. If the URL is the same there is no need to check if we're overwriting the image. Also we're not storing anything on the DB that we'd need to clean when we're done

@monsieurtanuki
Copy link
Contributor

Capture d’écran 2022-09-04 à 16 07 32

@VaiTon
Copy link
Member Author

VaiTon commented Sep 4, 2022

@monsieurtanuki I don't really find it something we should care about, as the probability of getting a collision should be really low, decreased by the fact that we should be dealing with different URLs each time.

Despite that, just to make sure that there is no such case, I'm implementing the progressive number check.

Now a question is: when do we need to reset the number to 0 ? Never? Every X days?

@monsieurtanuki
Copy link
Contributor

@VaiTon Good, it's safer this way. And make sure that we can reuse your getNextSequenceNumber method; I'll use it in #2872 (depending on which code is merged first).
The limit as int seems to be 2^53-1, which is a lot, so we are safe here, no need to cycle.
The space taken by the files will be taken care for by the system as we're talking about files in a "temporary folder".

To avoid race conditions for image downloads (see previous commit)
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.

Thank you @VaiTon for those exchanges!
LGTM
Still not convinced by the RRect around the images, but we'll see later, let's merge now!

@VaiTon VaiTon merged commit c706839 into openfoodfacts:develop Sep 4, 2022
@VaiTon VaiTon deleted the feat/image_gallery branch September 4, 2022 17:33
@VaiTon
Copy link
Member Author

VaiTon commented Sep 4, 2022

Thank you @monsieurtanuki for your reviews ! 🚀 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
image carousel https://github.com/openfoodfacts/smooth-app/issues/966
Development

Successfully merging this pull request may close these issues.

Simplify the way to access a product picture
4 participants