-
-
Notifications
You must be signed in to change notification settings - Fork 259
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: 3941 - refactoring about up-to-date product data for StatefulWidgets #4262
Conversation
…dgets New file: * `up_to_date_manager.dart`: Provides the most up-to-date local product data for a StatefulWidget. Impacted files: * `add_new_product_page.dart`: now using new class `UpToDateManager` * `edit_new_packagings.dart`: now using new class `UpToDateManager` * `edit_ocr_page.dart`: now using new class `UpToDateManager` * `edit_product_page.dart`: now using new class `UpToDateManager` * `knowledge_panel_page.dart`: now using new class `UpToDateManager` * `new_product_page.dart`: now using new class `UpToDateManager` * `nutrition_page_loaded.dart`: now using new class `UpToDateManager` * `product_image_gallery.dart`: now using new class `UpToDateManager` * `product_image_swipeable.dart`: now using new class `UpToDateManager` * `product_image_viewer.dart`: now using new class `UpToDateManager` * `summary_card.dart`: now using new class `UpToDateManager`
Codecov Report
@@ Coverage Diff @@
## develop #4262 +/- ##
===========================================
+ Coverage 10.79% 10.82% +0.02%
===========================================
Files 287 288 +1
Lines 14288 14251 -37
===========================================
Hits 1542 1542
+ Misses 12746 12709 -37
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
/// * we get the most up-to-date local product data | ||
/// * we re-launch the task manager if relevant | ||
/// * we track the barcodes currently "opened" by the app | ||
class UpToDateManager { |
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.
Why don't use a mixin
here?
As it's specifically design for a StatefullWidget
, it would simplify the integration
Otherwise, looks OK 👍
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.
@g123k Thank you for the suggestion! I edited the code in the latest push: my first experience with mixin
.
Not 100% convinced in that particular case, specifically because we somehow need a constructor - replaced by an "init" method as mixin
doesn't accept constructors.
In the end, it looks a bit better than before.
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 clearly prefer this implementation, even if indeed the number of lines is similar.
(I am also very sorry, I receive random notifications from GitHub, and I totally missed your answer)
What
Fixes bug(s)
Files
New file:
up_to_date_manager.dart
: Provides the most up-to-date local product data for a StatefulWidget.Impacted files:
add_new_product_page.dart
: now using new classUpToDateManager
edit_new_packagings.dart
: now using new classUpToDateManager
edit_ocr_page.dart
: now using new classUpToDateManager
edit_product_page.dart
: now using new classUpToDateManager
knowledge_panel_page.dart
: now using new classUpToDateManager
new_product_page.dart
: now using new classUpToDateManager
nutrition_page_loaded.dart
: now using new classUpToDateManager
product_image_gallery.dart
: now using new classUpToDateManager
product_image_swipeable.dart
: now using new classUpToDateManager
product_image_viewer.dart
: now using new classUpToDateManager
summary_card.dart
: now using new classUpToDateManager