refactor(mobile): log asyncvalue errors (immich-app#5327)

* refactor: scaffoldwhen to log errors during scaffold body render * refactor: onError and onLoading scaffoldbody * refactor: more scaffold body to custom extension * refactor: add skiploadingonrefresh * Snackbar color --------- Co-authored-by: shalong-tanwen <[email protected]> Co-authored-by: Alex Tran <[email protected]>
MiguelAraCo · Nov 29, 2023 · 513f252 · 513f252
1 parent 0fe704c
commit 513f252
Show file tree

Hide file tree

Showing 26 changed files with 203 additions and 215 deletions.
diff --git a/mobile/lib/extensions/asyncvalue_extensions.dart b/mobile/lib/extensions/asyncvalue_extensions.dart
@@ -4,22 +4,32 @@ import 'package:immich_mobile/shared/ui/immich_loading_indicator.dart';
 import 'package:immich_mobile/shared/ui/scaffold_error_body.dart';
 import 'package:logging/logging.dart';
 
-extension ScaffoldBody<T> on AsyncValue<T> {
-  static final Logger _scaffoldBodyLog = Logger("ScaffoldBody");
+extension LogOnError<T> on AsyncValue<T> {
+  static final Logger _asyncErrorLogger = Logger("AsyncValue");
 
-  Widget scaffoldBodyWhen({
+  Widget widgetWhen({
+    bool skipLoadingOnRefresh = true,
+    Widget Function()? onLoading,
+    Widget Function(Object? error, StackTrace? stack)? onError,
     required Widget Function(T data) onData,
-    Widget? onError,
   }) {
     if (isLoading) {
-      return const Center(
-        child: ImmichLoadingIndicator(),
-      );
+      bool skip = false;
+      if (isRefreshing) {
+        skip = skipLoadingOnRefresh;
+      }
+
+      if (!skip) {
+        return onLoading?.call() ??
+            const Center(
+              child: ImmichLoadingIndicator(),
+            );
+      }
     }
 
     if (hasError && !hasValue) {
-      _scaffoldBodyLog.severe("Error occured in AsyncValue", error, stackTrace);
-      return onError ?? const ScaffoldErrorBody();
+      _asyncErrorLogger.severe("Error occured", error, stackTrace);
+      return onError?.call(error, stackTrace) ?? const ScaffoldErrorBody();
     }
 
     return onData(requireValue);

diff --git a/mobile/lib/modules/activities/views/activities_page.dart b/mobile/lib/modules/activities/views/activities_page.dart
@@ -4,12 +4,12 @@ import 'package:easy_localization/easy_localization.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter_hooks/flutter_hooks.dart' hide Store;
 import 'package:hooks_riverpod/hooks_riverpod.dart';
+import 'package:immich_mobile/extensions/asyncvalue_extensions.dart';
 import 'package:immich_mobile/extensions/build_context_extensions.dart';
 import 'package:immich_mobile/modules/activities/models/activity.model.dart';
 import 'package:immich_mobile/modules/activities/providers/activity.provider.dart';
 import 'package:immich_mobile/shared/models/store.dart';
 import 'package:immich_mobile/shared/ui/confirm_dialog.dart';
-import 'package:immich_mobile/shared/ui/immich_loading_indicator.dart';
 import 'package:immich_mobile/shared/ui/user_circle_avatar.dart';
 import 'package:immich_mobile/extensions/datetime_extensions.dart';
 import 'package:immich_mobile/utils/image_url_builder.dart';
@@ -88,7 +88,7 @@ class ActivitiesPage extends HookConsumerWidget {
               width: 40,
               height: 30,
               decoration: BoxDecoration(
-                borderRadius: BorderRadius.circular(4),
+                borderRadius: const BorderRadius.all(Radius.circular(4)),
                 image: DecorationImage(
                   image: CachedNetworkImageProvider(
                     getThumbnailUrlForRemoteId(
@@ -231,11 +231,8 @@ class ActivitiesPage extends HookConsumerWidget {
 
     return Scaffold(
       appBar: AppBar(title: Text(appBarTitle)),
-      body: activities.maybeWhen(
-        orElse: () {
-          return const Center(child: ImmichLoadingIndicator());
-        },
-        data: (data) {
+      body: activities.widgetWhen(
+        onData: (data) {
           final liked = data.firstWhereOrNull(
             (a) =>
                 a.type == ActivityType.like &&

diff --git a/mobile/lib/modules/album/views/album_options_part.dart b/mobile/lib/modules/album/views/album_options_part.dart
@@ -180,9 +180,7 @@ class AlbumOptionsPage extends HookConsumerWidget {
       appBar: AppBar(
         leading: IconButton(
           icon: const Icon(Icons.arrow_back_ios_new_rounded),
-          onPressed: () {
-            context.autoPop(null);
-          },
+          onPressed: () => context.autoPop(null),
         ),
         centerTitle: true,
         title: Text("translated_text_options".tr()),

diff --git a/mobile/lib/modules/album/views/album_viewer_page.dart b/mobile/lib/modules/album/views/album_viewer_page.dart
@@ -4,6 +4,7 @@ import 'package:easy_localization/easy_localization.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter_hooks/flutter_hooks.dart';
 import 'package:hooks_riverpod/hooks_riverpod.dart';
+import 'package:immich_mobile/extensions/asyncvalue_extensions.dart';
 import 'package:immich_mobile/extensions/build_context_extensions.dart';
 import 'package:immich_mobile/modules/album/models/asset_selection_page_result.model.dart';
 import 'package:immich_mobile/modules/album/providers/album_detail.provider.dart';
@@ -17,7 +18,6 @@ import 'package:immich_mobile/routing/router.dart';
 import 'package:immich_mobile/shared/models/album.dart';
 import 'package:immich_mobile/shared/models/asset.dart';
 import 'package:immich_mobile/shared/providers/asset.provider.dart';
-import 'package:immich_mobile/shared/ui/immich_loading_indicator.dart';
 import 'package:immich_mobile/shared/ui/user_circle_avatar.dart';
 import 'package:immich_mobile/shared/views/immich_loading_overlay.dart';
 
@@ -260,13 +260,11 @@ class AlbumViewerPage extends HookConsumerWidget {
         error: (error, stackTrace) => AppBar(title: const Text("Error")),
         loading: () => AppBar(),
       ),
-      body: album.when(
-        data: (data) => WillPopScope(
+      body: album.widgetWhen(
+        onData: (data) => WillPopScope(
           onWillPop: onWillPop,
           child: GestureDetector(
-            onTap: () {
-              titleFocusNode.unfocus();
-            },
+            onTap: () => titleFocusNode.unfocus(),
             child: ImmichAssetGrid(
               renderList: data.renderList,
               listener: selectionListener,
@@ -285,10 +283,6 @@ class AlbumViewerPage extends HookConsumerWidget {
             ),
           ),
         ),
-        error: (e, _) => Center(child: Text("Error loading album info!\n$e")),
-        loading: () => const Center(
-          child: ImmichLoadingIndicator(),
-        ),
       ),
     );
   }

diff --git a/mobile/lib/modules/album/views/asset_selection_page.dart b/mobile/lib/modules/album/views/asset_selection_page.dart
@@ -3,6 +3,7 @@ import 'package:easy_localization/easy_localization.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter_hooks/flutter_hooks.dart';
 import 'package:hooks_riverpod/hooks_riverpod.dart';
+import 'package:immich_mobile/extensions/asyncvalue_extensions.dart';
 import 'package:immich_mobile/extensions/build_context_extensions.dart';
 import 'package:immich_mobile/modules/album/models/asset_selection_page_result.model.dart';
 import 'package:immich_mobile/modules/asset_viewer/providers/render_list.provider.dart';
@@ -85,12 +86,8 @@ class AssetSelectionPage extends HookConsumerWidget {
             ),
         ],
       ),
-      body: renderList.when(
-        data: (data) => buildBody(data),
-        error: (error, stackTrace) => Center(
-          child: Text(error.toString()),
-        ),
-        loading: () => const Center(child: CircularProgressIndicator()),
+      body: renderList.widgetWhen(
+        onData: (data) => buildBody(data),
       ),
     );
   }

diff --git a/mobile/lib/modules/album/views/select_additional_user_for_sharing_page.dart b/mobile/lib/modules/album/views/select_additional_user_for_sharing_page.dart
@@ -2,11 +2,11 @@ import 'package:easy_localization/easy_localization.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter_hooks/flutter_hooks.dart';
 import 'package:hooks_riverpod/hooks_riverpod.dart';
+import 'package:immich_mobile/extensions/asyncvalue_extensions.dart';
 import 'package:immich_mobile/extensions/build_context_extensions.dart';
 import 'package:immich_mobile/modules/album/providers/suggested_shared_users.provider.dart';
 import 'package:immich_mobile/shared/models/album.dart';
 import 'package:immich_mobile/shared/models/user.dart';
-import 'package:immich_mobile/shared/ui/immich_loading_indicator.dart';
 import 'package:immich_mobile/shared/ui/user_circle_avatar.dart';
 
 class SelectAdditionalUserForSharingPage extends HookConsumerWidget {
@@ -137,8 +137,8 @@ class SelectAdditionalUserForSharingPage extends HookConsumerWidget {
           ),
         ],
       ),
-      body: suggestedShareUsers.when(
-        data: (users) {
+      body: suggestedShareUsers.widgetWhen(
+        onData: (users) {
           for (var sharedUsers in album.sharedUsers) {
             users.removeWhere(
               (u) => u.id == sharedUsers.id || u.id == album.ownerId,
@@ -147,10 +147,6 @@ class SelectAdditionalUserForSharingPage extends HookConsumerWidget {
 
           return buildUserList(users);
         },
-        error: (e, _) => Text("Error loading suggested users $e"),
-        loading: () => const Center(
-          child: ImmichLoadingIndicator(),
-        ),
       ),
     );
   }

diff --git a/mobile/lib/modules/album/views/select_user_for_sharing_page.dart b/mobile/lib/modules/album/views/select_user_for_sharing_page.dart
@@ -2,14 +2,14 @@ import 'package:easy_localization/easy_localization.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter_hooks/flutter_hooks.dart';
 import 'package:hooks_riverpod/hooks_riverpod.dart';
+import 'package:immich_mobile/extensions/asyncvalue_extensions.dart';
 import 'package:immich_mobile/extensions/build_context_extensions.dart';
 import 'package:immich_mobile/modules/album/providers/album_title.provider.dart';
 import 'package:immich_mobile/modules/album/providers/shared_album.provider.dart';
 import 'package:immich_mobile/modules/album/providers/suggested_shared_users.provider.dart';
 import 'package:immich_mobile/routing/router.dart';
 import 'package:immich_mobile/shared/models/asset.dart';
 import 'package:immich_mobile/shared/models/user.dart';
-import 'package:immich_mobile/shared/ui/immich_loading_indicator.dart';
 import 'package:immich_mobile/shared/ui/user_circle_avatar.dart';
 
 class SelectUserForSharingPage extends HookConsumerWidget {
@@ -42,7 +42,12 @@ class SelectUserForSharingPage extends HookConsumerWidget {
 
       ScaffoldMessenger(
         child: SnackBar(
-          content: const Text('select_user_for_sharing_page_err_album').tr(),
+          content: Text(
+            'select_user_for_sharing_page_err_album',
+            style: context.textTheme.bodyLarge?.copyWith(
+              color: context.primaryColor,
+            ),
+          ).tr(),
         ),
       );
     }
@@ -166,14 +171,10 @@ class SelectUserForSharingPage extends HookConsumerWidget {
           ),
         ],
       ),
-      body: suggestedShareUsers.when(
-        data: (users) {
+      body: suggestedShareUsers.widgetWhen(
+        onData: (users) {
           return buildUserList(users);
         },
-        error: (e, _) => Text("Error loading suggested users $e"),
-        loading: () => const Center(
-          child: ImmichLoadingIndicator(),
-        ),
       ),
     );
   }

diff --git a/mobile/lib/modules/archive/views/archive_page.dart b/mobile/lib/modules/archive/views/archive_page.dart
@@ -2,6 +2,7 @@ import 'package:easy_localization/easy_localization.dart';
 import 'package:flutter/material.dart';
 import 'package:flutter_hooks/flutter_hooks.dart';
 import 'package:hooks_riverpod/hooks_riverpod.dart';
+import 'package:immich_mobile/extensions/asyncvalue_extensions.dart';
 import 'package:immich_mobile/extensions/build_context_extensions.dart';
 import 'package:immich_mobile/modules/archive/providers/archive_asset_provider.dart';
 import 'package:immich_mobile/modules/home/ui/asset_grid/immich_asset_grid.dart';
@@ -48,56 +49,47 @@ class ArchivePage extends HookConsumerWidget {
           child: SizedBox(
             height: 64,
             child: Card(
-              child: Column(
-                children: [
-                  ListTile(
-                    shape: RoundedRectangleBorder(
-                      borderRadius: BorderRadius.circular(10),
-                    ),
-                    leading: const Icon(
-                      Icons.unarchive_rounded,
-                    ),
-                    title: Text(
-                      'control_bottom_app_bar_unarchive'.tr(),
-                      style: const TextStyle(fontSize: 14),
-                    ),
-                    onTap: processing.value
-                        ? null
-                        : () async {
-                            processing.value = true;
-                            try {
-                              await handleArchiveAssets(
-                                ref,
-                                context,
-                                selection.value.toList(),
-                                shouldArchive: false,
-                              );
-                            } finally {
-                              processing.value = false;
-                              selectionEnabledHook.value = false;
-                            }
-                          },
-                  ),
-                ],
+              child: ListTile(
+                shape: const RoundedRectangleBorder(
+                  borderRadius: BorderRadius.all(Radius.circular(10)),
+                ),
+                leading: const Icon(
+                  Icons.unarchive_rounded,
+                ),
+                title: Text(
+                  'control_bottom_app_bar_unarchive'.tr(),
+                  style: const TextStyle(fontSize: 14),
+                ),
+                onTap: processing.value
+                    ? null
+                    : () async {
+                        processing.value = true;
+                        try {
+                          await handleArchiveAssets(
+                            ref,
+                            context,
+                            selection.value.toList(),
+                            shouldArchive: false,
+                          );
+                        } finally {
+                          processing.value = false;
+                          selectionEnabledHook.value = false;
+                        }
+                      },
               ),
             ),
           ),
         ),
       );
     }
 
-    return archivedAssets.when(
-      loading: () => Scaffold(
-        appBar: buildAppBar("?"),
-        body: const Center(child: CircularProgressIndicator()),
-      ),
-      error: (error, stackTrace) => Scaffold(
-        appBar: buildAppBar("Error"),
-        body: Center(child: Text(error.toString())),
+    return Scaffold(
+      appBar: archivedAssets.maybeWhen(
+        data: (data) => buildAppBar(data.totalAssets.toString()),
+        orElse: () => buildAppBar("?"),
       ),
-      data: (data) => Scaffold(
-        appBar: buildAppBar(data.totalAssets.toString()),
-        body: data.isEmpty
+      body: archivedAssets.widgetWhen(
+        onData: (data) => data.isEmpty
             ? Center(
                 child: Text('archive_page_no_archived_assets'.tr()),
               )

diff --git a/mobile/lib/modules/asset_viewer/ui/advanced_bottom_sheet.dart b/mobile/lib/modules/asset_viewer/ui/advanced_bottom_sheet.dart
@@ -62,8 +62,14 @@ class AdvancedBottomSheet extends HookConsumerWidget {
                                   ClipboardData(text: assetDetail.toString()),
                                 ).then((_) {
                                   ScaffoldMessenger.of(context).showSnackBar(
-                                    const SnackBar(
-                                      content: Text("Copied to clipboard"),
+                                    SnackBar(
+                                      content: Text(
+                                        "Copied to clipboard",
+                                        style: context.textTheme.bodyLarge
+                                            ?.copyWith(
+                                          color: context.primaryColor,
+                                        ),
+                                      ),
                                     ),
                                   );
                                 });

diff --git a/mobile/lib/modules/backup/views/backup_controller_page.dart b/mobile/lib/modules/backup/views/backup_controller_page.dart
@@ -229,6 +229,9 @@ class BackupControllerPage extends HookConsumerWidget {
       final snackBar = SnackBar(
         content: Text(
           msg.tr(),
+          style: context.textTheme.bodyLarge?.copyWith(
+            color: context.primaryColor,
+          ),
         ),
         backgroundColor: Colors.red,
       );