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: #1139 - carousel spinning issues #1604

Merged

Conversation

cli1005
Copy link
Contributor

@cli1005 cli1005 commented Apr 21, 2022

What

Fix of strange spinning/reordering effect when opening a product page

more details : #1139 (comment)

Screenshot

rpreplay-final1650469846_CsmeQg9L.mp4

Fixes bug(s)

@cli1005 cli1005 requested a review from a team as a code owner April 21, 2022 09:47
@cli1005 cli1005 closed this Apr 21, 2022
@cli1005 cli1005 changed the title Bug/1139 carousel spinning issues fix: #1139 - carousel spinning issues Apr 21, 2022
@cli1005 cli1005 reopened this Apr 21, 2022
@monsieurtanuki
Copy link
Contributor

@cli1005 For the record I receive an email every time you decide to close or open a PR. That mean this morning at least 6 emails received just because of you. In that particular case maybe you can create PRs as draft, fine-tune what is needed, and then only when you're done remove the draft mode.

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 @cli1005! I don't know if it works but it looks rather OK.
Please have a look at my comments though.

packages/smooth_app/lib/pages/page_manager.dart Outdated Show resolved Hide resolved
@cli1005
Copy link
Contributor Author

cli1005 commented Apr 21, 2022

@cli1005 For the record I receive an email every time you decide to close or open a PR. That mean this morning at least 6 emails received just because of you. In that particular case maybe you can create PRs as draft, fine-tune what is needed, and then only when you're done remove the draft mode.

👌

@teolemon teolemon linked an issue Apr 22, 2022 that may be closed by this pull request
3 tasks
@teolemon teolemon removed a link to an issue Apr 22, 2022
3 tasks
Copy link
Member

@M123-dev M123-dev left a comment

Choose a reason for hiding this comment

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

Just one small comment, but the rest looks good, thanks @cli1005

@codecov-commenter
Copy link

Codecov Report

Merging #1604 (99e4b79) into develop (2b86e95) will decrease coverage by 0.24%.
The diff coverage is 0.00%.

@@            Coverage Diff             @@
##           develop   #1604      +/-   ##
==========================================
- Coverage     8.86%   8.61%   -0.25%     
==========================================
  Files          161     164       +3     
  Lines         6623    6870     +247     
==========================================
+ Hits           587     592       +5     
- Misses        6036    6278     +242     
Impacted Files Coverage Δ
...oth_app/lib/data_models/continuous_scan_model.dart 0.95% <0.00%> (-0.07%) ⬇️
packages/smooth_app/lib/pages/page_manager.dart 2.00% <0.00%> (-0.13%) ⬇️
...mooth_app/lib/widgets/smooth_product_carousel.dart 0.00% <0.00%> (ø)
packages/smooth_app/lib/main.dart 17.00% <0.00%> (-0.90%) ⬇️
...es/smooth_app/lib/pages/scan/ml_kit_scan_page.dart 0.90% <0.00%> (-0.06%) ⬇️
...th_app/lib/pages/user_management/sign_up_page.dart 0.68% <0.00%> (-0.02%) ⬇️
packages/smooth_app/lib/pages/question_page.dart 0.59% <0.00%> (-0.02%) ⬇️
...ckages/smooth_app/lib/database/local_database.dart 0.00% <0.00%> (ø)
...kages/smooth_app/lib/helpers/analytics_helper.dart 0.00% <0.00%> (ø)
...ges/smooth_app/lib/generic_lib/loading_dialog.dart 0.00% <0.00%> (ø)
... and 25 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2b86e95...99e4b79. Read the comment docs.

@cli1005 cli1005 merged commit e08e813 into openfoodfacts:develop Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

5 participants