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: 4424 - longer label for "empty the list" + refactoring #4447

Merged
merged 1 commit into from
Aug 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion packages/smooth_app/lib/l10n/app_en.arb
Original file line number Diff line number Diff line change
Expand Up @@ -774,7 +774,11 @@
"recently_seen_products": "History",
"clear": "Clear",
"@clear": {
"description": "Clears a product list"
"description": "Clears a product list (short label)"
},
"clear_long": "Empty the list",
"@clear_long": {
"description": "Clears a product list (long label)"
},
"really_clear": "Do you really want to delete this list?",
"@Plural": {},
Expand Down
4 changes: 4 additions & 0 deletions packages/smooth_app/lib/l10n/app_fr.arb
Original file line number Diff line number Diff line change
Expand Up @@ -776,6 +776,10 @@
"@clear": {
"description": "Clears a product list"
},
"clear_long": "Vider la liste",
"@clear_long": {
"description": "Clears a product list (long label)"
},
"really_clear": "Voulez-vous vraiment supprimer cette liste ?",
"@Plural": {},
"pct_match": "{percent}% de correspondance",
Expand Down
178 changes: 36 additions & 142 deletions packages/smooth_app/lib/pages/all_product_list_page.dart
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import 'package:smooth_app/generic_lib/design_constants.dart';
import 'package:smooth_app/pages/preferences/user_preferences_list_tile.dart';
import 'package:smooth_app/pages/product/common/product_list_popup_items.dart';
import 'package:smooth_app/pages/product/common/product_query_page_helper.dart';
import 'package:smooth_app/pages/product_list_user_dialog_helper.dart';

/// Page that lists all product lists.
class AllProductListModal extends StatelessWidget {
Expand All @@ -18,7 +17,7 @@ class AllProductListModal extends StatelessWidget {

final ProductList currentList;

final List<ProductList> hardcodedProductLists = <ProductList>[
final List<ProductList> _hardcodedProductLists = <ProductList>[
ProductList.scanSession(),
ProductList.scanHistory(),
ProductList.history(),
Expand All @@ -31,7 +30,7 @@ class AllProductListModal extends StatelessWidget {

final List<String> userLists = daoProductList.getUserLists();
final List<ProductList> productLists =
List<ProductList>.from(hardcodedProductLists);
List<ProductList>.from(_hardcodedProductLists);
for (final String userList in userLists) {
productLists.add(ProductList.user(userList));
}
Expand Down Expand Up @@ -62,39 +61,42 @@ class AllProductListModal extends StatelessWidget {
subtitle: Text(
appLocalizations.user_list_length(productsLength),
),
trailing: PopupMenuButton<PopupMenuEntries>(
trailing: PopupMenuButton<ProductListPopupMenuEntry>(
itemBuilder: (BuildContext context) {
return <PopupMenuEntry<PopupMenuEntries>>[
_shareMenu(
appLocalizations,
daoProductList,
localDatabase,
context,
productList,
),
_openInWebMenu(
appLocalizations,
daoProductList,
localDatabase,
context,
productList,
),
if (productsLength > 0)
_clearListMenu(
appLocalizations,
daoProductList,
localDatabase,
context,
productList,
),
if (productList.isEditable)
_deleteListMenu(
appLocalizations,
daoProductList,
context,
productList,
),
final bool enableRename =
productList.listType == ProductListType.USER;
final List<ProductListPopupItem> list =
<ProductListPopupItem>[
if (enableRename) ProductListPopupRename(),
ProductListPopupShare(),
ProductListPopupOpenInWeb(),
if (productsLength > 0) ProductListPopupClear(),
if (productList.isEditable) ProductListPopupDelete(),
];
final List<PopupMenuEntry<ProductListPopupMenuEntry>>
Copy link
Collaborator

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?

Copy link
Contributor Author

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.

result =
<PopupMenuEntry<ProductListPopupMenuEntry>>[];
for (final ProductListPopupItem item in list) {
result.add(
PopupMenuItem<ProductListPopupMenuEntry>(
value: item.getEntry(),
child: ListTile(
leading: Icon(item.getIconData()),
title: Text(item.getTitle(appLocalizations)),
contentPadding: EdgeInsets.zero,
onTap: () async {
Navigator.of(context).pop();
await item.doSomething(
productList: productList,
localDatabase: localDatabase,
context: context,
);
},
),
),
);
}
return result;
},
icon: const Icon(Icons.more_vert),
),
Expand All @@ -119,112 +121,4 @@ class AllProductListModal extends StatelessWidget {
),
);
}

PopupMenuItem<PopupMenuEntries> _shareMenu(
AppLocalizations appLocalizations,
DaoProductList daoProductList,
LocalDatabase localDatabase,
BuildContext context,
ProductList productList,
) {
final ProductListPopupShare popupShare = ProductListPopupShare();
return PopupMenuItem<PopupMenuEntries>(
value: PopupMenuEntries.shareList,
child: ListTile(
leading: const Icon(Icons.share),
title: Text(popupShare.getTitle(appLocalizations)),
contentPadding: EdgeInsets.zero,
onTap: () {
Navigator.of(context).pop();
popupShare.doSomething(
productList: productList,
localDatabase: localDatabase,
context: context,
);
},
),
);
}

PopupMenuItem<PopupMenuEntries> _openInWebMenu(
AppLocalizations appLocalizations,
DaoProductList daoProductList,
LocalDatabase localDatabase,
BuildContext context,
ProductList productList,
) {
final ProductListPopupOpenInWeb webItem = ProductListPopupOpenInWeb();
return PopupMenuItem<PopupMenuEntries>(
value: PopupMenuEntries.openListInBrowser,
child: ListTile(
leading: const Icon(Icons.public),
title: Text(webItem.getTitle(appLocalizations)),
contentPadding: EdgeInsets.zero,
onTap: () {
Navigator.of(context).pop();
webItem.doSomething(
productList: productList,
localDatabase: localDatabase,
context: context,
);
},
),
);
}

PopupMenuItem<PopupMenuEntries> _clearListMenu(
AppLocalizations appLocalizations,
DaoProductList daoProductList,
LocalDatabase localDatabase,
BuildContext context,
ProductList productList,
) {
final ProductListPopupClear clearItem = ProductListPopupClear();
return PopupMenuItem<PopupMenuEntries>(
value: PopupMenuEntries.clearList,
child: ListTile(
leading: const Icon(Icons.delete_sweep),
title: Text(clearItem.getTitle(appLocalizations)),
contentPadding: EdgeInsets.zero,
onTap: () async {
Navigator.of(context).pop();

clearItem.doSomething(
productList: productList,
localDatabase: localDatabase,
context: context,
);
},
),
);
}

PopupMenuItem<PopupMenuEntries> _deleteListMenu(
AppLocalizations appLocalizations,
DaoProductList daoProductList,
BuildContext context,
ProductList productList,
) {
return PopupMenuItem<PopupMenuEntries>(
value: PopupMenuEntries.deleteList,
child: ListTile(
leading: const Icon(Icons.delete),
title: Text(appLocalizations.action_delete_list),
contentPadding: EdgeInsets.zero,
),
onTap: () {
WidgetsBinding.instance.addPostFrameCallback((_) {
ProductListUserDialogHelper(daoProductList)
.showDeleteUserListDialog(context, productList);
});
});
}
}

enum PopupMenuEntries {
shareList,
openListInBrowser,
renameList,
clearList,
deleteList,
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,25 @@ import 'package:smooth_app/helpers/temp_product_list_share_helper.dart';
import 'package:smooth_app/pages/product_list_user_dialog_helper.dart';
import 'package:url_launcher/url_launcher.dart';

enum ProductListPopupMenuEntry {
share,
openInBrowser,
rename,
clear,
delete,
}

/// Popup menu items for the product list page.
abstract class ProductListPopupItem {
/// Title of the popup menu item.
@protected
String getTitle(final AppLocalizations appLocalizations);

/// IconData of the popup menu item.
IconData getIconData();

/// Popup menu entry of the popup menu item.
ProductListPopupMenuEntry getEntry();

/// Action of the popup menu item.
///
/// Returns a different product list if there are changes, else null.
Expand All @@ -39,7 +52,13 @@ abstract class ProductListPopupItem {
class ProductListPopupClear extends ProductListPopupItem {
@override
String getTitle(final AppLocalizations appLocalizations) =>
appLocalizations.clear;
appLocalizations.clear_long;

@override
IconData getIconData() => Icons.delete_sweep;

@override
ProductListPopupMenuEntry getEntry() => ProductListPopupMenuEntry.clear;

@override
Future<ProductList?> doSomething({
Expand Down Expand Up @@ -83,6 +102,12 @@ class ProductListPopupRename extends ProductListPopupItem {
String getTitle(final AppLocalizations appLocalizations) =>
appLocalizations.user_list_popup_rename;

@override
IconData getIconData() => Icons.edit;

@override
ProductListPopupMenuEntry getEntry() => ProductListPopupMenuEntry.rename;

@override
Future<ProductList?> doSomething({
required final ProductList productList,
Expand All @@ -99,6 +124,12 @@ class ProductListPopupShare extends ProductListPopupItem {
String getTitle(final AppLocalizations appLocalizations) =>
appLocalizations.share;

@override
IconData getIconData() => Icons.share;

@override
ProductListPopupMenuEntry getEntry() => ProductListPopupMenuEntry.share;

@override
Future<ProductList?> doSomething({
required final ProductList productList,
Expand All @@ -125,6 +156,13 @@ class ProductListPopupOpenInWeb extends ProductListPopupItem {
String getTitle(final AppLocalizations appLocalizations) =>
appLocalizations.label_web;

@override
IconData getIconData() => Icons.public;

@override
ProductListPopupMenuEntry getEntry() =>
ProductListPopupMenuEntry.openInBrowser;

@override
Future<ProductList?> doSomething({
required final ProductList productList,
Expand All @@ -137,3 +175,28 @@ class ProductListPopupOpenInWeb extends ProductListPopupItem {
return null;
}
}

/// Popup menu item for the product list page: delete.
class ProductListPopupDelete extends ProductListPopupItem {
@override
String getTitle(final AppLocalizations appLocalizations) =>
appLocalizations.action_delete_list;

@override
IconData getIconData() => Icons.delete;

@override
ProductListPopupMenuEntry getEntry() => ProductListPopupMenuEntry.delete;

@override
Future<ProductList?> doSomething({
required final ProductList productList,
required final LocalDatabase localDatabase,
required final BuildContext context,
}) async {
final bool deleted =
await ProductListUserDialogHelper(DaoProductList(localDatabase))
.showDeleteUserListDialog(context, productList);
return deleted ? null : productList;
}
}