-
-
Notifications
You must be signed in to change notification settings - Fork 368
feat: Add infinite scrolling to the various Open Prices ListViews. #6561
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
base: develop
Are you sure you want to change the base?
feat: Add infinite scrolling to the various Open Prices ListViews. #6561
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #6561 +/- ##
==========================================
- Coverage 9.54% 5.79% -3.76%
==========================================
Files 325 503 +178
Lines 16411 30130 +13719
==========================================
+ Hits 1567 1745 +178
- Misses 14844 28385 +13541 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have no strong opinion against your code, that seems to work. Possibly minor comments here and there.
The thing is that for the Prices (in a broad sense) we need different objects with a "load more" feature: prices (with this PR), proofs, contributors, locations and products.
Do you feel confident enough in your OOP skills? If so you should refactor your current code with generic concepts like:
- a loading class
- with a load method that has the page number as a parameter
- returning a list of items, a number of items, a number of pages
- an item display class that takes an item as a parameter and displays it
- a "load by page" class that displays items by page, with a "loader" and an "item display" in the constructor
I'm sure there are Design Patterns around those common features.
What do you think about that?
…ntribution using generic class.
@monsieurtanuki, I tried to implement what you suggested—looking forward to your feedback! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @AshutoshKhadse23!
Good job!
Please have a look at my comments, though, for code simplification.
packages/smooth_app/lib/pages/prices/generic_infinite_scroll.dart
Outdated
Show resolved
Hide resolved
packages/smooth_app/lib/pages/prices/generic_infinite_scroll.dart
Outdated
Show resolved
Hide resolved
packages/smooth_app/lib/pages/prices/generic_infinite_scroll.dart
Outdated
Show resolved
Hide resolved
packages/smooth_app/lib/pages/prices/prices_locations_page.dart
Outdated
Show resolved
Hide resolved
packages/smooth_app/lib/pages/prices/generic_infinite_scroll.dart
Outdated
Show resolved
Hide resolved
packages/smooth_app/lib/pages/prices/generic_infinite_scroll.dart
Outdated
Show resolved
Hide resolved
packages/smooth_app/lib/pages/prices/generic_infinite_scroll.dart
Outdated
Show resolved
Hide resolved
packages/smooth_app/lib/pages/prices/generic_infinite_scroll.dart
Outdated
Show resolved
Hide resolved
packages/smooth_app/lib/pages/prices/generic_infinite_scroll.dart
Outdated
Show resolved
Hide resolved
Hello @monsieurtanuki , |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @AshutoshKhadse23!
Obviously something wrong in your latest push, as we have twice the same class.
The idea was to get rid of parameters that are ALWAYS the same, and that's not was you pushed.
packages/smooth_app/lib/pages/prices/infinite_scroll_controller.dart
Outdated
Show resolved
Hide resolved
@monsieurtanuki, changed the code according to the comment. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @AshutoshKhadse23!
Please have a look at my comments.
packages/smooth_app/lib/pages/prices/product_price_refresher.dart
Outdated
Show resolved
Hide resolved
packages/smooth_app/lib/pages/prices/infinite_scroll_controller.dart
Outdated
Show resolved
Hide resolved
… add/infinite-scroll-prices
@monsieurtanuki, changed the code according to the comment. |
Closes: #6478
I added infinite scrolling to open the prices list view.
This is how the solution looks :
issue.6478.new.mp4