refactor(mobile): album sort (#5510)

* refactor: migrate album sort option to provider * refactor: use sort order in add to album sheet list * test(mobile): album_sort_options_provider unit tests * refactor: sort shared albums with user selected sort * refactor: use listview to render shared albums * refactor: rename to AlbumSortByOptions * refactor: remove filtering inside sort functions * font size --------- Co-authored-by: shalong-tanwen <139912620+shalong-tanwen@users.noreply.github.com> Co-authored-by: Alex Tran <alex.tran1502@gmail.com>
2025-12-16 09:13:13 +03:00 · 2023-12-07 18:14:09 +00:00
parent 2e59b07cc6
commit c5504aae6e
13 changed files with 546 additions and 94 deletions
--- a/mobile/lib/modules/album/providers/album_sort_by_options.provider.dart
+++ b/mobile/lib/modules/album/providers/album_sort_by_options.provider.dart
@@ -0,0 +1,131 @@
+import 'package:collection/collection.dart';
+import 'package:immich_mobile/modules/settings/providers/app_settings.provider.dart';
+import 'package:immich_mobile/modules/settings/services/app_settings.service.dart';
+import 'package:immich_mobile/shared/models/album.dart';
+import 'package:riverpod_annotation/riverpod_annotation.dart';
+
+part 'album_sort_by_options.provider.g.dart';
+
+typedef AlbumSortFn = List<Album> Function(List<Album> albums, bool isReverse);
+
+class _AlbumSortHandlers {
+  const _AlbumSortHandlers._();
+
+  static const AlbumSortFn created = _sortByCreated;
+  static List<Album> _sortByCreated(List<Album> albums, bool isReverse) {
+    final sorted = albums.sortedBy((album) => album.createdAt);
+    return (isReverse ? sorted.reversed : sorted).toList();
+  }
+
+  static const AlbumSortFn title = _sortByTitle;
+  static List<Album> _sortByTitle(List<Album> albums, bool isReverse) {
+    final sorted = albums.sortedBy((album) => album.name);
+    return (isReverse ? sorted.reversed : sorted).toList();
+  }
+
+  static const AlbumSortFn lastModified = _sortByLastModified;
+  static List<Album> _sortByLastModified(List<Album> albums, bool isReverse) {
+    final sorted = albums.sortedBy((album) => album.modifiedAt);
+    return (isReverse ? sorted.reversed : sorted).toList();
+  }
+
+  static const AlbumSortFn assetCount = _sortByAssetCount;
+  static List<Album> _sortByAssetCount(List<Album> albums, bool isReverse) {
+    final sorted =
+        albums.sorted((a, b) => a.assetCount.compareTo(b.assetCount));
+    return (isReverse ? sorted.reversed : sorted).toList();
+  }
+
+  static const AlbumSortFn mostRecent = _sortByMostRecent;
+  static List<Album> _sortByMostRecent(List<Album> albums, bool isReverse) {
+    final sorted = albums.sorted((a, b) {
+      if (a.endDate != null && b.endDate != null) {
+        return a.endDate!.compareTo(b.endDate!);
+      }
+      if (a.endDate == null) return 1;
+      if (b.endDate == null) return -1;
+      return 0;
+    });
+    return (isReverse ? sorted.reversed : sorted).toList();
+  }
+
+  static const AlbumSortFn mostOldest = _sortByMostOldest;
+  static List<Album> _sortByMostOldest(List<Album> albums, bool isReverse) {
+    final sorted = albums.sorted((a, b) {
+      if (a.startDate != null && b.startDate != null) {
+        return a.startDate!.compareTo(b.startDate!);
+      }
+      if (a.startDate == null) return 1;
+      if (b.startDate == null) return -1;
+      return 0;
+    });
+    return (isReverse ? sorted.reversed : sorted).toList();
+  }
+}
+
+// Store index allows us to re-arrange the values without affecting the saved prefs
+enum AlbumSortMode {
+  title(1, "library_page_sort_title", _AlbumSortHandlers.title),
+  assetCount(4, "library_page_sort_asset_count", _AlbumSortHandlers.assetCount),
+  lastModified(
+    3,
+    "library_page_sort_last_modified",
+    _AlbumSortHandlers.lastModified,
+  ),
+  created(0, "library_page_sort_created", _AlbumSortHandlers.created),
+  mostRecent(
+    2,
+    "library_page_sort_most_recent_photo",
+    _AlbumSortHandlers.mostRecent,
+  ),
+  mostOldest(
+    5,
+    "library_page_sort_most_oldest_photo",
+    _AlbumSortHandlers.mostOldest,
+  );
+
+  final int storeIndex;
+  final String label;
+  final AlbumSortFn sortFn;
+
+  const AlbumSortMode(this.storeIndex, this.label, this.sortFn);
+}
+
+@riverpod
+class AlbumSortByOptions extends _$AlbumSortByOptions {
+  @override
+  AlbumSortMode build() {
+    final sortOpt = ref
+        .watch(appSettingsServiceProvider)
+        .getSetting(AppSettingsEnum.selectedAlbumSortOrder);
+    return AlbumSortMode.values.firstWhere(
+      (e) => e.index == sortOpt,
+      orElse: () => AlbumSortMode.title,
+    );
+  }
+
+  void changeSortMode(AlbumSortMode sortOption) {
+    state = sortOption;
+    ref.watch(appSettingsServiceProvider).setSetting(
+          AppSettingsEnum.selectedAlbumSortOrder,
+          sortOption.storeIndex,
+        );
+  }
+}
+
+@riverpod
+class AlbumSortOrder extends _$AlbumSortOrder {
+  @override
+  bool build() {
+    return ref
+        .watch(appSettingsServiceProvider)
+        .getSetting(AppSettingsEnum.selectedAlbumSortReverse);
+  }
+
+  void changeSortDirection(bool isReverse) {
+    state = isReverse;
+    ref
+        .watch(appSettingsServiceProvider)
+        .setSetting(AppSettingsEnum.selectedAlbumSortReverse, isReverse);
+  }
+}
--- a/mobile/lib/modules/album/providers/album_sort_by_options.provider.g.dart
+++ b/mobile/lib/modules/album/providers/album_sort_by_options.provider.g.dart
@@ -0,0 +1,43 @@
+// GENERATED CODE - DO NOT MODIFY BY HAND
+
+part of 'album_sort_by_options.provider.dart';
+
+// **************************************************************************
+// RiverpodGenerator
+// **************************************************************************
+
+String _$albumSortByOptionsHash() =>
+    r'8d22fa8b7cbca2d3d7ed20a83bf00211dc948004';
+
+/// See also [AlbumSortByOptions].
+@ProviderFor(AlbumSortByOptions)
+final albumSortByOptionsProvider =
+    AutoDisposeNotifierProvider<AlbumSortByOptions, AlbumSortMode>.internal(
+  AlbumSortByOptions.new,
+  name: r'albumSortByOptionsProvider',
+  debugGetCreateSourceHash: const bool.fromEnvironment('dart.vm.product')
+      ? null
+      : _$albumSortByOptionsHash,
+  dependencies: null,
+  allTransitiveDependencies: null,
+);
+
+typedef _$AlbumSortByOptions = AutoDisposeNotifier<AlbumSortMode>;
+String _$albumSortOrderHash() => r'573dea45b4519e69386fc7104c72522e35713440';
+
+/// See also [AlbumSortOrder].
+@ProviderFor(AlbumSortOrder)
+final albumSortOrderProvider =
+    AutoDisposeNotifierProvider<AlbumSortOrder, bool>.internal(
+  AlbumSortOrder.new,
+  name: r'albumSortOrderProvider',
+  debugGetCreateSourceHash: const bool.fromEnvironment('dart.vm.product')
+      ? null
+      : _$albumSortOrderHash,
+  dependencies: null,
+  allTransitiveDependencies: null,
+);
+
+typedef _$AlbumSortOrder = AutoDisposeNotifier<bool>;
+// ignore_for_file: type=lint
+// ignore_for_file: subtype_of_sealed_class, invalid_use_of_internal_member, invalid_use_of_visible_for_testing_member
--- a/mobile/lib/modules/album/ui/add_to_album_sliverlist.dart
+++ b/mobile/lib/modules/album/ui/add_to_album_sliverlist.dart
@@ -1,6 +1,7 @@
 import 'package:easy_localization/easy_localization.dart';
 import 'package:flutter/material.dart';
 import 'package:hooks_riverpod/hooks_riverpod.dart';
+import 'package:immich_mobile/modules/album/providers/album_sort_by_options.provider.dart';
 import 'package:immich_mobile/modules/album/ui/album_thumbnail_listtile.dart';
 import 'package:immich_mobile/shared/models/album.dart';

@@ -21,33 +22,44 @@ class AddToAlbumSliverList extends HookConsumerWidget {

  @override
  Widget build(BuildContext context, WidgetRef ref) {
+    final albumSortMode = ref.watch(albumSortByOptionsProvider);
+    final albumSortIsReverse = ref.watch(albumSortOrderProvider);
+    final sortedAlbums = albumSortMode.sortFn(albums, albumSortIsReverse);
+    final sortedSharedAlbums =
+        albumSortMode.sortFn(sharedAlbums, albumSortIsReverse);
+
    return SliverList(
      delegate: SliverChildBuilderDelegate(
          childCount: albums.length + (sharedAlbums.isEmpty ? 0 : 1),
          (context, index) {
        // Build shared expander
-        if (index == 0 && sharedAlbums.isNotEmpty) {
+        if (index == 0 && sortedSharedAlbums.isNotEmpty) {
          return Padding(
            padding: const EdgeInsets.only(bottom: 8),
            child: ExpansionTile(
              title: Text('common_shared'.tr()),
              tilePadding: const EdgeInsets.symmetric(horizontal: 10.0),
              leading: const Icon(Icons.group),
-              children: sharedAlbums
-                  .map(
-                    (album) => AlbumThumbnailListTile(
-                      album: album,
-                      onTap: enabled ? () => onAddToAlbum(album) : () {},
-                    ),
-                  )
-                  .toList(),
+              children: [
+                ListView.builder(
+                  shrinkWrap: true,
+                  physics: const ClampingScrollPhysics(),
+                  itemCount: sortedSharedAlbums.length,
+                  itemBuilder: (context, index) => AlbumThumbnailListTile(
+                    album: sortedSharedAlbums[index],
+                    onTap: enabled
+                        ? () => onAddToAlbum(sortedSharedAlbums[index])
+                        : () {},
+                  ),
+                ),
+              ],
            ),
          );
        }

        // Build albums list
        final offset = index - (sharedAlbums.isNotEmpty ? 1 : 0);
-        final album = albums[offset];
+        final album = sortedAlbums[offset];
        return AlbumThumbnailListTile(
          album: album,
          onTap: enabled ? () => onAddToAlbum(album) : () {},
--- a/mobile/lib/modules/album/ui/album_thumbnail_card.dart
+++ b/mobile/lib/modules/album/ui/album_thumbnail_card.dart
@@ -110,6 +110,7 @@ class AlbumThumbnailCard extends StatelessWidget {
                        width: cardSize,
                        child: Text(
                          album.name,
+                          overflow: TextOverflow.ellipsis,
                          style: context.textTheme.bodyMedium?.copyWith(
                            color: context.primaryColor,
                            fontWeight: FontWeight.w500,
--- a/mobile/lib/modules/album/views/library_page.dart
+++ b/mobile/lib/modules/album/views/library_page.dart
@@ -1,15 +1,12 @@
-import 'package:collection/collection.dart';
 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/build_context_extensions.dart';
 import 'package:immich_mobile/modules/album/providers/album.provider.dart';
+import 'package:immich_mobile/modules/album/providers/album_sort_by_options.provider.dart';
 import 'package:immich_mobile/modules/album/ui/album_thumbnail_card.dart';
 import 'package:immich_mobile/routing/router.dart';
-import 'package:immich_mobile/shared/models/album.dart';
-import 'package:immich_mobile/modules/settings/providers/app_settings.provider.dart';
-import 'package:immich_mobile/modules/settings/services/app_settings.service.dart';
 import 'package:immich_mobile/shared/providers/server_info.provider.dart';
 import 'package:immich_mobile/shared/ui/immich_app_bar.dart';

@@ -21,8 +18,9 @@ class LibraryPage extends HookConsumerWidget {
    final trashEnabled =
        ref.watch(serverInfoProvider.select((v) => v.serverFeatures.trash));
    final albums = ref.watch(albumProvider);
-    var isDarkTheme = context.isDarkTheme;
-    var settings = ref.watch(appSettingsServiceProvider);
+    final isDarkTheme = context.isDarkTheme;
+    final albumSortOption = ref.watch(albumSortByOptionsProvider);
+    final albumSortIsReverse = ref.watch(albumSortOrderProvider);

    useEffect(
      () {
@@ -32,64 +30,15 @@ class LibraryPage extends HookConsumerWidget {
      [],
    );

-    final selectedAlbumSortOrder =
-        useState(settings.getSetting(AppSettingsEnum.selectedAlbumSortOrder));
-
-    List<Album> sortedAlbums() {
-      // Created.
-      if (selectedAlbumSortOrder.value == 0) {
-        return albums
-            .where((a) => a.isRemote)
-            .sortedBy((album) => album.createdAt)
-            .reversed
-            .toList();
-      }
-      // Album title.
-      if (selectedAlbumSortOrder.value == 1) {
-        return albums.where((a) => a.isRemote).sortedBy((album) => album.name);
-      }
-      // Most recent photo, if unset (e.g. empty album, use modifiedAt / updatedAt).
-      if (selectedAlbumSortOrder.value == 2) {
-        return albums
-            .where((a) => a.isRemote)
-            .sorted(
-              (a, b) => a.lastModifiedAssetTimestamp != null &&
-                      b.lastModifiedAssetTimestamp != null
-                  ? a.lastModifiedAssetTimestamp!
-                      .compareTo(b.lastModifiedAssetTimestamp!)
-                  : a.modifiedAt.compareTo(b.modifiedAt),
-            )
-            .reversed
-            .toList();
-      }
-      // Last modified.
-      if (selectedAlbumSortOrder.value == 3) {
-        return albums
-            .where((a) => a.isRemote)
-            .sortedBy((album) => album.modifiedAt)
-            .reversed
-            .toList();
-      }
-
-      // Fallback: Album title.
-      return albums.where((a) => a.isRemote).sortedBy((album) => album.name);
-    }
-
    Widget buildSortButton() {
-      final options = [
-        "library_page_sort_created".tr(),
-        "library_page_sort_title".tr(),
-        "library_page_sort_most_recent_photo".tr(),
-        "library_page_sort_last_modified".tr(),
-      ];
-
      return PopupMenuButton(
        position: PopupMenuPosition.over,
        itemBuilder: (BuildContext context) {
-          return options.mapIndexed<PopupMenuEntry<int>>((index, option) {
-            final selected = selectedAlbumSortOrder.value == index;
+          return AlbumSortMode.values
+              .map<PopupMenuEntry<AlbumSortMode>>((option) {
+            final selected = albumSortOption == option;
            return PopupMenuItem(
-              value: index,
+              value: option,
              child: Row(
                children: [
                  Padding(
@@ -101,10 +50,10 @@ class LibraryPage extends HookConsumerWidget {
                    ),
                  ),
                  Text(
-                    option,
+                    option.label.tr(),
                    style: TextStyle(
                      color: selected ? context.primaryColor : null,
-                      fontSize: 12.0,
+                      fontSize: 14.0,
                    ),
                  ),
                ],
@@ -112,19 +61,31 @@ class LibraryPage extends HookConsumerWidget {
            );
          }).toList();
        },
-        onSelected: (int value) {
-          selectedAlbumSortOrder.value = value;
-          settings.setSetting(AppSettingsEnum.selectedAlbumSortOrder, value);
+        onSelected: (AlbumSortMode value) {
+          final selected = albumSortOption == value;
+          // Switch direction
+          if (selected) {
+            ref
+                .read(albumSortOrderProvider.notifier)
+                .changeSortDirection(!albumSortIsReverse);
+          } else {
+            ref.read(albumSortByOptionsProvider.notifier).changeSortMode(value);
+          }
        },
        child: Row(
          children: [
-            Icon(
-              Icons.swap_vert_rounded,
-              size: 18,
-              color: context.primaryColor,
+            Padding(
+              padding: const EdgeInsets.only(right: 5),
+              child: Icon(
+                albumSortIsReverse
+                    ? Icons.arrow_downward_rounded
+                    : Icons.arrow_upward_rounded,
+                size: 14,
+                color: context.primaryColor,
+              ),
            ),
            Text(
-              options[selectedAlbumSortOrder.value],
+              albumSortOption.label.tr(),
              style: context.textTheme.labelLarge?.copyWith(
                color: context.primaryColor,
              ),
@@ -140,9 +101,8 @@ class LibraryPage extends HookConsumerWidget {
          var cardSize = constraints.maxWidth;

          return GestureDetector(
-            onTap: () {
-              context.autoPush(CreateAlbumRoute(isSharedAlbum: false));
-            },
+            onTap: () =>
+                context.autoPush(CreateAlbumRoute(isSharedAlbum: false)),
            child: Padding(
              padding:
                  const EdgeInsets.only(bottom: 32), // Adjust padding to suit
@@ -160,7 +120,7 @@ class LibraryPage extends HookConsumerWidget {
                            : const Color.fromARGB(255, 203, 203, 203),
                      ),
                      color: isDarkTheme ? Colors.grey[900] : Colors.grey[50],
-                      borderRadius: BorderRadius.circular(20),
+                      borderRadius: const BorderRadius.all(Radius.circular(20)),
                    ),
                    child: Center(
                      child: Icon(
@@ -223,15 +183,15 @@ class LibraryPage extends HookConsumerWidget {
      );
    }

-    final sorted = sortedAlbums();
-
+    final remote = albums.where((a) => a.isRemote).toList();
+    final sorted = albumSortOption.sortFn(remote, albumSortIsReverse);
    final local = albums.where((a) => a.isLocal).toList();

    Widget? shareTrashButton() {
      return trashEnabled
          ? InkWell(
              onTap: () => context.autoPush(const TrashRoute()),
-              borderRadius: BorderRadius.circular(12),
+              borderRadius: const BorderRadius.all(Radius.circular(12)),
              child: const Icon(
                Icons.delete_rounded,
                size: 25,
--- a/mobile/lib/modules/album/views/sharing_page.dart
+++ b/mobile/lib/modules/album/views/sharing_page.dart
@@ -3,12 +3,12 @@ import 'package:flutter/material.dart';
 import 'package:flutter_hooks/flutter_hooks.dart';
 import 'package:hooks_riverpod/hooks_riverpod.dart';
 import 'package:immich_mobile/extensions/build_context_extensions.dart';
+import 'package:immich_mobile/modules/album/providers/album_sort_by_options.provider.dart';
 import 'package:immich_mobile/modules/album/providers/shared_album.provider.dart';
 import 'package:immich_mobile/modules/album/ui/album_thumbnail_card.dart';
 import 'package:immich_mobile/modules/partner/providers/partner.provider.dart';
 import 'package:immich_mobile/modules/partner/ui/partner_list.dart';
 import 'package:immich_mobile/routing/router.dart';
-import 'package:immich_mobile/shared/models/album.dart';
 import 'package:immich_mobile/shared/providers/user.provider.dart';
 import 'package:immich_mobile/shared/ui/immich_app_bar.dart';
 import 'package:immich_mobile/shared/ui/immich_image.dart';
@@ -18,7 +18,10 @@ class SharingPage extends HookConsumerWidget {

  @override
  Widget build(BuildContext context, WidgetRef ref) {
-    final List<Album> sharedAlbums = ref.watch(sharedAlbumProvider);
+    final albumSortOption = ref.watch(albumSortByOptionsProvider);
+    final albumSortIsReverse = ref.watch(albumSortOrderProvider);
+    final albums = ref.watch(sharedAlbumProvider);
+    final sharedAlbums = albumSortOption.sortFn(albums, albumSortIsReverse);
    final userId = ref.watch(currentUserProvider)?.id;
    final partner = ref.watch(partnerSharedWithProvider);

@@ -68,7 +71,7 @@ class SharingPage extends HookConsumerWidget {
            return ListTile(
              contentPadding: const EdgeInsets.symmetric(horizontal: 12),
              leading: ClipRRect(
-                borderRadius: BorderRadius.circular(8),
+                borderRadius: const BorderRadius.all(Radius.circular(8)),
                child: ImmichImage(
                  album.thumbnail.value,
                  width: 60,
@@ -167,9 +170,9 @@ class SharingPage extends HookConsumerWidget {
          padding: const EdgeInsets.all(8.0),
          child: Card(
            elevation: 0,
-            shape: RoundedRectangleBorder(
-              borderRadius: BorderRadius.circular(20),
-              side: const BorderSide(
+            shape: const RoundedRectangleBorder(
+              borderRadius: BorderRadius.all(Radius.circular(20)),
+              side: BorderSide(
                color: Colors.grey,
                width: 0.5,
              ),
@@ -212,7 +215,7 @@ class SharingPage extends HookConsumerWidget {
    Widget sharePartnerButton() {
      return InkWell(
        onTap: () => context.autoPush(const PartnerRoute()),
-        borderRadius: BorderRadius.circular(12),
+        borderRadius: const BorderRadius.all(Radius.circular(12)),
        child: const Icon(
          Icons.swap_horizontal_circle_rounded,
          size: 25,
--- a/mobile/lib/modules/settings/services/app_settings.service.dart
+++ b/mobile/lib/modules/settings/services/app_settings.service.dart
@@ -52,6 +52,11 @@ enum AppSettingsEnum<T> {
  mapRelativeDate<int>(StoreKey.mapRelativeDate, null, 0),
  allowSelfSignedSSLCert<bool>(StoreKey.selfSignedCert, null, false),
  ignoreIcloudAssets<bool>(StoreKey.ignoreIcloudAssets, null, false),
+  selectedAlbumSortReverse<bool>(
+    StoreKey.selectedAlbumSortReverse,
+    null,
+    false,
+  ),
  ;

  const AppSettingsEnum(this.storeKey, this.hiveKey, this.defaultValue);