diff --git a/lib/models/festival.dart b/lib/models/festival.dart index 5228dbca..a57d019e 100644 --- a/lib/models/festival.dart +++ b/lib/models/festival.dart @@ -1,3 +1,5 @@ +import 'package:intl/intl.dart'; + /// Status of a festival based on dates enum FestivalStatus { /// Festival is currently running (between start and end dates) @@ -115,37 +117,45 @@ class Festival { } /// Format the festival dates for display + /// + /// A single day reads `May 18, 2026`; a range inside one month collapses to + /// `May 18-23, 2026`; a range crossing a month boundary names both months as + /// `Dec 30 - Jan 2, 2026`. String get formattedDates { if (startDate == null) return ''; final start = startDate!; final end = endDate; - final months = [ - 'Jan', - 'Feb', - 'Mar', - 'Apr', - 'May', - 'Jun', - 'Jul', - 'Aug', - 'Sep', - 'Oct', - 'Nov', - 'Dec', - ]; + final dayMonth = DateFormat('MMM d'); + final dayMonthYear = DateFormat('MMM d, y'); if (end == null) { - return '${months[start.month - 1]} ${start.day}, ${start.year}'; + return dayMonthYear.format(start); } if (start.month == end.month && start.year == end.year) { - return '${months[start.month - 1]} ${start.day}-${end.day}, ${start.year}'; + return '${dayMonth.format(start)}-${end.day}, ${start.year}'; } - return '${months[start.month - 1]} ${start.day} - ${months[end.month - 1]} ${end.day}, ${start.year}'; + // The end date carries its own year so a festival spanning New Year does + // not report both ends under the start year. + return '${dayMonth.format(start)} - ${dayMonthYear.format(end)}'; + } + + /// Festivals are identified by [id] — a festival read from cache and the same + /// festival read from the network are the same festival. + /// + /// An empty [id] falls back to identity, matching [Producer] and [Product]: + /// an unidentifiable festival must not collapse into every other one. + @override + bool operator ==(Object other) { + if (id.isEmpty) return identical(this, other); + return other is Festival && other.id == id; } + @override + int get hashCode => id.isEmpty ? identityHashCode(this) : id.hashCode; + /// Check if the festival is currently live (between start and end dates) bool isLive([DateTime? now]) { if (startDate == null) return false; diff --git a/lib/screens/my_festival_screen.dart b/lib/screens/my_festival_screen.dart index 48ad39e2..28198957 100644 --- a/lib/screens/my_festival_screen.dart +++ b/lib/screens/my_festival_screen.dart @@ -220,7 +220,7 @@ class _MyFestivalScreenState extends State { Widget _buildSectionHeader(BuildContext context, String title, int count) { return Semantics( header: true, - label: '$title section, $count ${count == 1 ? 'drink' : 'drinks'}', + label: '$title section, ${StringFormattingHelper.drinkCountLabel(count)}', child: Padding( padding: const EdgeInsets.fromLTRB(16, 16, 16, 8), // Use the theme's titleLarge (the app's Playfair "poster" voice) rather diff --git a/lib/utils/string_formatting_helper.dart b/lib/utils/string_formatting_helper.dart index ecbdbbb2..60a90cd0 100644 --- a/lib/utils/string_formatting_helper.dart +++ b/lib/utils/string_formatting_helper.dart @@ -14,4 +14,14 @@ class StringFormattingHelper { if (text.isEmpty) return text; return text[0].toUpperCase() + text.substring(1); } + + /// Format a drink count with the correctly pluralised noun. + /// + /// Used in screen-reader labels, where an inlined `count == 1 ? ... : ...` + /// ternary has twice been forgotten and announced '1 drinks' (#506, #513). + /// + /// Example: 0 -> '0 drinks', 1 -> '1 drink', 2 -> '2 drinks' + static String drinkCountLabel(int count) { + return '$count ${count == 1 ? 'drink' : 'drinks'}'; + } } diff --git a/lib/widgets/drink_card.dart b/lib/widgets/drink_card.dart index eaa0859d..954e96ef 100644 --- a/lib/widgets/drink_card.dart +++ b/lib/widgets/drink_card.dart @@ -159,26 +159,18 @@ class DrinkCard extends StatelessWidget { buffer.write(', ${drink.breweryLocation}'); } if (drink.availabilityStatus != null) { - switch (drink.availabilityStatus!) { - case AvailabilityStatus.plenty: - buffer.write(', Available'); - break; - case AvailabilityStatus.good: - buffer.write(', Some remaining'); - break; - case AvailabilityStatus.low: - buffer.write(', Low availability'); - break; - case AvailabilityStatus.veryLow: - buffer.write(', Very low availability'); - break; - case AvailabilityStatus.out: - buffer.write(', Sold out'); - break; - case AvailabilityStatus.unknown: - buffer.write(', ${drink.statusText ?? 'Unknown availability'}'); - break; - } + // Switch *expression*, deliberately without a wildcard arm: a new + // AvailabilityStatus value must break the build here rather than + // silently drop availability from the screen-reader label (#534). + buffer.write(switch (drink.availabilityStatus!) { + AvailabilityStatus.plenty => ', Available', + AvailabilityStatus.good => ', Some remaining', + AvailabilityStatus.low => ', Low availability', + AvailabilityStatus.veryLow => ', Very low availability', + AvailabilityStatus.out => ', Sold out', + AvailabilityStatus.unknown => + ', ${drink.statusText ?? 'Unknown availability'}', + }); } if (drink.rating != null) { buffer.write(', Rated ${drink.rating} out of 5 stars'); @@ -274,35 +266,15 @@ class _AvailabilityChip extends StatelessWidget { status, theme.colorScheme, ); - String label; - IconData icon; - - switch (status) { - case AvailabilityStatus.plenty: - label = 'Available'; - icon = Icons.check_circle; - break; - case AvailabilityStatus.good: - label = 'Some Left'; - icon = Icons.check_circle_outline; - break; - case AvailabilityStatus.low: - label = 'Low'; - icon = Icons.warning; - break; - case AvailabilityStatus.veryLow: - label = 'Nearly Gone'; - icon = Icons.warning_amber; - break; - case AvailabilityStatus.out: - label = 'Sold Out'; - icon = Icons.cancel; - break; - case AvailabilityStatus.unknown: - label = rawText ?? 'Unknown'; - icon = Icons.info_outline; - break; - } + // Switch *expression*, deliberately without a wildcard arm — see #534. + final (label, icon) = switch (status) { + AvailabilityStatus.plenty => ('Available', Icons.check_circle), + AvailabilityStatus.good => ('Some Left', Icons.check_circle_outline), + AvailabilityStatus.low => ('Low', Icons.warning), + AvailabilityStatus.veryLow => ('Nearly Gone', Icons.warning_amber), + AvailabilityStatus.out => ('Sold Out', Icons.cancel), + AvailabilityStatus.unknown => (rawText ?? 'Unknown', Icons.info_outline), + }; return Container( padding: const EdgeInsets.symmetric(horizontal: 8, vertical: 4), diff --git a/lib/widgets/drink_filter_sheets.dart b/lib/widgets/drink_filter_sheets.dart index bdd079a6..3c783fc8 100644 --- a/lib/widgets/drink_filter_sheets.dart +++ b/lib/widgets/drink_filter_sheets.dart @@ -49,7 +49,7 @@ class CategoryFilterSheet extends StatelessWidget { return Container( padding: const EdgeInsets.all(16), constraints: BoxConstraints( - maxHeight: MediaQuery.of(context).size.height * 0.7, + maxHeight: MediaQuery.sizeOf(context).height * 0.7, ), child: Column( mainAxisSize: MainAxisSize.min, @@ -113,8 +113,8 @@ class CategoryFilterSheet extends StatelessWidget { ); return Semantics( label: - 'Filter by $formattedCategory, $count ' - '${count == 1 ? 'drink' : 'drinks'}', + 'Filter by $formattedCategory, ' + '${StringFormattingHelper.drinkCountLabel(count)}', value: isSelected ? 'Selected' : 'Not selected', selected: isSelected, button: true, @@ -156,7 +156,7 @@ class SortOptionsSheet extends StatelessWidget { return Container( padding: const EdgeInsets.all(16), constraints: BoxConstraints( - maxHeight: MediaQuery.of(context).size.height * 0.7, + maxHeight: MediaQuery.sizeOf(context).height * 0.7, ), child: Column( mainAxisSize: MainAxisSize.min, @@ -233,7 +233,7 @@ class StyleFilterSheet extends StatelessWidget { return Container( padding: const EdgeInsets.all(16), constraints: BoxConstraints( - maxHeight: MediaQuery.of(context).size.height * 0.7, + maxHeight: MediaQuery.sizeOf(context).height * 0.7, ), child: Column( mainAxisSize: MainAxisSize.min, @@ -337,8 +337,8 @@ class StyleFilterSheet extends StatelessWidget { final isSelected = selectedStyles.contains(style); return Semantics( label: - 'Filter by $style, $count ' - '${count == 1 ? 'drink' : 'drinks'}', + 'Filter by $style, ' + '${StringFormattingHelper.drinkCountLabel(count)}', value: isSelected ? 'Selected' : 'Not selected', selected: isSelected, button: true, @@ -382,7 +382,7 @@ class VisibilityFilterSheet extends StatelessWidget { return Container( padding: const EdgeInsets.all(16), constraints: BoxConstraints( - maxHeight: MediaQuery.of(context).size.height * 0.7, + maxHeight: MediaQuery.sizeOf(context).height * 0.7, ), child: Column( mainAxisSize: MainAxisSize.min, diff --git a/lib/widgets/festival_header.dart b/lib/widgets/festival_header.dart index 70b481ac..44de4a4d 100644 --- a/lib/widgets/festival_header.dart +++ b/lib/widgets/festival_header.dart @@ -1,6 +1,7 @@ import 'package:flutter/material.dart'; import '../models/models.dart'; import '../providers/providers.dart'; +import '../utils/utils.dart'; /// App-bar title for the drinks screen: app icon, current festival name, the /// drink count, and a coloured status badge. @@ -17,8 +18,7 @@ class FestivalHeader extends StatelessWidget { provider.sortedFestivals, ); final drinkCount = provider.drinks.length; - final drinkCountLabel = - '$drinkCount ${drinkCount == 1 ? 'drink' : 'drinks'}'; + final drinkCountLabel = StringFormattingHelper.drinkCountLabel(drinkCount); // Fold the status into the label and exclude child semantics so screen // readers announce one coherent phrase instead of the name, count, and diff --git a/lib/widgets/festival_menu_sheets.dart b/lib/widgets/festival_menu_sheets.dart index 4418913b..bfe94a08 100644 --- a/lib/widgets/festival_menu_sheets.dart +++ b/lib/widgets/festival_menu_sheets.dart @@ -85,7 +85,7 @@ class FestivalSelectorSheet extends StatelessWidget { return Container( padding: const EdgeInsets.all(16), constraints: BoxConstraints( - maxHeight: MediaQuery.of(context).size.height * 0.7, + maxHeight: MediaQuery.sizeOf(context).height * 0.7, ), child: Column( mainAxisSize: MainAxisSize.min, diff --git a/test/models_test.dart b/test/models_test.dart index 0a58c7d0..b1a55476 100644 --- a/test/models_test.dart +++ b/test/models_test.dart @@ -1424,6 +1424,61 @@ void main() { expect(festival.formattedDates, contains(months[i])); } }); + + test('carries both years for a range spanning New Year', () { + final festival = Festival( + id: 'cbfw2025', + name: 'Cambridge Winter Beer Festival', + startDate: DateTime(2025, 12, 30), + endDate: DateTime(2026, 1, 2), + dataBaseUrl: 'https://example.com/cbfw2025', + ); + + expect(festival.formattedDates, 'Dec 30 - Jan 2, 2026'); + }); + }); + + group('equality', () { + const json = { + 'id': 'cbf2025', + 'name': 'Cambridge Beer Festival 2025', + 'data_base_url': 'https://example.com/cbf2025', + }; + + test('two instances parsed from the same JSON are equal', () { + final a = Festival.fromJson(Map.from(json)); + final b = Festival.fromJson(Map.from(json)); + + expect(a, equals(b)); + expect(a.hashCode, equals(b.hashCode)); + }); + + test('a Set de-duplicates instances with the same id', () { + final cached = Festival.fromJson(Map.from(json)); + final fromNetwork = Festival.fromJson(Map.from(json)); + + expect({cached, fromNetwork}, hasLength(1)); + }); + + test('festivals with different ids are not equal', () { + final a = Festival.fromJson(Map.from(json)); + final b = Festival.fromJson({...json, 'id': 'cbf2026'}); + + expect(a, isNot(equals(b))); + }); + + test('an empty id falls back to identity', () { + // Built via fromJson so the two instances are distinct objects — a + // const literal pair would be canonicalised to the same instance and + // could not distinguish identity equality from id equality. + final emptyIdJson = {...json, 'id': ''}; + final a = Festival.fromJson(Map.from(emptyIdJson)); + final b = Festival.fromJson(Map.from(emptyIdJson)); + + expect(a, equals(a)); + expect(a, isNot(equals(b))); + expect({a, b}, hasLength(2)); + }); }); group('fromJson', () { diff --git a/test/string_formatting_helper_test.dart b/test/string_formatting_helper_test.dart index 3897759e..b1bb1658 100644 --- a/test/string_formatting_helper_test.dart +++ b/test/string_formatting_helper_test.dart @@ -28,5 +28,20 @@ void main() { expect(StringFormattingHelper.capitalizeFirst('a'), 'A'); }); }); + + group('drinkCountLabel', () { + test('pluralises zero', () { + expect(StringFormattingHelper.drinkCountLabel(0), '0 drinks'); + }); + + test('uses the singular for exactly one', () { + expect(StringFormattingHelper.drinkCountLabel(1), '1 drink'); + }); + + test('pluralises counts above one', () { + expect(StringFormattingHelper.drinkCountLabel(2), '2 drinks'); + expect(StringFormattingHelper.drinkCountLabel(147), '147 drinks'); + }); + }); }); }