Skip to content

Commit 93391b3

Browse files
refactor: clear five low-priority backlog issues (#539)
* refactor(models): format festival dates with intl and give Festival value equality formattedDates carried its own month-name table alongside three range branches, while intl was already a dependency used by three screens. DateFormat replaces the table; output is byte-identical for every existing case. The cross-month branch printed the start year on both ends, so a festival spanning New Year reported the wrong year on its end date. The end date now carries its own year. Festival also had no == or hashCode while Drink, Product and Producer all implement identity-by-id, so two instances of the same festival parsed from cache and from the network compared unequal in any collection operation. Adds id-based equality with the same empty-id identity fallback Producer uses. Fixes #532 Fixes #529 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ER7MeRfgMqxnaKBGDaSRWy * refactor(widgets): make AvailabilityStatus switches exhaustiveness-checked Both switches over AvailabilityStatus in drink_card.dart were C-style switch statements, which are not exhaustiveness-checked. The semantic label switch had no safety net at all: a seventh enum value would fall through and silently omit availability from the screen-reader label. #349 established that the festival status_text vocabulary is not stable across festivals, so a new status value is a realistic event. Both are now switch expressions with no wildcard arm, so a new enum value is a compile error. Verified by adding a seventh value and confirming non_exhaustive_switch_expression fires at both sites. Output is unchanged. Fixes #534 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ER7MeRfgMqxnaKBGDaSRWy * refactor(ui): extract a shared drink-count pluralisation helper The 'count == 1 ? drink : drinks' ternary was inlined at four call sites, past the extract-a-helper threshold in AGENTS.md. The failure mode is silent and accessibility-facing: a new label that forgets the ternary announces "1 drinks" to a screen reader and nothing fails. Two of the four sites had exactly that bug until review caught it in #506. Adds StringFormattingHelper.drinkCountLabel and routes all four sites through it. Labels are unchanged. Fixes #513 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ER7MeRfgMqxnaKBGDaSRWy * refactor(widgets): read sheet height with MediaQuery.sizeOf Five bottom-sheet builders capped their height with MediaQuery.of(context).size.height, which subscribes the sheet to every MediaQuery change — keyboard insets, text scale, brightness, padding — not just size. For a bottom sheet the keyboard case is the one that actually fires. MediaQuery.sizeOf establishes a dependency on size alone. The value read is identical, so there is no behaviour change. Fixes #531 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ER7MeRfgMqxnaKBGDaSRWy --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 801acec commit 93391b3

9 files changed

Lines changed: 140 additions & 78 deletions

lib/models/festival.dart

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import 'package:intl/intl.dart';
2+
13
/// Status of a festival based on dates
24
enum FestivalStatus {
35
/// Festival is currently running (between start and end dates)
@@ -115,37 +117,45 @@ class Festival {
115117
}
116118

117119
/// Format the festival dates for display
120+
///
121+
/// A single day reads `May 18, 2026`; a range inside one month collapses to
122+
/// `May 18-23, 2026`; a range crossing a month boundary names both months as
123+
/// `Dec 30 - Jan 2, 2026`.
118124
String get formattedDates {
119125
if (startDate == null) return '';
120126
final start = startDate!;
121127
final end = endDate;
122128

123-
final months = [
124-
'Jan',
125-
'Feb',
126-
'Mar',
127-
'Apr',
128-
'May',
129-
'Jun',
130-
'Jul',
131-
'Aug',
132-
'Sep',
133-
'Oct',
134-
'Nov',
135-
'Dec',
136-
];
129+
final dayMonth = DateFormat('MMM d');
130+
final dayMonthYear = DateFormat('MMM d, y');
137131

138132
if (end == null) {
139-
return '${months[start.month - 1]} ${start.day}, ${start.year}';
133+
return dayMonthYear.format(start);
140134
}
141135

142136
if (start.month == end.month && start.year == end.year) {
143-
return '${months[start.month - 1]} ${start.day}-${end.day}, ${start.year}';
137+
return '${dayMonth.format(start)}-${end.day}, ${start.year}';
144138
}
145139

146-
return '${months[start.month - 1]} ${start.day} - ${months[end.month - 1]} ${end.day}, ${start.year}';
140+
// The end date carries its own year so a festival spanning New Year does
141+
// not report both ends under the start year.
142+
return '${dayMonth.format(start)} - ${dayMonthYear.format(end)}';
143+
}
144+
145+
/// Festivals are identified by [id] — a festival read from cache and the same
146+
/// festival read from the network are the same festival.
147+
///
148+
/// An empty [id] falls back to identity, matching [Producer] and [Product]:
149+
/// an unidentifiable festival must not collapse into every other one.
150+
@override
151+
bool operator ==(Object other) {
152+
if (id.isEmpty) return identical(this, other);
153+
return other is Festival && other.id == id;
147154
}
148155

156+
@override
157+
int get hashCode => id.isEmpty ? identityHashCode(this) : id.hashCode;
158+
149159
/// Check if the festival is currently live (between start and end dates)
150160
bool isLive([DateTime? now]) {
151161
if (startDate == null) return false;

lib/screens/my_festival_screen.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ class _MyFestivalScreenState extends State<MyFestivalScreen> {
220220
Widget _buildSectionHeader(BuildContext context, String title, int count) {
221221
return Semantics(
222222
header: true,
223-
label: '$title section, $count ${count == 1 ? 'drink' : 'drinks'}',
223+
label: '$title section, ${StringFormattingHelper.drinkCountLabel(count)}',
224224
child: Padding(
225225
padding: const EdgeInsets.fromLTRB(16, 16, 16, 8),
226226
// Use the theme's titleLarge (the app's Playfair "poster" voice) rather

lib/utils/string_formatting_helper.dart

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,4 +14,14 @@ class StringFormattingHelper {
1414
if (text.isEmpty) return text;
1515
return text[0].toUpperCase() + text.substring(1);
1616
}
17+
18+
/// Format a drink count with the correctly pluralised noun.
19+
///
20+
/// Used in screen-reader labels, where an inlined `count == 1 ? ... : ...`
21+
/// ternary has twice been forgotten and announced '1 drinks' (#506, #513).
22+
///
23+
/// Example: 0 -> '0 drinks', 1 -> '1 drink', 2 -> '2 drinks'
24+
static String drinkCountLabel(int count) {
25+
return '$count ${count == 1 ? 'drink' : 'drinks'}';
26+
}
1727
}

lib/widgets/drink_card.dart

Lines changed: 21 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -159,26 +159,18 @@ class DrinkCard extends StatelessWidget {
159159
buffer.write(', ${drink.breweryLocation}');
160160
}
161161
if (drink.availabilityStatus != null) {
162-
switch (drink.availabilityStatus!) {
163-
case AvailabilityStatus.plenty:
164-
buffer.write(', Available');
165-
break;
166-
case AvailabilityStatus.good:
167-
buffer.write(', Some remaining');
168-
break;
169-
case AvailabilityStatus.low:
170-
buffer.write(', Low availability');
171-
break;
172-
case AvailabilityStatus.veryLow:
173-
buffer.write(', Very low availability');
174-
break;
175-
case AvailabilityStatus.out:
176-
buffer.write(', Sold out');
177-
break;
178-
case AvailabilityStatus.unknown:
179-
buffer.write(', ${drink.statusText ?? 'Unknown availability'}');
180-
break;
181-
}
162+
// Switch *expression*, deliberately without a wildcard arm: a new
163+
// AvailabilityStatus value must break the build here rather than
164+
// silently drop availability from the screen-reader label (#534).
165+
buffer.write(switch (drink.availabilityStatus!) {
166+
AvailabilityStatus.plenty => ', Available',
167+
AvailabilityStatus.good => ', Some remaining',
168+
AvailabilityStatus.low => ', Low availability',
169+
AvailabilityStatus.veryLow => ', Very low availability',
170+
AvailabilityStatus.out => ', Sold out',
171+
AvailabilityStatus.unknown =>
172+
', ${drink.statusText ?? 'Unknown availability'}',
173+
});
182174
}
183175
if (drink.rating != null) {
184176
buffer.write(', Rated ${drink.rating} out of 5 stars');
@@ -274,35 +266,15 @@ class _AvailabilityChip extends StatelessWidget {
274266
status,
275267
theme.colorScheme,
276268
);
277-
String label;
278-
IconData icon;
279-
280-
switch (status) {
281-
case AvailabilityStatus.plenty:
282-
label = 'Available';
283-
icon = Icons.check_circle;
284-
break;
285-
case AvailabilityStatus.good:
286-
label = 'Some Left';
287-
icon = Icons.check_circle_outline;
288-
break;
289-
case AvailabilityStatus.low:
290-
label = 'Low';
291-
icon = Icons.warning;
292-
break;
293-
case AvailabilityStatus.veryLow:
294-
label = 'Nearly Gone';
295-
icon = Icons.warning_amber;
296-
break;
297-
case AvailabilityStatus.out:
298-
label = 'Sold Out';
299-
icon = Icons.cancel;
300-
break;
301-
case AvailabilityStatus.unknown:
302-
label = rawText ?? 'Unknown';
303-
icon = Icons.info_outline;
304-
break;
305-
}
269+
// Switch *expression*, deliberately without a wildcard arm — see #534.
270+
final (label, icon) = switch (status) {
271+
AvailabilityStatus.plenty => ('Available', Icons.check_circle),
272+
AvailabilityStatus.good => ('Some Left', Icons.check_circle_outline),
273+
AvailabilityStatus.low => ('Low', Icons.warning),
274+
AvailabilityStatus.veryLow => ('Nearly Gone', Icons.warning_amber),
275+
AvailabilityStatus.out => ('Sold Out', Icons.cancel),
276+
AvailabilityStatus.unknown => (rawText ?? 'Unknown', Icons.info_outline),
277+
};
306278

307279
return Container(
308280
padding: const EdgeInsets.symmetric(horizontal: 8, vertical: 4),

lib/widgets/drink_filter_sheets.dart

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ class CategoryFilterSheet extends StatelessWidget {
4949
return Container(
5050
padding: const EdgeInsets.all(16),
5151
constraints: BoxConstraints(
52-
maxHeight: MediaQuery.of(context).size.height * 0.7,
52+
maxHeight: MediaQuery.sizeOf(context).height * 0.7,
5353
),
5454
child: Column(
5555
mainAxisSize: MainAxisSize.min,
@@ -113,8 +113,8 @@ class CategoryFilterSheet extends StatelessWidget {
113113
);
114114
return Semantics(
115115
label:
116-
'Filter by $formattedCategory, $count '
117-
'${count == 1 ? 'drink' : 'drinks'}',
116+
'Filter by $formattedCategory, '
117+
'${StringFormattingHelper.drinkCountLabel(count)}',
118118
value: isSelected ? 'Selected' : 'Not selected',
119119
selected: isSelected,
120120
button: true,
@@ -156,7 +156,7 @@ class SortOptionsSheet extends StatelessWidget {
156156
return Container(
157157
padding: const EdgeInsets.all(16),
158158
constraints: BoxConstraints(
159-
maxHeight: MediaQuery.of(context).size.height * 0.7,
159+
maxHeight: MediaQuery.sizeOf(context).height * 0.7,
160160
),
161161
child: Column(
162162
mainAxisSize: MainAxisSize.min,
@@ -233,7 +233,7 @@ class StyleFilterSheet extends StatelessWidget {
233233
return Container(
234234
padding: const EdgeInsets.all(16),
235235
constraints: BoxConstraints(
236-
maxHeight: MediaQuery.of(context).size.height * 0.7,
236+
maxHeight: MediaQuery.sizeOf(context).height * 0.7,
237237
),
238238
child: Column(
239239
mainAxisSize: MainAxisSize.min,
@@ -337,8 +337,8 @@ class StyleFilterSheet extends StatelessWidget {
337337
final isSelected = selectedStyles.contains(style);
338338
return Semantics(
339339
label:
340-
'Filter by $style, $count '
341-
'${count == 1 ? 'drink' : 'drinks'}',
340+
'Filter by $style, '
341+
'${StringFormattingHelper.drinkCountLabel(count)}',
342342
value: isSelected ? 'Selected' : 'Not selected',
343343
selected: isSelected,
344344
button: true,
@@ -382,7 +382,7 @@ class VisibilityFilterSheet extends StatelessWidget {
382382
return Container(
383383
padding: const EdgeInsets.all(16),
384384
constraints: BoxConstraints(
385-
maxHeight: MediaQuery.of(context).size.height * 0.7,
385+
maxHeight: MediaQuery.sizeOf(context).height * 0.7,
386386
),
387387
child: Column(
388388
mainAxisSize: MainAxisSize.min,

lib/widgets/festival_header.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import 'package:flutter/material.dart';
22
import '../models/models.dart';
33
import '../providers/providers.dart';
4+
import '../utils/utils.dart';
45

56
/// App-bar title for the drinks screen: app icon, current festival name, the
67
/// drink count, and a coloured status badge.
@@ -17,8 +18,7 @@ class FestivalHeader extends StatelessWidget {
1718
provider.sortedFestivals,
1819
);
1920
final drinkCount = provider.drinks.length;
20-
final drinkCountLabel =
21-
'$drinkCount ${drinkCount == 1 ? 'drink' : 'drinks'}';
21+
final drinkCountLabel = StringFormattingHelper.drinkCountLabel(drinkCount);
2222

2323
// Fold the status into the label and exclude child semantics so screen
2424
// readers announce one coherent phrase instead of the name, count, and

lib/widgets/festival_menu_sheets.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ class FestivalSelectorSheet extends StatelessWidget {
8585
return Container(
8686
padding: const EdgeInsets.all(16),
8787
constraints: BoxConstraints(
88-
maxHeight: MediaQuery.of(context).size.height * 0.7,
88+
maxHeight: MediaQuery.sizeOf(context).height * 0.7,
8989
),
9090
child: Column(
9191
mainAxisSize: MainAxisSize.min,

test/models_test.dart

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1424,6 +1424,61 @@ void main() {
14241424
expect(festival.formattedDates, contains(months[i]));
14251425
}
14261426
});
1427+
1428+
test('carries both years for a range spanning New Year', () {
1429+
final festival = Festival(
1430+
id: 'cbfw2025',
1431+
name: 'Cambridge Winter Beer Festival',
1432+
startDate: DateTime(2025, 12, 30),
1433+
endDate: DateTime(2026, 1, 2),
1434+
dataBaseUrl: 'https://example.com/cbfw2025',
1435+
);
1436+
1437+
expect(festival.formattedDates, 'Dec 30 - Jan 2, 2026');
1438+
});
1439+
});
1440+
1441+
group('equality', () {
1442+
const json = {
1443+
'id': 'cbf2025',
1444+
'name': 'Cambridge Beer Festival 2025',
1445+
'data_base_url': 'https://example.com/cbf2025',
1446+
};
1447+
1448+
test('two instances parsed from the same JSON are equal', () {
1449+
final a = Festival.fromJson(Map<String, dynamic>.from(json));
1450+
final b = Festival.fromJson(Map<String, dynamic>.from(json));
1451+
1452+
expect(a, equals(b));
1453+
expect(a.hashCode, equals(b.hashCode));
1454+
});
1455+
1456+
test('a Set de-duplicates instances with the same id', () {
1457+
final cached = Festival.fromJson(Map<String, dynamic>.from(json));
1458+
final fromNetwork = Festival.fromJson(Map<String, dynamic>.from(json));
1459+
1460+
expect({cached, fromNetwork}, hasLength(1));
1461+
});
1462+
1463+
test('festivals with different ids are not equal', () {
1464+
final a = Festival.fromJson(Map<String, dynamic>.from(json));
1465+
final b = Festival.fromJson({...json, 'id': 'cbf2026'});
1466+
1467+
expect(a, isNot(equals(b)));
1468+
});
1469+
1470+
test('an empty id falls back to identity', () {
1471+
// Built via fromJson so the two instances are distinct objects — a
1472+
// const literal pair would be canonicalised to the same instance and
1473+
// could not distinguish identity equality from id equality.
1474+
final emptyIdJson = <String, dynamic>{...json, 'id': ''};
1475+
final a = Festival.fromJson(Map<String, dynamic>.from(emptyIdJson));
1476+
final b = Festival.fromJson(Map<String, dynamic>.from(emptyIdJson));
1477+
1478+
expect(a, equals(a));
1479+
expect(a, isNot(equals(b)));
1480+
expect({a, b}, hasLength(2));
1481+
});
14271482
});
14281483

14291484
group('fromJson', () {

test/string_formatting_helper_test.dart

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,5 +28,20 @@ void main() {
2828
expect(StringFormattingHelper.capitalizeFirst('a'), 'A');
2929
});
3030
});
31+
32+
group('drinkCountLabel', () {
33+
test('pluralises zero', () {
34+
expect(StringFormattingHelper.drinkCountLabel(0), '0 drinks');
35+
});
36+
37+
test('uses the singular for exactly one', () {
38+
expect(StringFormattingHelper.drinkCountLabel(1), '1 drink');
39+
});
40+
41+
test('pluralises counts above one', () {
42+
expect(StringFormattingHelper.drinkCountLabel(2), '2 drinks');
43+
expect(StringFormattingHelper.drinkCountLabel(147), '147 drinks');
44+
});
45+
});
3146
});
3247
}

0 commit comments

Comments
 (0)