fix(mobile): don't update search filters in-place (#26965)

* fix(mobile): don't update search filters in-place

Search filters are currently modified in-place, which can feel quite
janky. The chips behind the bottom sheet update instantly, and the
search page gets confused because filters have been applied but no
search has been initiated. Filters should keep their own copy of the
filter when they're opened, and the commit + search on apply.

The previous filter and pre-filter concepts were also cleaned up. They
added complexity, and `search()` now owns the full life cycle of the
filter.

This now also reverts the changes from #27155, as this solution should
be cleaner.

* refactor & color tweak

---------

Co-authored-by: Alex <alex.tran1502@gmail.com>

mobile/lib/presentation/pages/search/drift_search.page.dart

@@ -52,24 +52,20 @@ class DriftSearchPage extends HookConsumerWidget {
           : 'file_name_or_extension'.t(context: context),
     );
     final textSearchController = useTextEditingController();
-    final preFilter = ref.watch(searchPreFilterProvider);
     final filter = useState<SearchFilter>(
       SearchFilter(
-        people: preFilter?.people ?? {},
+        people: {},
-        location: preFilter?.location ?? SearchLocationFilter(),
+        location: SearchLocationFilter(),
-        camera: preFilter?.camera ?? SearchCameraFilter(),
+        camera: SearchCameraFilter(),
-        date: preFilter?.date ?? SearchDateFilter(),
+        date: SearchDateFilter(),
-        display: preFilter?.display ?? SearchDisplayFilters(isNotInAlbum: false, isArchive: false, isFavorite: false),
+        display: SearchDisplayFilters(isNotInAlbum: false, isArchive: false, isFavorite: false),
-        rating: preFilter?.rating ?? SearchRatingFilter(),
+        rating: SearchRatingFilter(),
-        mediaType: preFilter?.mediaType ?? AssetType.other,
+        mediaType: AssetType.other,
         language: "${context.locale.languageCode}-${context.locale.countryCode}",
-        assetId: preFilter?.assetId,
+        tagIds: [],
-        tagIds: preFilter?.tagIds ?? [],
       ),
     );
 
-    final previousFilter = useState<SearchFilter?>(null);
-    final hasRequestedSearch = useState<bool>(false);
     final dateInputFilter = useState<DateFilterInputModel?>(null);
 
     final peopleCurrentFilterWidget = useState<Widget?>(null);
