Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
122 changes: 102 additions & 20 deletions lib/domain/controllers/drink_filter_controller.dart
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import 'package:collection/collection.dart';

import '../../models/models.dart';
import '../../utils/string_comparison_helper.dart';
import '../models/models.dart';
Expand Down Expand Up @@ -57,7 +59,7 @@ class DrinkFilterController {
List<Drink> _source = [];
List<Drink> _filtered = [];

String? _selectedCategory;
Set<String> _selectedCategories = {};
Set<String> _selectedStyles = {};
DrinkSort _currentSort = DrinkSort.nameAsc;
String _searchQuery = '';
Expand All @@ -67,7 +69,7 @@ class DrinkFilterController {

// --- Criteria getters ---

String? get selectedCategory => _selectedCategory;
Set<String> get selectedCategories => Set.unmodifiable(_selectedCategories);
Set<String> get selectedStyles => _selectedStyles;
DrinkSort get currentSort => _currentSort;
String get searchQuery => _searchQuery;
Expand All @@ -86,13 +88,11 @@ class DrinkFilterController {
List<Drink> 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<String> 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();
}

Expand All @@ -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<String, List<String>> get stylesByCategory {
final byCategory = <String, Set<String>>{};
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<String, int> get categoryCountsMap {
final counts = <String, int>{};
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;
}
Expand Down Expand Up @@ -170,7 +219,7 @@ class DrinkFilterController {
void recompute() {
final filtered = _filterService.filterDrinks(
_source,
category: _selectedCategory,
categories: _selectedCategories,
styles: _selectedStyles,
favoritesOnly: _showFavoritesOnly,
visibilityFilters: _visibilityFilters,
Expand All @@ -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)) {
Expand Down Expand Up @@ -265,7 +347,7 @@ class DrinkFilterController {
/// festivals). Sort, visibility, and allergen preferences are intentionally
/// preserved.
void clearCategoryStyleSearch() {
_selectedCategory = null;
_selectedCategories = {};
_selectedStyles = {};
_searchQuery = '';
recompute();
Expand All @@ -291,7 +373,7 @@ class DrinkFilterController {
/// applied here (see class doc).
Iterable<Drink> _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,
Expand Down
17 changes: 10 additions & 7 deletions lib/domain/services/drink_filter_service.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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<Drink> filterByCategory(Iterable<Drink> drinks, String? category) {
if (category == null) return drinks;
return drinks.where((d) => d.category == category);
Iterable<Drink> filterByCategories(
Iterable<Drink> drinks,
Set<String> categories,
) {
if (categories.isEmpty) return drinks;
return drinks.where((d) => categories.contains(d.category));
}

/// Filter drinks by styles (multi-select with OR logic)
Expand Down Expand Up @@ -138,14 +141,14 @@ class DrinkFilterService {
/// chain materialises once at the end.
List<Drink> filterDrinks(
List<Drink> drinks, {
String? category,
Set<String>? categories,
Set<String>? styles,
bool favoritesOnly = false,
Set<DrinkVisibilityFilter> visibilityFilters = const {},
Set<String> excludedAllergens = const {},
String searchQuery = '',
}) {
Iterable<Drink> result = filterByCategory(drinks, category);
Iterable<Drink> result = filterByCategories(drinks, categories ?? const {});
result = filterByStyles(result, styles ?? const {});
result = filterByFavorites(result, favoritesOnly: favoritesOnly);
result = filterByAvailability(
Expand Down
38 changes: 30 additions & 8 deletions lib/providers/beer_provider.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> get selectedCategories => _filter.selectedCategories;
Set<String> get selectedStyles => _filter.selectedStyles;
DrinkSort get currentSort => _filter.currentSort;
String get searchQuery => _filter.searchQuery;
Expand Down Expand Up @@ -152,6 +152,10 @@ class BeerProvider extends ChangeNotifier {
/// Get unique styles from loaded drinks (filtered by category if selected)
List<String> get availableStyles => _filter.availableStyles;

/// Get [availableStyles] grouped by category, for the headed style filter
/// sheet sections. See [DrinkFilterController.stylesByCategory].
Map<String, List<String>> get stylesByCategory => _filter.stylesByCategory;

/// Get drink count by category
Map<String, int> get categoryCountsMap => _filter.categoryCountsMap;

Expand Down Expand Up @@ -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);
Expand Down
30 changes: 21 additions & 9 deletions lib/screens/drinks_screen.dart
Original file line number Diff line number Diff line change
Expand Up @@ -143,20 +143,32 @@ class _DrinksScreenState extends State<DrinksScreen> {
: 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),
child: Row(
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) ...[
Expand Down Expand Up @@ -344,15 +356,15 @@ class _DrinksScreenState extends State<DrinksScreen> {
),
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,
Comment thread
richardthe3rd marked this conversation as resolved.
excludeSemantics: true,
child: OutlinedButton(
onPressed: () => provider.setCategory(null),
onPressed: () => provider.clearCategories(),
child: const Text('Clear Filters'),
),
),
Expand Down
Loading
Loading