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: loop loading when scanning #2669 #2713

Merged

Conversation

AshAman999
Copy link
Member

@AshAman999 AshAman999 commented Aug 2, 2022

What

  • Did the research, the bug arrived with this pr feat: #2396 - better memory management - load products only when needed #2609
  • IDK why the constructor ScanProductCardLoader is always called even when there is no other product in scan range, tried looking into past codes, the code was everywhere the same, so doing this small change fixed the problem
  • The loading indicator is already there, so I guess no need to do extra work here, and this fixed the visual issue now

Screenshot

scanloop.mp4

Fixes bug(s)

@AshAman999 AshAman999 requested a review from a team as a code owner August 2, 2022 13:09
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 @AshAman999!
Sorry about that: as I cannot test the scan part there may be side-effects.
That said, we should probably unify the management of getting/displaying a single product. One day.
So far so good!

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.

LGTM

@AshAman999
Copy link
Member Author

@M123-dev ping
Merge this before we get merge conflicts 🙃

@g123k g123k merged commit 6b5747a into openfoodfacts:develop Aug 4, 2022
@M123-dev
Copy link
Member

M123-dev commented Aug 5, 2022

Feel free to also merge your PR's if they are approved @AshAman999 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Scanning with the offline more (or online mode on an offline product) creates a visual loop
4 participants