Skip to content

Commit 6337d0c

Browse files
richardthe3rdrichardthe3rd
authored
Merge pull request #196 from richardthe3rd/claude/review-festival-browsing-4hQKA
Review festival browsing selection changes
2 parents 6abdcc3 + 6020009 commit 6337d0c

11 files changed

Lines changed: 1312 additions & 657 deletions

‎docs/ui-components.md‎

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,71 @@
11
# UI Components
22

3+
## Overflow Menu
4+
5+
A shared three-dot menu providing access to global app features.
6+
7+
### Usage
8+
9+
```dart
10+
import 'package:cambridge_beer_festival/widgets/widgets.dart';
11+
12+
// Add to AppBar actions
13+
AppBar(
14+
title: Text('Screen Title'),
15+
actions: [
16+
buildOverflowMenu(context),
17+
],
18+
)
19+
```
20+
21+
### Menu Options
22+
23+
The overflow menu provides access to:
24+
- **Browse Festivals** - Opens festival selector bottom sheet
25+
- **Settings** - Opens app settings bottom sheet
26+
- **About** - Navigates to `/about` screen
27+
28+
### Where to Use
29+
30+
✅ **Include overflow menu on:**
31+
- Drinks screen (`DrinksScreen`)
32+
- Favorites screen (`FavoritesScreen`)
33+
- Any screen where users need access to festival switching or settings
34+
35+
❌ **Do NOT include on:**
36+
- About screen (already in the app menu)
37+
- Festival info screen (festival-specific, not global)
38+
- Detail screens (drink/brewery/style) - use back navigation only
39+
- Modal bottom sheets (use sheet close instead)
40+
41+
### Implementation Details
42+
43+
**Pattern:** Shared function (not a widget) that returns a `PopupMenuButton`
44+
45+
```dart
46+
Widget buildOverflowMenu(BuildContext context)
47+
```
48+
49+
**Accessibility:**
50+
- Main button has `Semantics` label: "Menu"
51+
- Main button has tooltip: "Menu"
52+
- Icons are decorative: wrapped in `ExcludeSemantics`
53+
- Screen readers announce: "Browse Festivals", "Settings", "About" (icon is skipped)
54+
55+
**Navigation:**
56+
- Festival browser and Settings open as modal bottom sheets
57+
- About navigates to `/about` route using `context.go()`
58+
59+
### Related Widgets
60+
61+
The overflow menu triggers these modal sheets:
62+
- `showFestivalBrowser(context)` - Shows `FestivalSelectorSheet`
63+
- `showSettingsSheet(context)` - Shows `SettingsSheet`
64+
65+
Both sheets are defined in `lib/widgets/festival_menu_sheets.dart`.
66+
67+
---
68+
369
## BreadcrumbBar
470

571
A navigation breadcrumb bar for detail screens.

‎lib/main.dart‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -365,6 +365,9 @@ class FavoritesScreen extends StatelessWidget {
365365
Text('${favorites.length} favorites', style: theme.textTheme.bodySmall),
366366
],
367367
),
368+
actions: [
369+
buildOverflowMenu(context),
370+
],
368371
),
369372
body: favorites.isEmpty
370373
? Semantics(

‎lib/router.dart‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,12 @@ final GoRouter appRouter = GoRouter(
2323
initialLocation: '/',
2424
debugLogDiagnostics: kDebugMode,
2525
routes: [
26+
// Global routes FIRST (before festival routes)
27+
// Must come before /:festivalId to avoid being caught as festival ID
28+
GoRoute(
29+
path: '/about',
30+
builder: (context, state) => const AboutScreen(),
31+
),
2632
// Parent shell - Ensures provider initialization for ALL routes
2733
// This fixes deep linking by initializing data before any screen renders
2834
ShellRoute(
@@ -135,11 +141,6 @@ final GoRouter appRouter = GoRouter(
135141
return FestivalInfoScreen(festivalId: festivalId);
136142
},
137143
),
138-
// Global routes (no festival scope)
139-
GoRoute(
140-
path: '/about',
141-
builder: (context, state) => const AboutScreen(),
142-
),
143144
],
144145
),
145146
],

