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: Some improvements for the carousel (changing pages, a11n…) #4225

Merged
merged 3 commits into from
Jun 25, 2023

Conversation

g123k
Copy link
Collaborator

@g123k g123k commented Jun 24, 2023

Hi everyone,

The carousel works when now, but sometimes it moves in all directions when barcodes are detected.
Here are the improvements implements:

  • If the selected card is also the barcode scanned => do nothing
  • Wait for an animation to be finished, before starting a new one
  • There were some specific cases, where the control wouldn't be called to change the page
  • The vibration + audio part is now moved to the Scan page, to notify according to the carousel events and not the scanner
  • There is also an improvement in terms of a11n: when a barcode is decoded, a voice will tell it

I'm not 100% sure that it will prevent the weird behavior that when a barcode is scanned, sometimes it goes back to the first card. But after my changes, I can't reproduce it anymore, so let's 🤞.

As always, a video is better than words: https://github.com/openfoodfacts/smooth-app/assets/246838/c23c40c2-6f40-4327-81b8-4587050ca80b

@g123k g123k requested a review from a team as a code owner June 24, 2023 10:27
@g123k g123k self-assigned this Jun 24, 2023
@github-actions github-actions bot added the 🤳🥫 Scan We need to be able to scan on low-end, old devices, even with a bad camera, connexion… label Jun 24, 2023
@codecov-commenter
Copy link

codecov-commenter commented Jun 24, 2023

Codecov Report

Merging #4225 (5af9906) into develop (e4a02f8) will decrease coverage by 0.03%.
The diff coverage is 0.00%.

@@             Coverage Diff             @@
##           develop    #4225      +/-   ##
===========================================
- Coverage    10.93%   10.91%   -0.03%     
===========================================
  Files          282      282              
  Lines        14096    14119      +23     
===========================================
- Hits          1542     1541       -1     
- Misses       12554    12578      +24     
Impacted Files Coverage Δ
...oth_app/lib/data_models/continuous_scan_model.dart 0.00% <ø> (ø)
...es/smooth_app/lib/pages/scan/camera_scan_page.dart 2.12% <ø> (+0.61%) ⬆️
packages/smooth_app/lib/pages/scan/scan_page.dart 2.10% <0.00%> (-0.93%) ⬇️
...mooth_app/lib/widgets/smooth_product_carousel.dart 1.70% <0.00%> (-0.75%) ⬇️

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

@teolemon
Copy link
Member

If a vibration part is moved to the actual movement on the carousel, how do we know a barcode was scanned ?

@g123k
Copy link
Collaborator Author

g123k commented Jun 24, 2023

We know when a new barcode is scanned when the controller moves to a new page, and not by a swipe gesture.
The callback is called in this specific case

packages/smooth_app/lib/l10n/app_en.arb Outdated Show resolved Hide resolved
@teolemon
Copy link
Member

Merge conflict @g123k

@g123k g123k force-pushed the carousel_improvements branch from b113d50 to 5af9906 Compare June 25, 2023 13:48
@teolemon teolemon merged commit 097a905 into openfoodfacts:develop Jun 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🌐 l10n product scan carousel 🤳🥫 Scan We need to be able to scan on low-end, old devices, even with a bad camera, connexion…
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants