diff --git a/lib/domain/controllers/drink_filter_controller.dart b/lib/domain/controllers/drink_filter_controller.dart index b2ff8a09..5e7f35c6 100644 --- a/lib/domain/controllers/drink_filter_controller.dart +++ b/lib/domain/controllers/drink_filter_controller.dart @@ -1,3 +1,5 @@ +import 'package:collection/collection.dart'; + import '../../models/models.dart'; import '../../utils/string_comparison_helper.dart'; import '../models/models.dart'; @@ -57,7 +59,7 @@ class DrinkFilterController { List _source = []; List _filtered = []; - String? _selectedCategory; + Set _selectedCategories = {}; Set _selectedStyles = {}; DrinkSort _currentSort = DrinkSort.nameAsc; String _searchQuery = ''; @@ -67,7 +69,7 @@ class DrinkFilterController { // --- Criteria getters --- - String? get selectedCategory => _selectedCategory; + Set get selectedCategories => Set.unmodifiable(_selectedCategories); Set get selectedStyles => _selectedStyles; DrinkSort get currentSort => _currentSort; String get searchQuery => _searchQuery; @@ -86,13 +88,11 @@ class DrinkFilterController { List get filteredDrinks => _filtered; /// Unique categories present in scope (see class doc), sorted naturally. - /// The [selectedCategory], if any, is always included even if its scoped + /// Every [selectedCategories] entry is always included even if its scoped /// count is 0 (invariant 1). List get availableCategories { - final categories = _scopeFor( - _Facet.category, - ).map((d) => d.category).toSet(); - if (_selectedCategory != null) categories.add(_selectedCategory!); + final categories = _scopeFor(_Facet.category).map((d) => d.category).toSet() + ..addAll(_selectedCategories); return categories.toList()..sort(); } @@ -112,15 +112,64 @@ class DrinkFilterController { return styles.toList()..sort(StringComparisonHelper.compareCaseInsensitive); } - /// Drink count per category, scoped per the class doc. A [selectedCategory] - /// with no matches in scope is still present, mapped to 0 (invariant 1). + /// [availableStyles] grouped by category, for presentation as headed + /// sections in the style filter sheet (issue #318 — a flat, alphabetical + /// style list mixes styles from unrelated categories, e.g. wine and perry + /// styles interleaved with beer styles). Built from the same style facet + /// scope [_scopeFor] already defines — this is not a second scoping rule, + /// just a different shape of the same scoped data. + /// + /// Keys are categories sorted naturally; values are that category's + /// styles, sorted case-insensitively via + /// [StringComparisonHelper.compareCaseInsensitive] — matching + /// [availableStyles]'s own ordering. All ordering lives here, not in the + /// UI. + /// + /// Invariant 1 still applies: a [selectedStyles] entry absent from scope + /// is still included, grouped under the category it carries in the full, + /// unfiltered [_source] (falling back to the first source drink with that + /// style, since scope has none to offer). + /// + /// Counts are deliberately not part of this view — read them from + /// [styleCountsMap], which is scoped identically (per style name, not per + /// category), so the number shown next to a style always matches what + /// ticking it actually yields. A style name occurring under two + /// categories would appear in both groups sharing that one count; this is + /// verified to be zero occurrences in current festival data, but nothing + /// here assumes it can't happen. + Map> get stylesByCategory { + final byCategory = >{}; + for (final drink in _scopeFor(_Facet.style)) { + if (drink.style == null || drink.style!.isEmpty) continue; + byCategory.putIfAbsent(drink.category, () => {}).add(drink.style!); + } + for (final style in _selectedStyles) { + if (byCategory.values.any((styles) => styles.contains(style))) { + continue; + } + final sourceDrink = _source.firstWhereOrNull((d) => d.style == style); + if (sourceDrink != null) { + byCategory.putIfAbsent(sourceDrink.category, () => {}).add(style); + } + } + final sortedCategories = byCategory.keys.toList()..sort(); + return { + for (final category in sortedCategories) + category: byCategory[category]!.toList() + ..sort(StringComparisonHelper.compareCaseInsensitive), + }; + } + + /// Drink count per category, scoped per the class doc. Every entry of + /// [selectedCategories] with no matches in scope is still present, mapped + /// to 0 (invariant 1). Map get categoryCountsMap { final counts = {}; for (final drink in _scopeFor(_Facet.category)) { counts[drink.category] = (counts[drink.category] ?? 0) + 1; } - if (_selectedCategory != null) { - counts.putIfAbsent(_selectedCategory!, () => 0); + for (final category in _selectedCategories) { + counts.putIfAbsent(category, () => 0); } return counts; } @@ -170,7 +219,7 @@ class DrinkFilterController { void recompute() { final filtered = _filterService.filterDrinks( _source, - category: _selectedCategory, + categories: _selectedCategories, styles: _selectedStyles, favoritesOnly: _showFavoritesOnly, visibilityFilters: _visibilityFilters, @@ -182,16 +231,49 @@ class DrinkFilterController { // --- Mutators (synchronous, no side effects) --- - /// Set the category filter. Clears any active style filter, since styles are - /// category-dependent. - void setCategory(String? category) { - _selectedCategory = category; - if (_selectedStyles.isNotEmpty) { - _selectedStyles = {}; + /// Toggle a single category in the multi-select category filter. + /// + /// Prunes (rather than clears) the style selection: a style survives the + /// toggle only if it is still present in the style facet's scope under the + /// *new* category selection (see [_scopeFor] / [availableStyles]). Under + /// single-select this used to be an unconditional clear, but that is too + /// destructive for multi-select — e.g. adding "perry" to an existing + /// "cider" selection would otherwise wipe a cider style the user just + /// picked, even though it's still relevant to the combined selection. + void toggleCategory(String category) { + if (_selectedCategories.contains(category)) { + _selectedCategories = Set.from(_selectedCategories)..remove(category); + } else { + _selectedCategories = Set.from(_selectedCategories)..add(category); } + _pruneStylesToScope(); + recompute(); + } + + /// Clear all selected categories. + void clearCategories() { + _selectedCategories = {}; + _pruneStylesToScope(); recompute(); } + /// Drop any selected style no longer present in the style facet's current + /// scope (i.e. under the just-changed category selection). Recomputes + /// scope directly against `_selectedStyles` rather than [availableStyles] + /// so it isn't affected by that getter's own invariant-1 re-inclusion of + /// already-selected styles. + void _pruneStylesToScope() { + if (_selectedStyles.isEmpty) return; + final scopedStyles = _scopeFor(_Facet.style) + .where((d) => d.style != null && d.style!.isNotEmpty) + .map((d) => d.style!) + .toSet(); + final pruned = _selectedStyles.where(scopedStyles.contains).toSet(); + if (pruned.length != _selectedStyles.length) { + _selectedStyles = pruned; + } + } + /// Toggle a single style in the multi-select style filter. void toggleStyle(String style) { if (_selectedStyles.contains(style)) { @@ -265,7 +347,7 @@ class DrinkFilterController { /// festivals). Sort, visibility, and allergen preferences are intentionally /// preserved. void clearCategoryStyleSearch() { - _selectedCategory = null; + _selectedCategories = {}; _selectedStyles = {}; _searchQuery = ''; recompute(); @@ -291,7 +373,7 @@ class DrinkFilterController { /// applied here (see class doc). Iterable _scopeFor(_Facet facet) => _filterService.filterDrinks( _source, - category: facet == _Facet.category ? null : _selectedCategory, + categories: facet == _Facet.category ? const {} : _selectedCategories, styles: facet == _Facet.style ? const {} : _selectedStyles, favoritesOnly: _showFavoritesOnly, visibilityFilters: _visibilityFilters, diff --git a/lib/domain/services/drink_filter_service.dart b/lib/domain/services/drink_filter_service.dart index 21f143da..1ce00ed9 100644 --- a/lib/domain/services/drink_filter_service.dart +++ b/lib/domain/services/drink_filter_service.dart @@ -10,13 +10,16 @@ class DrinkFilterService { /// Shared source of truth for which fields free-text search covers. static const SearchMatchService _searchMatcher = SearchMatchService(); - /// Filter drinks by category + /// Filter drinks by categories (multi-select with OR logic) /// - /// Returns all drinks if [category] is null + /// Returns all drinks if [categories] is empty /// Uses lazy evaluation - call .toList() to materialize - Iterable filterByCategory(Iterable drinks, String? category) { - if (category == null) return drinks; - return drinks.where((d) => d.category == category); + Iterable filterByCategories( + Iterable drinks, + Set categories, + ) { + if (categories.isEmpty) return drinks; + return drinks.where((d) => categories.contains(d.category)); } /// Filter drinks by styles (multi-select with OR logic) @@ -138,14 +141,14 @@ class DrinkFilterService { /// chain materialises once at the end. List filterDrinks( List drinks, { - String? category, + Set? categories, Set? styles, bool favoritesOnly = false, Set visibilityFilters = const {}, Set excludedAllergens = const {}, String searchQuery = '', }) { - Iterable result = filterByCategory(drinks, category); + Iterable result = filterByCategories(drinks, categories ?? const {}); result = filterByStyles(result, styles ?? const {}); result = filterByFavorites(result, favoritesOnly: favoritesOnly); result = filterByAvailability( diff --git a/lib/providers/beer_provider.dart b/lib/providers/beer_provider.dart index 57fd3879..0ab2994c 100644 --- a/lib/providers/beer_provider.dart +++ b/lib/providers/beer_provider.dart @@ -118,7 +118,7 @@ class BeerProvider extends ChangeNotifier { /// Non-null when a background refresh failed but cached drinks remain shown. String? get refreshNotice => _refreshNotice; String? get festivalsError => _festivalsError; - String? get selectedCategory => _filter.selectedCategory; + Set get selectedCategories => _filter.selectedCategories; Set get selectedStyles => _filter.selectedStyles; DrinkSort get currentSort => _filter.currentSort; String get searchQuery => _filter.searchQuery; @@ -152,6 +152,10 @@ class BeerProvider extends ChangeNotifier { /// Get unique styles from loaded drinks (filtered by category if selected) List get availableStyles => _filter.availableStyles; + /// Get [availableStyles] grouped by category, for the headed style filter + /// sheet sections. See [DrinkFilterController.stylesByCategory]. + Map> get stylesByCategory => _filter.stylesByCategory; + /// Get drink count by category Map get categoryCountsMap => _filter.categoryCountsMap; @@ -657,16 +661,34 @@ class BeerProvider extends ChangeNotifier { } } - /// Set category filter - void setCategory(String? category) { - // The controller clears the style filter when the category changes, since - // styles are category-dependent. - _filter.setCategory(category); + /// Toggle a category filter (supports multiple category selection). + /// + /// The controller prunes any selected style no longer in scope under the + /// new category selection, since styles are category-dependent. + void toggleCategory(String category) { + _filter.toggleCategory(category); notifyListeners(); - // Log analytics event (fire and forget) - unawaited(_analyticsService.logCategoryFilter(category)); + // Log analytics event (fire and forget). Canonical value: null when the + // selection is empty, otherwise the selected categories sorted and + // joined with ',' — logCategoryFilter's signature is unchanged, so this + // keeps a single, order-independent value per selection. + unawaited(_analyticsService.logCategoryFilter(_canonicalCategoryFilter)); } + /// Clear all category filters. + void clearCategories() { + _filter.clearCategories(); + notifyListeners(); + unawaited(_analyticsService.logCategoryFilter(_canonicalCategoryFilter)); + } + + /// Canonical analytics value for the current category selection: `null` + /// when empty, otherwise the selected categories sorted and joined with + /// ',' so the logged value doesn't depend on selection order. + String? get _canonicalCategoryFilter => _filter.selectedCategories.isEmpty + ? null + : (_filter.selectedCategories.toList()..sort()).join(','); + /// Toggle a style filter (supports multiple style selection) void toggleStyle(String style) { _filter.toggleStyle(style); diff --git a/lib/screens/drinks_screen.dart b/lib/screens/drinks_screen.dart index 785446f3..705fef5a 100644 --- a/lib/screens/drinks_screen.dart +++ b/lib/screens/drinks_screen.dart @@ -143,6 +143,18 @@ class _DrinksScreenState extends State { : provider.selectedStyles.length == 1 ? provider.selectedStyles.first : '${provider.selectedStyles.length} styles'; + // Formatted and sorted so the screen reader announces the same names a + // sighted user sees, in a deterministic order (a Set has none). + final formattedCategories = + provider.selectedCategories + .map(BeverageTypeHelper.formatBeverageType) + .toList() + ..sort(); + final categoryLabel = provider.selectedCategories.isEmpty + ? 'Category' + : formattedCategories.length == 1 + ? formattedCategories.first + : '${formattedCategories.length} categories'; return Container( padding: const EdgeInsets.symmetric(horizontal: 12, vertical: 6), @@ -150,13 +162,13 @@ class _DrinksScreenState extends State { children: [ Expanded( child: FilterButton( - label: provider.selectedCategory ?? 'Category', - semanticLabel: provider.selectedCategory != null - ? 'Filter by category: ${provider.selectedCategory}' - : 'Filter by category', + label: categoryLabel, + semanticLabel: formattedCategories.isEmpty + ? 'Filter by category' + : 'Filter by category: ${formattedCategories.join(', ')}', icon: Icons.filter_list, onPressed: () => showCategoryFilter(context), - isActive: provider.selectedCategory != null, + isActive: provider.selectedCategories.isNotEmpty, ), ), if (hasStyleFilter) ...[ @@ -344,15 +356,15 @@ class _DrinksScreenState extends State { ), const SizedBox(height: 8), const Text('Try adjusting your filters'), - if (provider.selectedCategory != null) ...[ + if (provider.selectedCategories.isNotEmpty) ...[ const SizedBox(height: 16), Semantics( - label: 'Clear category filter', - hint: 'Double tap to show all drinks', + label: 'Clear all category filters', + hint: 'Double tap to show every category', button: true, excludeSemantics: true, child: OutlinedButton( - onPressed: () => provider.setCategory(null), + onPressed: () => provider.clearCategories(), child: const Text('Clear Filters'), ), ), diff --git a/lib/widgets/drink_filter_sheets.dart b/lib/widgets/drink_filter_sheets.dart index 84b3dae0..a39f23af 100644 --- a/lib/widgets/drink_filter_sheets.dart +++ b/lib/widgets/drink_filter_sheets.dart @@ -5,11 +5,9 @@ import '../providers/providers.dart'; import '../utils/utils.dart'; import 'sheet_handle.dart'; -/// Shows the category filter as a modal bottom sheet. -void showCategoryFilter(BuildContext context) { - final provider = context.read(); - _showSheet(context, (_) => CategoryFilterSheet(provider: provider)); -} +/// Shows the category filter (multi-select) as a modal bottom sheet. +void showCategoryFilter(BuildContext context) => + _showSheet(context, (_) => const CategoryFilterSheet()); /// Shows the style filter (multi-select) as a modal bottom sheet. void showStyleFilter(BuildContext context) => @@ -33,84 +31,114 @@ void _showSheet(BuildContext context, WidgetBuilder builder) { ); } -/// Single-select category filter sheet. +/// Category filter sheet with checkboxes for multi-select. Categories arrive +/// already sorted naturally from [BeerProvider.availableCategories]. class CategoryFilterSheet extends StatelessWidget { - final BeerProvider provider; - - const CategoryFilterSheet({required this.provider, super.key}); + const CategoryFilterSheet({super.key}); @override Widget build(BuildContext context) { final theme = Theme.of(context); - final categories = provider.availableCategories; - final counts = provider.categoryCountsMap; - return Container( - padding: const EdgeInsets.all(16), - constraints: BoxConstraints( - maxHeight: MediaQuery.of(context).size.height * 0.7, - ), - child: Column( - mainAxisSize: MainAxisSize.min, - crossAxisAlignment: CrossAxisAlignment.start, - children: [ - const SheetHandle(), - const SizedBox(height: 16), - Text('Filter by Category', style: theme.textTheme.titleLarge), - const SizedBox(height: 16), - Flexible( - child: SingleChildScrollView( - child: RadioGroup( - groupValue: provider.selectedCategory, - onChanged: (value) { - provider.setCategory(value); - Navigator.pop(context); - }, - child: Column( - mainAxisSize: MainAxisSize.min, - children: [ + return Consumer( + builder: (context, beerProvider, child) { + final categories = beerProvider.availableCategories; + final counts = beerProvider.categoryCountsMap; + final selectedCategories = beerProvider.selectedCategories; + + return Container( + padding: const EdgeInsets.all(16), + constraints: BoxConstraints( + maxHeight: MediaQuery.of(context).size.height * 0.7, + ), + child: Column( + mainAxisSize: MainAxisSize.min, + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + const SheetHandle(), + const SizedBox(height: 16), + Row( + mainAxisAlignment: MainAxisAlignment.spaceBetween, + children: [ + Text('Filter by Category', style: theme.textTheme.titleLarge), + if (selectedCategories.isNotEmpty) Semantics( - label: - 'Show all drinks, ${provider.allDrinks.length} total', - selected: provider.selectedCategory == null, + label: 'Clear all category filters', + hint: 'Double tap to remove all category filters', button: true, excludeSemantics: true, - child: ListTile( - leading: const Radio(value: null), - title: Text('All (${provider.allDrinks.length})'), - onTap: () { - provider.setCategory(null); - Navigator.pop(context); + child: TextButton.icon( + icon: const Icon(Icons.clear, size: 18), + label: const Text('Clear'), + onPressed: () { + beerProvider.clearCategories(); }, + style: TextButton.styleFrom( + visualDensity: VisualDensity.compact, + ), ), ), - ...categories.map((category) { - final formattedCategory = - BeverageTypeHelper.formatBeverageType(category); - final count = counts[category] ?? 0; - return Semantics( - label: 'Filter by $formattedCategory, $count drinks', - selected: provider.selectedCategory == category, + ], + ), + const SizedBox(height: 16), + Flexible( + child: SingleChildScrollView( + child: Column( + mainAxisSize: MainAxisSize.min, + children: [ + Semantics( + label: + 'Show all drinks, ${beerProvider.allDrinks.length} total', + value: selectedCategories.isEmpty + ? 'Selected' + : 'Not selected', + selected: selectedCategories.isEmpty, button: true, excludeSemantics: true, - child: ListTile( - leading: Radio(value: category), - title: Text('$formattedCategory ($count)'), - onTap: () { - provider.setCategory(category); - Navigator.pop(context); - }, + child: CheckboxListTile( + key: const ValueKey('category-all'), + value: selectedCategories.isEmpty, + onChanged: (_) => beerProvider.clearCategories(), + title: Text('All (${beerProvider.allDrinks.length})'), + controlAffinity: ListTileControlAffinity.leading, + dense: true, ), - ); - }), - ], + ), + ...categories.map((category) { + final formattedCategory = + BeverageTypeHelper.formatBeverageType(category); + final count = counts[category] ?? 0; + final isSelected = selectedCategories.contains( + category, + ); + return Semantics( + label: + 'Filter by $formattedCategory, $count ' + '${count == 1 ? 'drink' : 'drinks'}', + value: isSelected ? 'Selected' : 'Not selected', + selected: isSelected, + button: true, + excludeSemantics: true, + child: CheckboxListTile( + key: ValueKey('category-$category'), + value: isSelected, + onChanged: (_) => + beerProvider.toggleCategory(category), + title: Text('$formattedCategory ($count)'), + controlAffinity: ListTileControlAffinity.leading, + dense: true, + ), + ); + }), + ], + ), ), ), - ), + const SizedBox(height: 16), + ], ), - const SizedBox(height: 16), - ], - ), + ); + }, ); } } @@ -178,8 +206,13 @@ class SortOptionsSheet extends StatelessWidget { } } -/// Style filter sheet with checkboxes for multi-select. Styles arrive already -/// sorted (case-insensitively) from [BeerProvider.availableStyles]. +/// Style filter sheet with checkboxes for multi-select. Styles arrive +/// grouped by category, already sorted, from +/// [BeerProvider.stylesByCategory] — see that getter's doc for the ordering +/// and grouping rules. A category header renders above each group, unless +/// there is exactly one group (the common case once a single category is +/// selected), in which case the list renders flat with no header — a lone +/// header is noise. class StyleFilterSheet extends StatelessWidget { const StyleFilterSheet({super.key}); @@ -189,9 +222,10 @@ class StyleFilterSheet extends StatelessWidget { return Consumer( builder: (context, beerProvider, child) { - final styles = beerProvider.availableStyles; + final stylesByCategory = beerProvider.stylesByCategory; final styleCounts = beerProvider.styleCountsMap; final selectedStyles = beerProvider.selectedStyles; + final showHeaders = stylesByCategory.length > 1; return Container( padding: const EdgeInsets.all(16), @@ -271,24 +305,52 @@ class StyleFilterSheet extends StatelessWidget { child: SingleChildScrollView( child: Column( mainAxisSize: MainAxisSize.min, - children: styles.map((style) { - final count = styleCounts[style] ?? 0; - final isSelected = selectedStyles.contains(style); - return Semantics( - label: 'Filter by $style, $count drinks', - value: isSelected ? 'Selected' : 'Not selected', - selected: isSelected, - button: true, - excludeSemantics: true, - child: CheckboxListTile( - value: isSelected, - onChanged: (_) => beerProvider.toggleStyle(style), - title: Text('$style ($count)'), - controlAffinity: ListTileControlAffinity.leading, - dense: true, - ), - ); - }).toList(), + // The category headers are the only children narrower than + // the sheet; without this they centre instead of sitting + // above their group, unlike every other Column here. + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + for (final entry in stylesByCategory.entries) ...[ + if (showHeaders) + Semantics( + header: true, + child: Padding( + padding: const EdgeInsets.symmetric( + horizontal: 16, + vertical: 4, + ), + child: Text( + BeverageTypeHelper.formatBeverageType( + entry.key, + ), + style: theme.textTheme.labelMedium?.copyWith( + color: theme.colorScheme.onSurfaceVariant, + ), + ), + ), + ), + ...entry.value.map((style) { + final count = styleCounts[style] ?? 0; + final isSelected = selectedStyles.contains(style); + return Semantics( + label: + 'Filter by $style, $count ' + '${count == 1 ? 'drink' : 'drinks'}', + value: isSelected ? 'Selected' : 'Not selected', + selected: isSelected, + button: true, + excludeSemantics: true, + child: CheckboxListTile( + value: isSelected, + onChanged: (_) => beerProvider.toggleStyle(style), + title: Text('$style ($count)'), + controlAffinity: ListTileControlAffinity.leading, + dense: true, + ), + ); + }), + ], + ], ), ), ), diff --git a/test/beer_provider_test.dart b/test/beer_provider_test.dart index a01a46cb..72ae5d94 100644 --- a/test/beer_provider_test.dart +++ b/test/beer_provider_test.dart @@ -217,7 +217,7 @@ void main() { }); group('category filter', () { - test('setCategory filters drinks by category', () async { + test('toggleCategory filters drinks by category', () async { provider = BeerProvider( drinkRepository: mockDrinkRepository, festivalRepository: mockFestivalRepository, @@ -231,13 +231,13 @@ void main() { ).thenAnswer((_) async => sampleDrinks); await provider.loadDrinks(); - provider.setCategory('beer'); + provider.toggleCategory('beer'); expect(provider.drinks.length, 2); expect(provider.drinks.every((d) => d.category == 'beer'), isTrue); }); - test('setCategory with null shows all drinks', () async { + test('toggling on two categories shows drinks from both', () async { provider = BeerProvider( drinkRepository: mockDrinkRepository, festivalRepository: mockFestivalRepository, @@ -251,14 +251,42 @@ void main() { ).thenAnswer((_) async => sampleDrinks); await provider.loadDrinks(); - provider.setCategory('beer'); + provider + ..toggleCategory('beer') + ..toggleCategory('cider'); + + expect(provider.selectedCategories, {'beer', 'cider'}); + expect(provider.drinks.length, 4); + expect( + provider.drinks.map((d) => d.name), + containsAll(['Alpha Ale', 'Beta Bitter', 'Crisp Cider']), + ); + }); + + test('clearCategories shows all drinks', () async { + provider = BeerProvider( + drinkRepository: mockDrinkRepository, + festivalRepository: mockFestivalRepository, + analyticsService: mockAnalyticsService, + ); + await provider.initialize(); + + final sampleDrinks = createSampleDrinks(); + when( + mockDrinkRepository.getDrinks(any), + ).thenAnswer((_) async => sampleDrinks); + await provider.loadDrinks(); + + provider.toggleCategory('beer'); expect(provider.drinks.length, 2); - provider.setCategory(null); + provider.clearCategories(); expect(provider.drinks.length, 4); + expect(provider.selectedCategories, isEmpty); }); - test('setCategory clears style filter', () async { + test('toggleCategory prunes a style no longer in the new category ' + 'scope', () async { provider = BeerProvider( drinkRepository: mockDrinkRepository, festivalRepository: mockFestivalRepository, @@ -275,7 +303,9 @@ void main() { provider.toggleStyle('IPA'); expect(provider.selectedStyles, contains('IPA')); - provider.setCategory('cider'); + // IPA (a beer style) is not present in cider, so restricting to + // cider prunes it from the selection. + provider.toggleCategory('cider'); expect(provider.selectedStyles, isEmpty); }); @@ -421,7 +451,7 @@ void main() { ).thenAnswer((_) async => sampleDrinks); await provider.loadDrinks(); - provider.setCategory('beer'); + provider.toggleCategory('beer'); final beerStyles = provider.availableStyles; expect(beerStyles, containsAll(['IPA', 'Bitter'])); expect(beerStyles, isNot(contains('Dry'))); @@ -1837,7 +1867,7 @@ void main() { await provider.loadDrinks(); provider - ..setCategory('beer') + ..toggleCategory('beer') ..toggleStyle('IPA'); expect(provider.drinks.length, 1); @@ -1859,7 +1889,7 @@ void main() { await provider.loadDrinks(); provider - ..setCategory('beer') + ..toggleCategory('beer') ..setSearchQuery('alpha'); expect(provider.drinks.length, 1); @@ -2895,7 +2925,7 @@ void main() { containsAll(['IPA', 'Bitter', 'Dry', 'Sweet']), ); - provider.setCategory('cider'); + provider.toggleCategory('cider'); expect(provider.styleCountsMap.keys, containsAll(['Dry', 'Sweet'])); expect(provider.styleCountsMap.containsKey('IPA'), isFalse); }); diff --git a/test/domain/controllers/drink_filter_controller_test.dart b/test/domain/controllers/drink_filter_controller_test.dart index 3995483b..fc89eadf 100644 --- a/test/domain/controllers/drink_filter_controller_test.dart +++ b/test/domain/controllers/drink_filter_controller_test.dart @@ -73,7 +73,7 @@ void main() { group('initial state', () { test('starts empty with default sort and no criteria', () { expect(controller.filteredDrinks, isEmpty); - expect(controller.selectedCategory, isNull); + expect(controller.selectedCategories, isEmpty); expect(controller.selectedStyles, isEmpty); expect(controller.currentSort, DrinkSort.nameAsc); expect(controller.searchQuery, isEmpty); @@ -118,30 +118,77 @@ void main() { test('narrows filtered drinks to the selected category', () { controller ..setSource(_sampleDrinks()) - ..setCategory('cider'); + ..toggleCategory('cider'); expect(controller.filteredDrinks.map((d) => d.name), [ 'Crisp Cider', 'Zesty Zider', ]); }); - test('setting category clears any active style filter', () { + test('toggleCategory adds then removes a category', () { controller ..setSource(_sampleDrinks()) - ..setCategory('beer') - ..toggleStyle('IPA'); + ..toggleCategory('beer'); + expect(controller.selectedCategories, {'beer'}); + expect(controller.filteredDrinks, hasLength(2)); + + controller.toggleCategory('beer'); + expect(controller.selectedCategories, isEmpty); + expect(controller.filteredDrinks, hasLength(4)); + }); + + test('multiple categories use OR logic', () { + controller + ..setSource(_sampleDrinks()) + ..toggleCategory('cider') + ..toggleCategory('beer'); + expect(controller.selectedCategories, {'cider', 'beer'}); + expect(controller.filteredDrinks, hasLength(4)); + }); + + test('clearCategories restores all categories', () { + controller + ..setSource(_sampleDrinks()) + ..toggleCategory('beer') + ..clearCategories(); + expect(controller.selectedCategories, isEmpty); + expect(controller.filteredDrinks, hasLength(4)); + }); + + test('toggling a category prunes a style that no longer matches the ' + 'new scope', () { + controller + ..setSource(_sampleDrinks()) + ..toggleCategory('beer') + ..toggleStyle('IPA'); // IPA only exists on a beer drink. expect(controller.selectedStyles, {'IPA'}); - controller.setCategory('cider'); + // Adding cider alongside beer keeps IPA in scope (beer is still + // selected) — this is the "don't wipe the whole selection" case. + controller.toggleCategory('cider'); + expect(controller.selectedStyles, {'IPA'}); + + // Removing beer leaves only cider selected; IPA no longer matches + // any drink in scope, so it is pruned. + controller.toggleCategory('beer'); expect(controller.selectedStyles, isEmpty); }); - test('null category shows all categories again', () { + test('toggling a category keeps a style that still matches the new ' + 'scope', () { controller ..setSource(_sampleDrinks()) - ..setCategory('beer') - ..setCategory(null); - expect(controller.filteredDrinks, hasLength(4)); + ..toggleCategory('cider') + ..toggleStyle('Dry'); // Dry only exists on a cider drink. + expect(controller.selectedStyles, {'Dry'}); + + // Adding beer alongside cider: Dry still matches (cider is still + // selected), so it survives the prune. The style filter still + // narrows the result to just the Dry cider, though — category and + // style filters combine with AND. + controller.toggleCategory('beer'); + expect(controller.selectedStyles, {'Dry'}); + expect(controller.filteredDrinks.map((d) => d.name), ['Crisp Cider']); }); }); @@ -383,7 +430,7 @@ void main() { 'still lists the others', () { controller ..setSource(_sampleDrinks()) - ..setCategory('beer'); + ..toggleCategory('beer'); expect(controller.availableCategories, ['beer', 'cider']); expect(controller.categoryCountsMap, {'beer': 2, 'cider': 2}); }); @@ -398,7 +445,7 @@ void main() { test('availableStyles narrows to the selected category', () { controller ..setSource(_sampleDrinks()) - ..setCategory('cider'); + ..toggleCategory('cider'); expect(controller.availableStyles, ['Dry', 'Sweet']); }); @@ -418,7 +465,7 @@ void main() { test('styleCountsMap narrows to the selected category', () { controller ..setSource(_sampleDrinks()) - ..setCategory('beer'); + ..toggleCategory('beer'); expect(controller.styleCountsMap, {'IPA': 1, 'Bitter': 1}); }); @@ -496,6 +543,53 @@ void main() { }); }); + group('stylesByCategory', () { + test('groups styles under each category, sorted, when no category is ' + 'selected', () { + controller.setSource(_sampleDrinks()); + expect(controller.stylesByCategory, { + 'beer': ['Bitter', 'IPA'], + 'cider': ['Dry', 'Sweet'], + }); + }); + + test('yields a single group when one category is selected', () { + controller + ..setSource(_sampleDrinks()) + ..toggleCategory('beer'); + expect(controller.stylesByCategory, { + 'beer': ['Bitter', 'IPA'], + }); + }); + + test('a selected style absent from scope is still grouped, under its ' + 'category in the full source', () { + controller + ..setSource(_sampleDrinks()) + ..toggleCategory('cider') + ..toggleStyle('IPA'); // IPA is a beer style; cider is selected. + expect(controller.stylesByCategory, { + 'beer': ['IPA'], + 'cider': ['Dry', 'Sweet'], + }); + }); + + test('a style name shared by two categories appears in both groups ' + '(not currently possible with real festival data, but the ' + 'grouping does not assume it can\'t happen)', () { + controller.setSource([ + _drink(id: 'a', name: 'A', category: 'beer', style: 'Dry'), + _drink(id: 'b', name: 'B', category: 'cider', style: 'Dry'), + ]); + expect(controller.stylesByCategory, { + 'beer': ['Dry'], + 'cider': ['Dry'], + }); + // The same style name shares one count, scoped by name not category. + expect(controller.styleCountsMap, {'Dry': 2}); + }); + }); + group('allergen facet scoping', () { test('availableAllergens narrows to the selected category', () { controller @@ -513,7 +607,7 @@ void main() { allergens: {'nuts': 1}, ), ]) - ..setCategory('beer'); + ..toggleCategory('beer'); expect(controller.availableAllergens, {'gluten'}); }); @@ -590,9 +684,9 @@ void main() { 'count 0', () { controller ..setSource(_sampleDrinks()) - ..setCategory('cider') + ..toggleCategory('cider') ..toggleStyle('IPA'); // IPA has no cider drinks. - expect(controller.selectedCategory, 'cider'); + expect(controller.selectedCategories, {'cider'}); expect(controller.availableCategories, containsAll(['beer', 'cider'])); expect(controller.categoryCountsMap['cider'], 0); expect(controller.categoryCountsMap['beer'], 1); @@ -602,7 +696,7 @@ void main() { 'count 0', () { controller ..setSource(_sampleDrinks()) - ..setCategory('cider') + ..toggleCategory('cider') ..toggleStyle('IPA'); // IPA has no cider drinks. expect(controller.selectedStyles, {'IPA'}); expect( @@ -636,7 +730,7 @@ void main() { ..setShowFavoritesOnly(value: true); expect(controller.categoryCountsMap, {'beer': 1}); - controller.setCategory('beer'); + controller.toggleCategory('beer'); expect(controller.filteredDrinks, hasLength(1)); expect(controller.filteredDrinks.single.name, 'Alpha Ale'); }); @@ -783,14 +877,14 @@ void main() { test('resets category, styles and search but keeps sort/visibility', () { controller ..setSource(_sampleDrinks()) - ..setCategory('beer') + ..toggleCategory('beer') ..toggleStyle('IPA') ..setSearchQuery('alpha') ..setSort(DrinkSort.nameDesc) ..setVisibilityFilter(DrinkVisibilityFilter.notTasted, active: true) ..clearCategoryStyleSearch(); - expect(controller.selectedCategory, isNull); + expect(controller.selectedCategories, isEmpty); expect(controller.selectedStyles, isEmpty); expect(controller.searchQuery, isEmpty); // Preserved diff --git a/test/domain/services/drink_filter_service_test.dart b/test/domain/services/drink_filter_service_test.dart index 88a48dd5..dd277271 100644 --- a/test/domain/services/drink_filter_service_test.dart +++ b/test/domain/services/drink_filter_service_test.dart @@ -87,20 +87,36 @@ void main() { ]; }); - group('filterByCategory', () { - test('filters drinks by category', () { - final result = service.filterByCategory(testDrinks, 'beer').toList(); + group('filterByCategories', () { + test('filters drinks by a single category', () { + final result = service.filterByCategories(testDrinks, { + 'beer', + }).toList(); expect(result, hasLength(3)); expect(result.every((d) => d.category == 'beer'), isTrue); }); - test('returns all drinks when category is null', () { - final result = service.filterByCategory(testDrinks, null).toList(); + test('filters drinks by multiple categories (OR logic)', () { + final result = service.filterByCategories(testDrinks, { + 'beer', + 'cider', + }).toList(); + expect(result, hasLength(5)); + expect( + result.every((d) => d.category == 'beer' || d.category == 'cider'), + isTrue, + ); + }); + + test('returns all drinks when categories set is empty', () { + final result = service.filterByCategories(testDrinks, {}).toList(); expect(result, hasLength(5)); }); - test('returns empty list when no drinks match category', () { - final result = service.filterByCategory(testDrinks, 'mead').toList(); + test('returns empty list when no drinks match any category', () { + final result = service.filterByCategories(testDrinks, { + 'mead', + }).toList(); expect(result, isEmpty); }); }); @@ -469,7 +485,7 @@ void main() { final result = service.filterDrinks( testDrinks, - category: 'beer', + categories: {'beer'}, styles: {'IPA'}, favoritesOnly: true, visibilityFilters: {DrinkVisibilityFilter.availableOnly}, @@ -486,15 +502,23 @@ void main() { }); test('applies only category filter', () { - final result = service.filterDrinks(testDrinks, category: 'cider'); + final result = service.filterDrinks(testDrinks, categories: {'cider'}); expect(result, hasLength(2)); expect(result.every((d) => d.category == 'cider'), isTrue); }); + test('applies multiple categories with OR logic', () { + final result = service.filterDrinks( + testDrinks, + categories: {'beer', 'cider'}, + ); + expect(result, hasLength(5)); + }); + test('applies category and style filters together', () { final result = service.filterDrinks( testDrinks, - category: 'beer', + categories: {'beer'}, styles: {'IPA'}, ); expect(result, hasLength(2)); @@ -511,7 +535,7 @@ void main() { // (only AvailabilityStatus.out is excluded). final result = service.filterDrinks( testDrinks, - category: 'beer', + categories: {'beer'}, visibilityFilters: {DrinkVisibilityFilter.availableOnly}, ); expect(result, hasLength(3)); @@ -520,7 +544,7 @@ void main() { test('returns empty list when filters exclude all drinks', () { final result = service.filterDrinks( testDrinks, - category: 'mead', // No meads in test data + categories: {'mead'}, // No meads in test data ); expect(result, isEmpty); }); diff --git a/test/drinks_screen_style_filter_test.dart b/test/drinks_screen_style_filter_test.dart index 698fbf32..9d83fb33 100644 --- a/test/drinks_screen_style_filter_test.dart +++ b/test/drinks_screen_style_filter_test.dart @@ -5,6 +5,7 @@ import 'package:cambridge_beer_festival/screens/screens.dart'; import 'package:cambridge_beer_festival/models/models.dart'; import 'package:cambridge_beer_festival/providers/providers.dart'; import 'package:cambridge_beer_festival/services/services.dart'; +import 'package:cambridge_beer_festival/widgets/widgets.dart'; import 'package:go_router/go_router.dart'; import 'package:provider/provider.dart'; import 'package:mockito/mockito.dart'; @@ -637,4 +638,175 @@ void main() { provider.dispose(); }); }); + + // The filter bar shows formatted category names ('International Beer'), so + // the screen reader must announce those same names rather than the raw + // category ids used as map keys ('international-beer'). + group('DrinksScreen category filter button label', () { + late MockDrinkRepository mockDrinkRepository; + late MockFestivalRepository mockFestivalRepository; + late MockAnalyticsService mockAnalyticsService; + late BeerProvider provider; + + const testFestival = Festival( + id: 'cbf2025', + name: 'Cambridge Beer Festival 2025', + dataBaseUrl: 'https://test.example.com/cbf2025', + ); + + Drink drinkIn(String category, String id) => Drink( + product: Product( + id: id, + name: 'Drink $id', + abv: 5, + category: category, + dispense: 'cask', + ), + producer: const Producer( + id: 'brewery1', + name: 'Test Brewery', + location: 'Cambridge', + products: [], + ), + festivalId: 'cbf2025', + ); + + setUp(() async { + SharedPreferences.setMockInitialValues({}); + mockDrinkRepository = MockDrinkRepository(); + mockFestivalRepository = MockFestivalRepository(); + mockAnalyticsService = MockAnalyticsService(); + + when(mockFestivalRepository.getFestivals()).thenAnswer( + (_) async => FestivalsResponse( + festivals: [testFestival], + defaultFestivalId: 'cbf2025', + baseUrl: 'https://example.com', + version: '1.0.0', + ), + ); + when( + mockFestivalRepository.getSelectedFestivalId(), + ).thenAnswer((_) async => null); + when(mockDrinkRepository.getDrinks(any)).thenAnswer( + (_) async => [ + drinkIn('international-beer', 'd1'), + drinkIn('cider', 'd2'), + ], + ); + + provider = BeerProvider( + drinkRepository: mockDrinkRepository, + festivalRepository: mockFestivalRepository, + analyticsService: mockAnalyticsService, + ); + await provider.initialize(); + await provider.loadDrinks(); + }); + + tearDown(() { + provider.dispose(); + }); + + // Drink cards render their own category chip, so the category name can + // appear more than once on screen. Scope assertions to the filter bar. + Finder buttonText(String text) => find.descendant( + of: find.byType(FilterButton), + matching: find.text(text), + ); + + Future pumpScreen(WidgetTester tester) async { + await tester.pumpWidget( + ChangeNotifierProvider.value( + value: provider, + child: const MaterialApp(home: DrinksScreen(festivalId: 'cbf2025')), + ), + ); + await tester.pumpAndSettle(); + } + + testWidgets('reads "Category" with no selection', (tester) async { + await pumpScreen(tester); + + expect(buttonText('Category'), findsOneWidget); + expect(find.bySemanticsLabel('Filter by category'), findsOneWidget); + }); + + testWidgets('announces the formatted name for a single selection', ( + tester, + ) async { + await pumpScreen(tester); + provider.toggleCategory('international-beer'); + await tester.pumpAndSettle(); + + expect(buttonText('International Beer'), findsOneWidget); + expect( + find.bySemanticsLabel('Filter by category: International Beer'), + findsOneWidget, + ); + // The raw id must not reach the user, visibly or audibly. + expect(buttonText('international-beer'), findsNothing); + expect( + find.bySemanticsLabel('Filter by category: international-beer'), + findsNothing, + ); + }); + + testWidgets('announces every formatted name, sorted, for a multi ' + 'selection while the visible label counts them', (tester) async { + await pumpScreen(tester); + provider + ..toggleCategory('international-beer') + ..toggleCategory('cider'); + await tester.pumpAndSettle(); + + expect(buttonText('2 categories'), findsOneWidget); + expect( + find.bySemanticsLabel('Filter by category: Cider, International Beer'), + findsOneWidget, + ); + }); + + testWidgets('returns to "Category" once the selection is cleared', ( + tester, + ) async { + await pumpScreen(tester); + provider.toggleCategory('cider'); + await tester.pumpAndSettle(); + expect(buttonText('Cider'), findsOneWidget); + + provider.clearCategories(); + await tester.pumpAndSettle(); + + expect(buttonText('Category'), findsOneWidget); + expect(find.bySemanticsLabel('Filter by category'), findsOneWidget); + }); + + // The empty-state button is the only escape hatch from a filter + // combination that matches nothing without reopening the sheet, so it + // has to actually clear the selection. + testWidgets('empty-state Clear Filters button clears the category ' + 'selection', (tester) async { + await pumpScreen(tester); + provider + ..toggleCategory('cider') + ..setSearchQuery('nothing matches this'); + await tester.pumpAndSettle(); + + expect(find.text('No drinks found'), findsOneWidget); + expect(find.text('Clear Filters'), findsOneWidget); + // The label has to describe clearing every category, not just one. + expect( + find.bySemanticsLabel('Clear all category filters'), + findsOneWidget, + ); + + await tester.tap(find.text('Clear Filters')); + await tester.pumpAndSettle(); + + expect(provider.selectedCategories, isEmpty); + expect(find.text('Clear Filters'), findsNothing); + expect(buttonText('Category'), findsOneWidget); + }); + }); } diff --git a/test/provider_test.dart b/test/provider_test.dart index 35635fbd..6dce0f31 100644 --- a/test/provider_test.dart +++ b/test/provider_test.dart @@ -540,7 +540,8 @@ void main() { verify(mockAnalyticsService.logFestivalSelected(testFestival)).called(1); }); - test('logs category filter event when category changes', () async { + test('logs category filter event with the canonical joined value when ' + 'multiple categories are selected', () async { final provider = BeerProvider( drinkRepository: mockDrinkRepository, festivalRepository: mockFestivalRepository, @@ -548,11 +549,33 @@ void main() { ); await provider.initialize(); - provider.setCategory('beer'); + provider.toggleCategory('cider'); + verify(mockAnalyticsService.logCategoryFilter('cider')).called(1); - verify(mockAnalyticsService.logCategoryFilter('beer')).called(1); + // Sorted, not selection order, so the logged value is independent of + // which category was toggled first. + provider.toggleCategory('beer'); + verify(mockAnalyticsService.logCategoryFilter('beer,cider')).called(1); }); + test( + 'logs category filter event with null when categories are cleared', + () async { + final provider = BeerProvider( + drinkRepository: mockDrinkRepository, + festivalRepository: mockFestivalRepository, + analyticsService: mockAnalyticsService, + ); + await provider.initialize(); + + provider + ..toggleCategory('beer') + ..clearCategories(); + + verify(mockAnalyticsService.logCategoryFilter(null)).called(1); + }, + ); + test('logs style filter event when style toggles', () async { final provider = BeerProvider( drinkRepository: mockDrinkRepository, diff --git a/test/widgets/drink_filter_sheets_test.dart b/test/widgets/drink_filter_sheets_test.dart index cdba1cef..66da9510 100644 --- a/test/widgets/drink_filter_sheets_test.dart +++ b/test/widgets/drink_filter_sheets_test.dart @@ -20,6 +20,7 @@ void main() { String id, String name, String style, { + String category = 'beer', Map allergens = const {}, bool? isVegan, }) => Drink( @@ -27,7 +28,7 @@ void main() { id: id, name: name, abv: 5.0, - category: 'beer', + category: category, dispense: 'cask', style: style, allergens: allergens, @@ -74,6 +75,7 @@ void main() { isVegan: true, ), beer('d2', 'Alpha Bitter', 'Bitter'), + beer('d3', 'Crisp Cider', 'Dry', category: 'cider'), ], ); @@ -135,16 +137,45 @@ void main() { testWidgets('CategoryFilterSheet lists categories with counts', ( tester, ) async { - await tester.pumpWidget( - directHost(CategoryFilterSheet(provider: provider)), - ); + await tester.pumpWidget(directHost(const CategoryFilterSheet())); await tester.pumpAndSettle(); expect(find.text('Filter by Category'), findsOneWidget); - expect(find.text('All (2)'), findsOneWidget); - expect(find.textContaining('Beer'), findsOneWidget); + expect(find.text('All (3)'), findsOneWidget); + expect(find.text('Beer (2)'), findsOneWidget); + expect(find.text('Cider (1)'), findsOneWidget); }); + testWidgets( + 'CategoryFilterSheet rows expose semantics label/value/selected', + (tester) async { + provider.toggleCategory('beer'); + await tester.pumpWidget(directHost(const CategoryFilterSheet())); + await tester.pumpAndSettle(); + + expect( + find.byWidgetPredicate( + (widget) => + widget is Semantics && + widget.properties.label == 'Filter by Beer, 2 drinks' && + widget.properties.value == 'Selected' && + widget.properties.selected == true, + ), + findsOneWidget, + ); + expect( + find.byWidgetPredicate( + (widget) => + widget is Semantics && + widget.properties.label == 'Filter by Cider, 1 drink' && + widget.properties.value == 'Not selected' && + widget.properties.selected == false, + ), + findsOneWidget, + ); + }, + ); + testWidgets('SortOptionsSheet lists every sort option label', ( tester, ) async { @@ -171,11 +202,99 @@ void main() { final ipaY = tester.getTopLeft(find.text('IPA (1)')).dy; expect(bitterY, lessThan(ipaY)); }); + + testWidgets('StyleFilterSheet style rows announce a singular drink ' + 'count grammatically', (tester) async { + await tester.pumpWidget(directHost(const StyleFilterSheet())); + await tester.pumpAndSettle(); + + expect( + find.byWidgetPredicate( + (widget) => + widget is Semantics && + widget.properties.label == 'Filter by IPA, 1 drink' && + widget.properties.value == 'Not selected', + ), + findsOneWidget, + ); + // The ungrammatical form must not reach a screen reader. + expect( + find.byWidgetPredicate( + (widget) => + widget is Semantics && + widget.properties.label == 'Filter by IPA, 1 drinks', + ), + findsNothing, + ); + }); + + testWidgets( + 'StyleFilterSheet renders a category header per group when no ' + 'category is selected', + (tester) async { + // No category selected: 2 categories are in scope (beer, cider), + // so headers render. + await tester.pumpWidget(directHost(const StyleFilterSheet())); + await tester.pumpAndSettle(); + + expect(find.text('Beer'), findsOneWidget); + expect(find.text('Cider'), findsOneWidget); + // The Cider header sits above the Dry row (its only style). + final headerY = tester.getTopLeft(find.text('Cider')).dy; + final dryY = tester.getTopLeft(find.text('Dry (1)')).dy; + expect(headerY, lessThan(dryY)); + + // A header is the only child narrower than the sheet, so it centres + // unless the enclosing Column aligns to the start. Pin it to the + // left: it must start well left of the middle, near its own rows. + final sheetWidth = tester + .getSize(find.byType(StyleFilterSheet)) + .width; + final headerX = tester.getTopLeft(find.text('Cider')).dx; + expect( + headerX, + lessThan(sheetWidth / 4), + reason: 'category header should be left-aligned, not centred', + ); + }, + ); + + testWidgets('StyleFilterSheet renders flat with no header when only one ' + 'category is in scope', (tester) async { + provider.toggleCategory('beer'); + await tester.pumpWidget(directHost(const StyleFilterSheet())); + await tester.pumpAndSettle(); + + expect(find.text('Beer'), findsNothing); + expect(find.text('Bitter (1)'), findsOneWidget); + expect(find.text('IPA (1)'), findsOneWidget); + }); + + testWidgets( + 'StyleFilterSheet category headers are exposed as semantics headers', + (tester) async { + final handle = tester.ensureSemantics(); + try { + await tester.pumpWidget(directHost(const StyleFilterSheet())); + await tester.pumpAndSettle(); + + final headerNode = tester.getSemantics(find.text('Cider')); + expect( + headerNode.flagsCollection.isHeader, + isTrue, + reason: 'category header should carry the isHeader flag', + ); + } finally { + handle.dispose(); + } + }, + ); }); group('via show* helpers', () { testWidgets( - 'showCategoryFilter: selecting a category sets it and closes', + 'showCategoryFilter: toggling a category selects it without closing ' + 'the sheet', (tester) async { await tester.pumpWidget(launcherHost()); await tester.tap(find.text('open-category')); @@ -184,23 +303,64 @@ void main() { await tester.tap(find.text('Beer (2)')); await tester.pumpAndSettle(); - expect(provider.selectedCategory, 'beer'); - expect(find.text('Filter by Category'), findsNothing); + expect(provider.selectedCategories, {'beer'}); + // Unlike the old single-select radio sheet, the sheet stays open + // so more categories can be toggled. + expect(find.text('Filter by Category'), findsOneWidget); }, ); - testWidgets('showCategoryFilter: selecting All clears the category', ( + testWidgets('showCategoryFilter: toggling two categories reflects both ' + 'selections', (tester) async { + await tester.pumpWidget(launcherHost()); + await tester.tap(find.text('open-category')); + await tester.pumpAndSettle(); + + await tester.tap(find.text('Beer (2)')); + await tester.pumpAndSettle(); + await tester.tap(find.text('Cider (1)')); + await tester.pumpAndSettle(); + + expect(provider.selectedCategories, {'beer', 'cider'}); + expect(find.text('Filter by Category'), findsOneWidget); + + final beerCheckbox = tester.widget( + find.widgetWithText(CheckboxListTile, 'Beer (2)'), + ); + final ciderCheckbox = tester.widget( + find.widgetWithText(CheckboxListTile, 'Cider (1)'), + ); + expect(beerCheckbox.value, isTrue); + expect(ciderCheckbox.value, isTrue); + }); + + testWidgets('showCategoryFilter: selecting All clears the categories', ( tester, ) async { - provider.setCategory('beer'); + provider.toggleCategory('beer'); await tester.pumpWidget(launcherHost()); await tester.tap(find.text('open-category')); await tester.pumpAndSettle(); - await tester.tap(find.text('All (2)')); + await tester.tap(find.text('All (3)')); + await tester.pumpAndSettle(); + + expect(provider.selectedCategories, isEmpty); + }); + + testWidgets('showCategoryFilter: Clear button removes all selected ' + 'categories', (tester) async { + provider + ..toggleCategory('beer') + ..toggleCategory('cider'); + await tester.pumpWidget(launcherHost()); + await tester.tap(find.text('open-category')); + await tester.pumpAndSettle(); + + await tester.tap(find.widgetWithText(TextButton, 'Clear')); await tester.pumpAndSettle(); - expect(provider.selectedCategory, isNull); + expect(provider.selectedCategories, isEmpty); }); testWidgets('showSortOptions: selecting a sort applies it and closes', (