@@ -83,68 +79,58 @@ class DriftSearchPage extends HookConsumerWidget {
 
     final userPreferences = ref.watch(userMetadataPreferencesProvider);
 
-    searchFilter(SearchFilter filter) {
+    search(SearchFilter f) {
-      if (preFilter == null && filter == previousFilter.value) {
+      if (f == filter.value) {
         return;
       }
 
+      filter.value = f;
+
       ref.read(paginatedSearchProvider.notifier).clear();
 
-      if (filter.isEmpty) {
+      if (!f.isEmpty) {
-        previousFilter.value = null;
+        unawaited(ref.read(paginatedSearchProvider.notifier).search(f));
-        hasRequestedSearch.value = false;
-        return;
       }
-
-      hasRequestedSearch.value = true;
-      unawaited(ref.read(paginatedSearchProvider.notifier).search(filter));
-      previousFilter.value = filter;
     }
 
-    search() => searchFilter(filter.value);
-
     loadMoreSearchResults() {
       unawaited(ref.read(paginatedSearchProvider.notifier).search(filter.value));
     }
 
-    searchPreFilter() {
+    // TODO: Use ref.listen with `fireImmediately` in the new riverpod version.
-      if (preFilter != null) {
+    final preFilter = ref.watch(searchPreFilterProvider);
-        Future.delayed(Duration.zero, () {
-          filter.value = preFilter;
-          textSearchController.clear();
-          searchFilter(preFilter);
-
-          if (preFilter.location.city != null) {
-            locationCurrentFilterWidget.value = Text(preFilter.location.city!, style: context.textTheme.labelLarge);
-          }
-        });
-      }
-    }
-
     useEffect(() {
-      Future.microtask(() => ref.invalidate(paginatedSearchProvider));
+      if (preFilter == null) {
-      searchPreFilter();
+        return null;
+      }
+
+      Future.microtask(() {
+        textSearchController.clear();
+        search(preFilter);
+        if (preFilter.location.city != null) {
+          locationCurrentFilterWidget.value = Text(preFilter.location.city!, style: context.textTheme.labelLarge);
+        }
+      });
 
       return null;
     }, [preFilter]);
 
     showPeoplePicker() {
-      handleOnSelect(Set<PersonDto> value) {
+      var people = filter.value.people;
-        filter.value = filter.value.copyWith(people: value);
 
-        final label = value.map((e) => e.name != '' ? e.name : 'no_name'.t(context: context)).join(', ');
+      handleOnSelect(Set<PersonDto> value) {
-        if (label.isNotEmpty) {
+        people = value;
-          peopleCurrentFilterWidget.value = Text(label, style: context.textTheme.labelLarge);
-        } else {
-          peopleCurrentFilterWidget.value = null;
-        }
       }
 
       handleClear() {
-        filter.value = filter.value.copyWith(people: {});
-
         peopleCurrentFilterWidget.value = null;
-        search();
+        search(filter.value.copyWith(people: {}));
+      }
+
+      handleApply() {
+        final label = people.map((e) => e.name != '' ? e.name : 'no_name'.t(context: context)).join(', ');
+        peopleCurrentFilterWidget.value = label.isNotEmpty ? Text(label, style: context.textTheme.labelLarge) : null;
+        search(filter.value.copyWith(people: people));
       }
 
       showFilterBottomSheet(
@@ -155,7 +141,7 @@ class DriftSearchPage extends HookConsumerWidget {
           child: FilterBottomSheetScaffold(
             title: 'search_filter_people_title'.t(context: context),
             expanded: true,
-            onSearch: search,
+            onSearch: handleApply,
             onClear: handleClear,
             child: PeoplePicker(onSelect: handleOnSelect, filter: filter.value.people),
           ),
@@ -164,23 +150,22 @@ class DriftSearchPage extends HookConsumerWidget {
     }
 
     showTagPicker() {
+      var tagIds = filter.value.tagIds ?? [];
+      String tagLabel = '';
+
       handleOnSelect(Iterable<Tag> tags) {
-        filter.value = filter.value.copyWith(tagIds: tags.map((t) => t.id).toList());
+        tagIds = tags.map((t) => t.id).toList();
-        final label = tags.map((t) => t.value).join(', ');
+        tagLabel = tags.map((t) => t.value).join(', ');
-        if (label.isEmpty) {
-          tagCurrentFilterWidget.value = null;
-        } else {
-          tagCurrentFilterWidget.value = Text(
-            label.isEmpty ? 'tags'.t(context: context) : label,
-            style: context.textTheme.labelLarge,
-          );
-        }
       }
 
       handleClear() {
-        filter.value = filter.value.copyWith(tagIds: []);
         tagCurrentFilterWidget.value = null;
-        search();
+        search(filter.value.copyWith(tagIds: []));
+      }
+
+      handleApply() {
+        tagCurrentFilterWidget.value = tagLabel.isNotEmpty ? Text(tagLabel, style: context.textTheme.labelLarge) : null;
+        search(filter.value.copyWith(tagIds: tagIds));
       }
 
       showFilterBottomSheet(
@@ -191,7 +176,7 @@ class DriftSearchPage extends HookConsumerWidget {
           child: FilterBottomSheetScaffold(
             title: 'search_filter_tags_title'.t(context: context),
             expanded: true,
-            onSearch: search,
+            onSearch: handleApply,
             onClear: handleClear,
             child: TagPicker(onSelect: handleOnSelect, filter: (filter.value.tagIds ?? []).toSet()),
           ),
@@ -200,32 +185,27 @@ class DriftSearchPage extends HookConsumerWidget {
     }
 
     showLocationPicker() {
+      var location = filter.value.location;
+
       handleOnSelect(Map<String, String?> value) {
-        filter.value = filter.value.copyWith(
+        location = SearchLocationFilter(country: value['country'], city: value['city'], state: value['state']);
-          location: SearchLocationFilter(country: value['country'], city: value['city'], state: value['state']),
-        );
-
-        final locationText = <String>[];
-        if (value['country'] != null) {
-          locationText.add(value['country']!);
-        }
-
-        if (value['state'] != null) {
-          locationText.add(value['state']!);
-        }
-
-        if (value['city'] != null) {
-          locationText.add(value['city']!);
-        }
-
-        locationCurrentFilterWidget.value = Text(locationText.join(', '), style: context.textTheme.labelLarge);
       }
 
       handleClear() {
-        filter.value = filter.value.copyWith(location: SearchLocationFilter());
-
         locationCurrentFilterWidget.value = null;
-        search();
+        search(filter.value.copyWith(location: SearchLocationFilter()));
+      }
+
+      handleApply() {
+        final locationText = [
+          if (location.country != null) location.country!,
+          if (location.state != null) location.state!,
+          if (location.city != null) location.city!,
+        ];
+        locationCurrentFilterWidget.value = locationText.isNotEmpty
+            ? Text(locationText.join(', '), style: context.textTheme.labelLarge)
+            : null;
+        search(filter.value.copyWith(location: location));
       }
 
       showFilterBottomSheet(
@@ -234,7 +214,7 @@ class DriftSearchPage extends HookConsumerWidget {
         isDismissible: true,
         child: FilterBottomSheetScaffold(
           title: 'search_filter_location_title'.t(context: context),
-          onSearch: search,
+          onSearch: handleApply,
           onClear: handleClear,
           child: Padding(
             padding: const EdgeInsets.symmetric(vertical: 16.0),
@@ -251,22 +231,24 @@ class DriftSearchPage extends HookConsumerWidget {
     }
 
     showCameraPicker() {
-      handleOnSelect(Map<String, String?> value) {
+      var camera = filter.value.camera;
-        filter.value = filter.value.copyWith(
-          camera: SearchCameraFilter(make: value['make'], model: value['model']),
-        );
 
-        cameraCurrentFilterWidget.value = Text(
+      handleOnSelect(Map<String, String?> value) {
-          '${value['make'] ?? ''} ${value['model'] ?? ''}',
+        camera = SearchCameraFilter(make: value['make'], model: value['model']);
-          style: context.textTheme.labelLarge,
-        );
       }
 
       handleClear() {
-        filter.value = filter.value.copyWith(camera: SearchCameraFilter());
-
         cameraCurrentFilterWidget.value = null;
-        search();
+        search(filter.value.copyWith(camera: SearchCameraFilter()));
+      }
+
+      handleApply() {
+        final make = camera.make ?? '';
+        final model = camera.model ?? '';
+        cameraCurrentFilterWidget.value = (make.isNotEmpty || model.isNotEmpty)
+            ? Text('$make $model', style: context.textTheme.labelLarge)
+            : null;
+        search(filter.value.copyWith(camera: camera));
       }
 
       showFilterBottomSheet(
@@ -275,7 +257,7 @@ class DriftSearchPage extends HookConsumerWidget {
         isDismissible: true,
         child: FilterBottomSheetScaffold(
           title: 'search_filter_camera_title'.t(context: context),
-          onSearch: search,
+          onSearch: handleApply,
           onClear: handleClear,
           child: Padding(
             padding: const EdgeInsets.all(16.0),
@@ -288,28 +270,24 @@ class DriftSearchPage extends HookConsumerWidget {
     datePicked(DateFilterInputModel? selectedDate) {
       dateInputFilter.value = selectedDate;
       if (selectedDate == null) {
-        filter.value = filter.value.copyWith(date: SearchDateFilter());
-
         dateRangeCurrentFilterWidget.value = null;
-        unawaited(search());
+        search(filter.value.copyWith(date: SearchDateFilter()));
         return;
       }
 
       final date = selectedDate.asDateTimeRange();
-
-      filter.value = filter.value.copyWith(
-        date: SearchDateFilter(
-          takenAfter: date.start,
-          takenBefore: date.end.add(const Duration(hours: 23, minutes: 59, seconds: 59)),
-        ),
-      );
-
       dateRangeCurrentFilterWidget.value = Text(
         selectedDate.asHumanReadable(context),
         style: context.textTheme.labelLarge,
       );
-
+      search(
-      unawaited(search());
+        filter.value.copyWith(
+          date: SearchDateFilter(
+            takenAfter: date.start,
+            takenBefore: date.end.add(const Duration(hours: 23, minutes: 59, seconds: 59)),
+          ),
+        ),
+      );
     }
 
     showDatePicker() async {
@@ -376,31 +354,32 @@ class DriftSearchPage extends HookConsumerWidget {
 
     // MEDIA PICKER
     showMediaTypePicker() {
-      handleOnSelected(AssetType assetType) {
+      var mediaType = filter.value.mediaType;
-        filter.value = filter.value.copyWith(mediaType: assetType);
 
-        mediaTypeCurrentFilterWidget.value = Text(
+      handleOnSelected(AssetType assetType) {
-          assetType == AssetType.image
+        mediaType = assetType;
-              ? 'image'.t(context: context)
-              : assetType == AssetType.video
-              ? 'video'.t(context: context)
-              : 'all'.t(context: context),
-          style: context.textTheme.labelLarge,
-        );
       }
 
       handleClear() {
-        filter.value = filter.value.copyWith(mediaType: AssetType.other);
-
         mediaTypeCurrentFilterWidget.value = null;
-        search();
+        search(filter.value.copyWith(mediaType: AssetType.other));
+      }
+
+      handleApply() {
+        mediaTypeCurrentFilterWidget.value = mediaType != AssetType.other
+            ? Text(
+                mediaType == AssetType.image ? 'image'.t(context: context) : 'video'.t(context: context),
+                style: context.textTheme.labelLarge,
+              )
+            : null;
+        search(filter.value.copyWith(mediaType: mediaType));
       }
 
       showFilterBottomSheet(
         context: context,
         child: FilterBottomSheetScaffold(
           title: 'search_filter_media_type_title'.t(context: context),
-          onSearch: search,
+          onSearch: handleApply,
           onClear: handleClear,
           child: MediaTypePicker(onSelect: handleOnSelected, filter: filter.value.mediaType),
         ),
@@ -409,19 +388,22 @@ class DriftSearchPage extends HookConsumerWidget {
 
     // STAR RATING PICKER
     showStarRatingPicker() {
-      handleOnSelected(SearchRatingFilter rating) {
+      var rating = filter.value.rating;
-        filter.value = filter.value.copyWith(rating: rating);
 
-        ratingCurrentFilterWidget.value = Text(
+      handleOnSelected(SearchRatingFilter value) {
-          'rating_count'.t(args: {'count': rating.rating!}),
+        rating = value;
-          style: context.textTheme.labelLarge,
-        );
       }
 
       handleClear() {
-        filter.value = filter.value.copyWith(rating: SearchRatingFilter(rating: null));
         ratingCurrentFilterWidget.value = null;
-        search();
+        search(filter.value.copyWith(rating: SearchRatingFilter(rating: null)));
+      }
+
+      handleApply() {
+        ratingCurrentFilterWidget.value = rating.rating != null
+            ? Text('rating_count'.t(args: {'count': rating.rating!}), style: context.textTheme.labelLarge)
+            : null;
+        search(filter.value.copyWith(rating: rating));
       }
 
       showFilterBottomSheet(
@@ -429,7 +411,7 @@ class DriftSearchPage extends HookConsumerWidget {
         isScrollControlled: true,
         child: FilterBottomSheetScaffold(
           title: 'rating'.t(context: context),
-          onSearch: search,
+          onSearch: handleApply,
           onClear: handleClear,
           child: StarRatingPicker(onSelect: handleOnSelected, filter: filter.value.rating),
         ),
@@ -438,79 +420,54 @@ class DriftSearchPage extends HookConsumerWidget {
 
     // DISPLAY OPTION
     showDisplayOptionPicker() {
+      var display = filter.value.display;
+
       handleOnSelect(Map<DisplayOption, bool> value) {
-        final filterText = <String>[];
+        display = display.copyWith(
-        value.forEach((key, value) {
+          isNotInAlbum: value[DisplayOption.notInAlbum],
-          switch (key) {
+          isArchive: value[DisplayOption.archive],
-            case DisplayOption.notInAlbum:
+          isFavorite: value[DisplayOption.favorite],
-              filter.value = filter.value.copyWith(display: filter.value.display.copyWith(isNotInAlbum: value));
+        );
-              if (value) {
-                filterText.add('search_filter_display_option_not_in_album'.t(context: context));
-              }
-              break;
-            case DisplayOption.archive:
-              filter.value = filter.value.copyWith(display: filter.value.display.copyWith(isArchive: value));
-              if (value) {
-                filterText.add('archive'.t(context: context));
-              }
-              break;
-            case DisplayOption.favorite:
-              filter.value = filter.value.copyWith(display: filter.value.display.copyWith(isFavorite: value));
-              if (value) {
-                filterText.add('favorite'.t(context: context));
-              }
-              break;
-          }
-        });
-
-        if (filterText.isEmpty) {
-          displayOptionCurrentFilterWidget.value = null;
-          return;
-        }
-
-        displayOptionCurrentFilterWidget.value = Text(filterText.join(', '), style: context.textTheme.labelLarge);
       }
 
       handleClear() {
-        filter.value = filter.value.copyWith(
-          display: SearchDisplayFilters(isNotInAlbum: false, isArchive: false, isFavorite: false),
-        );
-
         displayOptionCurrentFilterWidget.value = null;
-        search();
+        search(
+          filter.value.copyWith(
+            display: SearchDisplayFilters(isNotInAlbum: false, isArchive: false, isFavorite: false),
+          ),
+        );
+      }
+
+      handleApply() {
+        final filterText = [
+          if (display.isNotInAlbum) 'search_filter_display_option_not_in_album'.t(context: context),
+          if (display.isArchive) 'archive'.t(context: context),
+          if (display.isFavorite) 'favorite'.t(context: context),
+        ];
+        displayOptionCurrentFilterWidget.value = filterText.isNotEmpty
+            ? Text(filterText.join(', '), style: context.textTheme.labelLarge)
+            : null;
+        search(filter.value.copyWith(display: display));
       }
 
       showFilterBottomSheet(
         context: context,
         child: FilterBottomSheetScaffold(
           title: 'display_options'.t(context: context),
-          onSearch: search,
+          onSearch: handleApply,
           onClear: handleClear,
           child: DisplayOptionPicker(onSelect: handleOnSelect, filter: filter.value.display),
         ),
       );
     }
 
-    handleTextSubmitted(String value) {
+    handleTextSubmitted(String value) => search(switch (textSearchType.value) {
-      switch (textSearchType.value) {
+      TextSearchType.context => filter.value.copyWith(filename: '', context: value, description: '', ocr: ''),
-        case TextSearchType.context:
+      TextSearchType.filename => filter.value.copyWith(filename: value, context: '', description: '', ocr: ''),
-          filter.value = filter.value.copyWith(filename: '', context: value, description: '', ocr: '');
+      TextSearchType.description => filter.value.copyWith(filename: '', context: '', description: value, ocr: ''),
-
+      TextSearchType.ocr => filter.value.copyWith(filename: '', context: '', description: '', ocr: value),
-          break;
+    });
-        case TextSearchType.filename:
-          filter.value = filter.value.copyWith(filename: value, context: '', description: '', ocr: '');
-
-          break;
-        case TextSearchType.description:
-          filter.value = filter.value.copyWith(filename: '', context: '', description: value, ocr: '');
-          break;
-        case TextSearchType.ocr:
-          filter.value = filter.value.copyWith(filename: '', context: '', description: '', ocr: value);
-          break;
-      }
-
-      search();
-    }
 
     IconData getSearchPrefixIcon() => switch (textSearchType.value) {
       TextSearchType.context => Icons.image_search_rounded,
@@ -648,8 +605,10 @@ class DriftSearchPage extends HookConsumerWidget {
             hintText: searchHintText.value,
             key: const Key('search_text_field'),
             controller: textSearchController,
-            contentPadding: preFilter != null ? const EdgeInsets.only(left: 24) : const EdgeInsets.all(8),
+            contentPadding: filter.value.assetId != null ? const EdgeInsets.only(left: 24) : const EdgeInsets.all(8),
-            prefixIcon: preFilter != null ? null : Icon(getSearchPrefixIcon(), color: context.colorScheme.primary),
+            prefixIcon: filter.value.assetId != null
+                ? null
+                : Icon(getSearchPrefixIcon(), color: context.colorScheme.primary),
             onSubmitted: handleTextSubmitted,
             focusNode: ref.watch(searchInputFocusProvider),
           ),
@@ -724,7 +683,7 @@ class DriftSearchPage extends HookConsumerWidget {
               ),
             ),
           ),
-          if (!hasRequestedSearch.value)
+          if (filter.value.isEmpty)
             const _SearchSuggestions()
           else
             _SearchResultGrid(onScrollEnd: loadMoreSearchResults),

mobile/lib/widgets/search/search_filter/search_filter_chip.dart

@@ -16,7 +16,7 @@ class SearchFilterChip extends StatelessWidget {
         onTap: onTap,
         child: Card(
           elevation: 0,
-          color: context.primaryColor.withValues(alpha: .5),
+          color: context.colorScheme.secondaryContainer,
           shape: StadiumBorder(side: BorderSide(color: context.colorScheme.secondaryContainer)),
           child: Padding(
             padding: const EdgeInsets.symmetric(vertical: 2.0, horizontal: 14.0),
@@ -32,7 +32,13 @@ class SearchFilterChip extends StatelessWidget {
         shape: StadiumBorder(side: BorderSide(color: context.colorScheme.outline.withAlpha(15))),
         child: Padding(
           padding: const EdgeInsets.symmetric(vertical: 2.0, horizontal: 14.0),
-          child: Row(children: [Icon(icon, size: 18), const SizedBox(width: 4.0), Text(label)]),
+          child: Row(
+            children: [
+              Icon(icon, size: 18),
+              const SizedBox(width: 4.0),
+              Text(label, style: TextStyle(color: context.colorScheme.onSecondaryContainer)),
+            ],
+          ),
         ),
       ),
     );

mobile/pubspec.lock

@@ -1194,10 +1194,10 @@ packages:
     dependency: transitive
     description:
       name: meta
-      sha256: "23f08335362185a5ea2ad3a4e597f1375e78bce8a040df5c600c8d3552ef2394"
+      sha256: e3641ec5d63ebf0d9b41bd43201a66e3fc79a65db5f61fc181f04cd27aab950c
       url: "https://pub.dev"
     source: hosted
-    version: "1.17.0"
+    version: "1.16.0"
   mime:
     dependency: transitive
     description:
@@ -1897,10 +1897,10 @@ packages:
     dependency: transitive
     description:
       name: test_api
-      sha256: ab2726c1a94d3176a45960b6234466ec367179b87dd74f1611adb1f3b5fb9d55
+      sha256: "522f00f556e73044315fa4585ec3270f1808a4b186c936e612cab0b565ff1e00"
       url: "https://pub.dev"
     source: hosted
-    version: "0.7.7"
+    version: "0.7.6"
   thumbhash:
     dependency: "direct main"
     description: