Skip to content

Commit f14d73c

Browse files
Merge pull request #206 from richardthe3rd/copilot/simplify-code-reduce-duplication
Remove unused go_router imports
2 parents 4ad34cf + de783f8 commit f14d73c

10 files changed

Lines changed: 449 additions & 117 deletions

lib/screens/brewery_screen.dart

Lines changed: 6 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import 'dart:async';
22
import 'package:flutter/material.dart';
3-
import 'package:go_router/go_router.dart';
43
import 'package:provider/provider.dart';
54
import '../providers/providers.dart';
65
import '../models/models.dart';
@@ -39,25 +38,13 @@ class _BreweryScreenState extends State<BreweryScreen> {
3938
});
4039
}
4140

42-
/// Safely check if we can pop (handles test contexts without GoRouter)
43-
bool _canPop(BuildContext context) {
44-
try {
45-
return GoRouter.of(context).canPop();
46-
} catch (e) {
47-
return false;
48-
}
49-
}
50-
5141
@override
5242
Widget build(BuildContext context) {
5343
final provider = context.watch<BeerProvider>();
5444

5545
// Show loading state while drinks are being fetched
5646
if (provider.isLoading) {
57-
return Scaffold(
58-
appBar: AppBar(title: const Text('Loading...')),
59-
body: const Center(child: CircularProgressIndicator()),
60-
);
47+
return buildLoadingScaffold();
6148
}
6249

6350
// Get all drinks from this brewery
@@ -80,19 +67,7 @@ class _BreweryScreenState extends State<BreweryScreen> {
8067
return Scaffold(
8168
appBar: AppBar(
8269
title: _buildAppBarTitle(context, provider, producer),
83-
leading: _canPop(context)
84-
? null
85-
: Semantics(
86-
label: 'Go to home screen',
87-
hint: 'Double tap to return to drinks list',
88-
button: true,
89-
child: IconButton(
90-
icon: const Icon(Icons.home),
91-
onPressed: () =>
92-
context.go(buildFestivalHome(widget.festivalId)),
93-
tooltip: 'Home',
94-
),
95-
),
70+
leading: buildHomeLeadingButton(context, widget.festivalId),
9671
),
9772
body: CustomScrollView(
9873
slivers: [
@@ -122,25 +97,10 @@ class _BreweryScreenState extends State<BreweryScreen> {
12297
BeerProvider provider,
12398
Producer producer,
12499
) {
125-
final festivalName = provider.currentFestival.name;
126-
127-
return Column(
128-
mainAxisSize: MainAxisSize.min,
129-
crossAxisAlignment: CrossAxisAlignment.start,
130-
children: [
131-
Text(
132-
producer.name,
133-
style: Theme.of(context).textTheme.titleLarge,
134-
overflow: TextOverflow.ellipsis,
135-
),
136-
Text(
137-
festivalName,
138-
style: Theme.of(context).textTheme.bodySmall?.copyWith(
139-
color: Theme.of(context).colorScheme.onSurfaceVariant,
140-
),
141-
overflow: TextOverflow.ellipsis,
142-
),
143-
],
100+
return buildBreadcrumbTitle(
101+
context,
102+
title: producer.name,
103+
festivalName: provider.currentFestival.name,
144104
);
145105
}
146106

lib/screens/drink_detail_screen.dart

Lines changed: 2 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,7 @@ class _DrinkDetailScreenState extends State<DrinkDetailScreen> {
4646

4747
// Show loading state while drinks are being fetched
4848
if (provider.isLoading) {
49-
return Scaffold(
50-
appBar: AppBar(title: const Text('Loading...')),
51-
body: const Center(child: CircularProgressIndicator()),
52-
);
49+
return buildLoadingScaffold();
5350
}
5451

5552
final drink = provider.getDrinkById(widget.drinkId);
@@ -66,18 +63,7 @@ class _DrinkDetailScreenState extends State<DrinkDetailScreen> {
6663
return Scaffold(
6764
appBar: AppBar(
6865
title: _buildAppBarTitle(context, provider, drink),
69-
leading: _canPop(context)
70-
? null
71-
: Semantics(
72-
label: 'Go to home screen',
73-
hint: 'Double tap to return to drinks list',
74-
button: true,
75-
child: IconButton(
76-
icon: const Icon(Icons.home),
77-
onPressed: () => context.go(buildFestivalHome(widget.festivalId)),
78-
tooltip: 'Home',
79-
),
80-
),
66+
leading: buildHomeLeadingButton(context, widget.festivalId),
8167
),
8268
body: Column(
8369
children: [
@@ -475,13 +461,4 @@ class _DrinkDetailScreenState extends State<DrinkDetailScreen> {
475461
final provider = context.read<BeerProvider>();
476462
unawaited(provider.analyticsService.logDrinkShared(drink));
477463
}
478-
479-
bool _canPop(BuildContext context) {
480-
try {
481-
GoRouter.of(context);
482-
return context.canPop();
483-
} catch (e) {
484-
return true;
485-
}
486-
}
487464
}

lib/screens/style_screen.dart

Lines changed: 6 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,5 @@
11
import 'dart:async';
22
import 'package:flutter/material.dart';
3-
import 'package:go_router/go_router.dart';
43
import 'package:provider/provider.dart';
54
import '../providers/providers.dart';
65
import '../models/models.dart';
@@ -33,25 +32,13 @@ class _StyleScreenState extends State<StyleScreen> {
3332
});
3433
}
3534

36-
/// Safely check if we can pop (handles test contexts without GoRouter)
37-
bool _canPop(BuildContext context) {
38-
try {
39-
return GoRouter.of(context).canPop();
40-
} catch (e) {
41-
return false;
42-
}
43-
}
44-
4535
@override
4636
Widget build(BuildContext context) {
4737
final provider = context.watch<BeerProvider>();
4838

4939
// Show loading state while drinks are being fetched
5040
if (provider.isLoading) {
51-
return Scaffold(
52-
appBar: AppBar(title: const Text('Loading...')),
53-
body: const Center(child: CircularProgressIndicator()),
54-
);
41+
return buildLoadingScaffold();
5542
}
5643

5744
// Get all drinks with this style
@@ -73,19 +60,7 @@ class _StyleScreenState extends State<StyleScreen> {
7360
return Scaffold(
7461
appBar: AppBar(
7562
title: _buildAppBarTitle(context, provider),
76-
leading: _canPop(context)
77-
? null
78-
: Semantics(
79-
label: 'Go to home screen',
80-
hint: 'Double tap to return to drinks list',
81-
button: true,
82-
child: IconButton(
83-
icon: const Icon(Icons.home),
84-
onPressed: () =>
85-
context.go(buildFestivalHome(widget.festivalId)),
86-
tooltip: 'Home',
87-
),
88-
),
63+
leading: buildHomeLeadingButton(context, widget.festivalId),
8964
),
9065
body: CustomScrollView(
9166
slivers: [
@@ -123,25 +98,10 @@ class _StyleScreenState extends State<StyleScreen> {
12398

12499
/// Build the app bar title with breadcrumb navigation
125100
Widget _buildAppBarTitle(BuildContext context, BeerProvider provider) {
126-
final festivalName = provider.currentFestival.name;
127-
128-
return Column(
129-
mainAxisSize: MainAxisSize.min,
130-
crossAxisAlignment: CrossAxisAlignment.start,
131-
children: [
132-
Text(
133-
widget.style,
134-
style: Theme.of(context).textTheme.titleLarge,
135-
overflow: TextOverflow.ellipsis,
136-
),
137-
Text(
138-
festivalName,
139-
style: Theme.of(context).textTheme.bodySmall?.copyWith(
140-
color: Theme.of(context).colorScheme.onSurfaceVariant,
141-
),
142-
overflow: TextOverflow.ellipsis,
143-
),
144-
],
101+
return buildBreadcrumbTitle(
102+
context,
103+
title: widget.style,
104+
festivalName: provider.currentFestival.name,
145105
);
146106
}
147107

lib/utils/navigation_helpers.dart

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77
/// characters safely.
88
library;
99

10+
import 'package:flutter/material.dart';
11+
import 'package:go_router/go_router.dart';
12+
1013
/// Builds a festival-scoped URL path.
1114
///
1215
/// The [festivalId] and [path] must not be empty.
@@ -191,3 +194,25 @@ String? extractFestivalId(String path) {
191194
bool isFestivalPath(String path) {
192195
return extractFestivalId(path) != null;
193196
}
197+
198+
/// Checks if navigation can pop in the current context.
199+
///
200+
/// Safely handles contexts where GoRouter may not be available (e.g., in tests).
201+
/// Returns `true` if the router can navigate back, `false` otherwise.
202+
///
203+
/// This is useful for determining whether to show a back button or a home button
204+
/// in the app bar.
205+
///
206+
/// Example:
207+
/// ```dart
208+
/// final canPop = canPopNavigation(context);
209+
/// leading: canPop ? null : IconButton(icon: Icon(Icons.home), ...)
210+
/// ```
211+
bool canPopNavigation(BuildContext context) {
212+
try {
213+
return GoRouter.of(context).canPop();
214+
} catch (e) {
215+
// GoRouter not available (e.g., in tests)
216+
return false;
217+
}
218+
}

lib/utils/utils.dart

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,4 @@ export 'string_comparison_helper.dart';
66
export 'string_formatting_helper.dart';
77
export 'style_description_helper.dart';
88
export 'url_launcher_helper.dart';
9+
export 'widget_builders.dart';

lib/utils/widget_builders.dart

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
/// Common widget builders for reducing duplication across screens.
2+
///
3+
/// This file contains reusable widget builders that are used across multiple
4+
/// screens to maintain consistency and reduce code duplication.
5+
library;
6+
7+
import 'package:flutter/material.dart';
8+
import 'package:go_router/go_router.dart';
9+
import 'navigation_helpers.dart';
10+
11+
/// Builds a loading scaffold with standard appearance.
12+
///
13+
/// Used when data is being fetched to show a consistent loading state
14+
/// across all screens.
15+
///
16+
/// Example:
17+
/// ```dart
18+
/// if (provider.isLoading) {
19+
/// return buildLoadingScaffold();
20+
/// }
21+
/// ```
22+
Widget buildLoadingScaffold() {
23+
return Scaffold(
24+
appBar: AppBar(title: const Text('Loading...')),
25+
body: const Center(child: CircularProgressIndicator()),
26+
);
27+
}
28+
29+
/// Builds a home button for the AppBar leading position.
30+
///
31+
/// Shows a home button instead of the back button when navigation cannot pop.
32+
/// This ensures users can always navigate back to the festival home.
33+
///
34+
/// The [festivalId] is used to navigate to the correct festival home page.
35+
///
36+
/// Example:
37+
/// ```dart
38+
/// AppBar(
39+
/// leading: buildHomeLeadingButton(context, festivalId),
40+
/// )
41+
/// ```
42+
Widget? buildHomeLeadingButton(BuildContext context, String festivalId) {
43+
if (canPopNavigation(context)) {
44+
return null; // Use default back button
45+
}
46+
47+
return Semantics(
48+
label: 'Go to home screen',
49+
hint: 'Double tap to return to drinks list',
50+
button: true,
51+
child: IconButton(
52+
icon: const Icon(Icons.home),
53+
onPressed: () => context.go(buildFestivalHome(festivalId)),
54+
tooltip: 'Home',
55+
),
56+
);
57+
}
58+
59+
/// Builds a breadcrumb-style title for the AppBar.
60+
///
61+
/// Shows a primary title with the festival name as a subtitle for context.
62+
/// This provides consistent navigation breadcrumbs across detail screens.
63+
///
64+
/// The [title] is the main heading (e.g., brewery name, style name, drink name).
65+
/// The [festivalName] appears as a smaller subtitle below the title.
66+
///
67+
/// Example:
68+
/// ```dart
69+
/// AppBar(
70+
/// title: buildBreadcrumbTitle(
71+
/// context,
72+
/// title: 'IPA',
73+
/// festivalName: 'Cambridge Beer Festival 2025',
74+
/// ),
75+
/// )
76+
/// ```
77+
Widget buildBreadcrumbTitle(
78+
BuildContext context, {
79+
required String title,
80+
required String festivalName,
81+
}) {
82+
final theme = Theme.of(context);
83+
84+
return Column(
85+
mainAxisSize: MainAxisSize.min,
86+
crossAxisAlignment: CrossAxisAlignment.start,
87+
children: [
88+
Text(
89+
title,
90+
style: theme.textTheme.titleLarge,
91+
overflow: TextOverflow.ellipsis,
92+
),
93+
Text(
94+
festivalName,
95+
style: theme.textTheme.bodySmall?.copyWith(
96+
color: theme.colorScheme.onSurfaceVariant,
97+
),
98+
overflow: TextOverflow.ellipsis,
99+
),
100+
],
101+
);
102+
}
39 Bytes
Loading
37 Bytes
Loading

test/utils/navigation_helpers_test.dart

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import 'package:cambridge_beer_festival/utils/utils.dart';
2+
import 'package:flutter/material.dart';
23
import 'package:flutter_test/flutter_test.dart';
34

45
void main() {
@@ -315,5 +316,28 @@ void main() {
315316
);
316317
});
317318
});
319+
320+
group('canPopNavigation', () {
321+
testWidgets('returns false when GoRouter is not available', (tester) async {
322+
// In test environment with MaterialApp but without GoRouter
323+
await tester.pumpWidget(
324+
MaterialApp(
325+
home: Builder(
326+
builder: (context) {
327+
final result = canPopNavigation(context);
328+
return Scaffold(
329+
body: Text('Can pop: $result'),
330+
);
331+
},
332+
),
333+
),
334+
);
335+
336+
await tester.pumpAndSettle();
337+
338+
// Without GoRouter, canPopNavigation should return false
339+
expect(find.text('Can pop: false'), findsOneWidget);
340+
});
341+
});
318342
});
319343
}

0 commit comments

Comments
 (0)