-
-
Notifications
You must be signed in to change notification settings - Fork 280
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: 4424 - longer label for "empty the list" + refactoring #4447
Conversation
Impacted files: * `all_products_list_page.dart`: refactored around `ProductListPopupItem` * `app_en.arb`: new label for a more explicit "empty the list" * `app_fr.arb`: new label for a more explicit "empty the list" * `product_list_popup_items.dart`: moved there `ProductListPopupMenuEntry`; added 2 methods to `ProductListPopupItem`; added 1 implementation of `ProductListPopupItem`
Codecov Report
@@ Coverage Diff @@
## develop #4447 +/- ##
========================================
Coverage 10.17% 10.17%
========================================
Files 295 295
Lines 15087 15081 -6
========================================
Hits 1535 1535
+ Misses 13552 13546 -6
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
You can test this PR on: Android |
]; | ||
final List<PopupMenuEntry<ProductListPopupMenuEntry>> |
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 not use the map
operator here instead?
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 tried to find something relevant but couldn't find anything.
I'm not at ease with those map/list methods anyway.
My code can be read by anyone, uses OOP and has no copy/paste, which is good enough for me.
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.
Thanks for the rewording
Thank you @teolemon for the review! |
What
Screenshot
Fixes bug(s)
Impacted files
all_products_list_page.dart
: refactored aroundProductListPopupItem
app_en.arb
: new label for a more explicit "empty the list"app_fr.arb
: new label for a more explicit "empty the list"product_list_popup_items.dart
: moved thereProductListPopupMenuEntry
; added 2 methods toProductListPopupItem
; added 1 implementation ofProductListPopupItem