‎lib/screens/about_screen.dart‎

Lines changed: 0 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,6 @@ class _AboutScreenState extends State<AboutScreen> {
7070
_buildHeader(context),
7171
_buildAppInfo(context),
7272
_buildBuildInfo(context),
73-
_buildDataInfo(context, provider),
7473
_buildSettings(context, provider),
7574
_buildLinks(context),
7675
_buildLegalInfo(context),
@@ -266,106 +265,6 @@ class _AboutScreenState extends State<AboutScreen> {
266265
}
267266
}
268267

269-
Widget _buildDataInfo(BuildContext context, BeerProvider provider) {
270-
final theme = Theme.of(context);
271-
final lastRefresh = provider.lastDrinksRefresh;
272-
273-
String refreshText;
274-
if (lastRefresh == null) {
275-
refreshText = 'Not yet loaded';
276-
} else {
277-
final now = DateTime.now();
278-
final difference = now.difference(lastRefresh);
279-
280-
if (difference.inMinutes < 1) {
281-
refreshText = 'Just now';
282-
} else if (difference.inHours < 1) {
283-
refreshText = '${difference.inMinutes} minute${difference.inMinutes == 1 ? '' : 's'} ago';
284-
} else if (difference.inDays < 1) {
285-
refreshText = '${difference.inHours} hour${difference.inHours == 1 ? '' : 's'} ago';
286-
} else {
287-
refreshText = DateFormat('MMM d, yyyy \'at\' h:mm a').format(lastRefresh);
288-
}
289-
}
290-
291-
return Padding(
292-
padding: const EdgeInsets.symmetric(horizontal: 16),
293-
child: Column(
294-
crossAxisAlignment: CrossAxisAlignment.start,
295-
children: [
296-
Text('Data', style: theme.textTheme.titleMedium),
297-
const SizedBox(height: 8),
298-
Card(
299-
child: Padding(
300-
padding: const EdgeInsets.all(16),
301-
child: Column(
302-
children: [
303-
Row(
304-
mainAxisAlignment: MainAxisAlignment.spaceBetween,
305-
children: [
306-
Text(
307-
'Last Updated',
308-
style: theme.textTheme.bodyMedium?.copyWith(
309-
color: theme.colorScheme.onSurfaceVariant,
310-
),
311-
),
312-
Text(
313-
refreshText,
314-
style: theme.textTheme.bodyMedium?.copyWith(
315-
fontWeight: FontWeight.w500,
316-
),
317-
),
318-
],
319-
),
320-
const SizedBox(height: 12),
321-
Row(
322-
mainAxisAlignment: MainAxisAlignment.spaceBetween,
323-
children: [
324-
Text(
325-
'Current Festival',
326-
style: theme.textTheme.bodyMedium?.copyWith(
327-
color: theme.colorScheme.onSurfaceVariant,
328-
),
329-
),
330-
Flexible(
331-
child: Text(
332-
provider.currentFestival.name,
333-
style: theme.textTheme.bodyMedium?.copyWith(
334-
fontWeight: FontWeight.w500,
335-
),
336-
textAlign: TextAlign.right,
337-
),
338-
),
339-
],
340-
),
341-
const SizedBox(height: 12),
342-
Row(
343-
mainAxisAlignment: MainAxisAlignment.spaceBetween,
344-
children: [
345-
Text(
346-
'Total Drinks',
347-
style: theme.textTheme.bodyMedium?.copyWith(
348-
color: theme.colorScheme.onSurfaceVariant,
349-
),
350-
),
351-
Text(
352-
'${provider.allDrinks.length}',
353-
style: theme.textTheme.bodyMedium?.copyWith(
354-
fontWeight: FontWeight.w500,
355-
),
356-
),
357-
],
358-
),
359-
],
360-
),
361-
),
362-
),
363-
const SizedBox(height: 16),
364-
],
365-
),
366-
);
367-
}
368-
369268
Widget _buildSettings(BuildContext context, BeerProvider provider) {
370269
final theme = Theme.of(context);
371270
final themeMode = provider.themeMode;

0 commit comments

Comments
 (0)