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
52 changes: 6 additions & 46 deletions lib/screens/brewery_screen.dart
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import 'dart:async';
import 'package:flutter/material.dart';
import 'package:go_router/go_router.dart';
import 'package:provider/provider.dart';
import '../providers/providers.dart';
import '../models/models.dart';
Expand Down Expand Up @@ -39,25 +38,13 @@ class _BreweryScreenState extends State<BreweryScreen> {
});
}

/// Safely check if we can pop (handles test contexts without GoRouter)
bool _canPop(BuildContext context) {
try {
return GoRouter.of(context).canPop();
} catch (e) {
return false;
}
}

@override
Widget build(BuildContext context) {
final provider = context.watch<BeerProvider>();

// Show loading state while drinks are being fetched
if (provider.isLoading) {
return Scaffold(
appBar: AppBar(title: const Text('Loading...')),
body: const Center(child: CircularProgressIndicator()),
);
return buildLoadingScaffold();
}

// Get all drinks from this brewery
Expand All @@ -80,19 +67,7 @@ class _BreweryScreenState extends State<BreweryScreen> {
return Scaffold(
appBar: AppBar(
title: _buildAppBarTitle(context, provider, producer),
leading: _canPop(context)
? null
: Semantics(
label: 'Go to home screen',
hint: 'Double tap to return to drinks list',
button: true,
child: IconButton(
icon: const Icon(Icons.home),
onPressed: () =>
context.go(buildFestivalHome(widget.festivalId)),
tooltip: 'Home',
),
),
leading: buildHomeLeadingButton(context, widget.festivalId),
),
body: CustomScrollView(
slivers: [
Expand Down Expand Up @@ -122,25 +97,10 @@ class _BreweryScreenState extends State<BreweryScreen> {
BeerProvider provider,
Producer producer,
) {
final festivalName = provider.currentFestival.name;

return Column(
mainAxisSize: MainAxisSize.min,
crossAxisAlignment: CrossAxisAlignment.start,
children: [
Text(
producer.name,
style: Theme.of(context).textTheme.titleLarge,
overflow: TextOverflow.ellipsis,
),
Text(
festivalName,
style: Theme.of(context).textTheme.bodySmall?.copyWith(
color: Theme.of(context).colorScheme.onSurfaceVariant,
),
overflow: TextOverflow.ellipsis,
),
],
return buildBreadcrumbTitle(
context,
title: producer.name,
festivalName: provider.currentFestival.name,
);
}

Expand Down
27 changes: 2 additions & 25 deletions lib/screens/drink_detail_screen.dart
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,7 @@ class _DrinkDetailScreenState extends State<DrinkDetailScreen> {

// Show loading state while drinks are being fetched
if (provider.isLoading) {
return Scaffold(
appBar: AppBar(title: const Text('Loading...')),
body: const Center(child: CircularProgressIndicator()),
);
return buildLoadingScaffold();
}

final drink = provider.getDrinkById(widget.drinkId);
Expand All @@ -66,18 +63,7 @@ class _DrinkDetailScreenState extends State<DrinkDetailScreen> {
return Scaffold(
appBar: AppBar(
title: _buildAppBarTitle(context, provider, drink),
leading: _canPop(context)
? null
: Semantics(
label: 'Go to home screen',
hint: 'Double tap to return to drinks list',
button: true,
child: IconButton(
icon: const Icon(Icons.home),
onPressed: () => context.go(buildFestivalHome(widget.festivalId)),
tooltip: 'Home',
),
),
leading: buildHomeLeadingButton(context, widget.festivalId),
),
body: Column(
children: [
Expand Down Expand Up @@ -475,13 +461,4 @@ class _DrinkDetailScreenState extends State<DrinkDetailScreen> {
final provider = context.read<BeerProvider>();
unawaited(provider.analyticsService.logDrinkShared(drink));
}

bool _canPop(BuildContext context) {
try {
GoRouter.of(context);
return context.canPop();
} catch (e) {
return true;
}
}
}
52 changes: 6 additions & 46 deletions lib/screens/style_screen.dart
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import 'dart:async';
import 'package:flutter/material.dart';
import 'package:go_router/go_router.dart';
import 'package:provider/provider.dart';
import '../providers/providers.dart';
import '../models/models.dart';
Expand Down Expand Up @@ -33,25 +32,13 @@ class _StyleScreenState extends State<StyleScreen> {
});
}

/// Safely check if we can pop (handles test contexts without GoRouter)
bool _canPop(BuildContext context) {
try {
return GoRouter.of(context).canPop();
} catch (e) {
return false;
}
}

@override
Widget build(BuildContext context) {
final provider = context.watch<BeerProvider>();

// Show loading state while drinks are being fetched
if (provider.isLoading) {
return Scaffold(
appBar: AppBar(title: const Text('Loading...')),
body: const Center(child: CircularProgressIndicator()),
);
return buildLoadingScaffold();
}

// Get all drinks with this style
Expand All @@ -73,19 +60,7 @@ class _StyleScreenState extends State<StyleScreen> {
return Scaffold(
appBar: AppBar(
title: _buildAppBarTitle(context, provider),
leading: _canPop(context)
? null
: Semantics(
label: 'Go to home screen',
hint: 'Double tap to return to drinks list',
button: true,
child: IconButton(
icon: const Icon(Icons.home),
onPressed: () =>
context.go(buildFestivalHome(widget.festivalId)),
tooltip: 'Home',
),
),
leading: buildHomeLeadingButton(context, widget.festivalId),
),
body: CustomScrollView(
slivers: [
Expand Down Expand Up @@ -123,25 +98,10 @@ class _StyleScreenState extends State<StyleScreen> {

/// Build the app bar title with breadcrumb navigation
Widget _buildAppBarTitle(BuildContext context, BeerProvider provider) {
final festivalName = provider.currentFestival.name;

return Column(
mainAxisSize: MainAxisSize.min,
crossAxisAlignment: CrossAxisAlignment.start,
children: [
Text(
widget.style,
style: Theme.of(context).textTheme.titleLarge,
overflow: TextOverflow.ellipsis,
),
Text(
festivalName,
style: Theme.of(context).textTheme.bodySmall?.copyWith(
color: Theme.of(context).colorScheme.onSurfaceVariant,
),
overflow: TextOverflow.ellipsis,
),
],
return buildBreadcrumbTitle(
context,
title: widget.style,
festivalName: provider.currentFestival.name,
);
}

Expand Down
25 changes: 25 additions & 0 deletions lib/utils/navigation_helpers.dart
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@
/// characters safely.
library;

import 'package:flutter/material.dart';
import 'package:go_router/go_router.dart';

/// Builds a festival-scoped URL path.
///
/// The [festivalId] and [path] must not be empty.
Expand Down Expand Up @@ -191,3 +194,25 @@ String? extractFestivalId(String path) {
bool isFestivalPath(String path) {
return extractFestivalId(path) != null;
}

/// Checks if navigation can pop in the current context.
///
/// Safely handles contexts where GoRouter may not be available (e.g., in tests).
/// Returns `true` if the router can navigate back, `false` otherwise.
///
/// This is useful for determining whether to show a back button or a home button
/// in the app bar.
///
/// Example:
/// ```dart
/// final canPop = canPopNavigation(context);
/// leading: canPop ? null : IconButton(icon: Icon(Icons.home), ...)
/// ```
bool canPopNavigation(BuildContext context) {
try {
return GoRouter.of(context).canPop();
} catch (e) {
// GoRouter not available (e.g., in tests)
return false;
}
}
Comment on lines +211 to +218

Copilot AI Jan 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new canPopNavigation function lacks test coverage. Other navigation helper functions in this file have comprehensive unit tests. Consider adding tests to verify the function returns true when GoRouter.canPop() is true, returns false when GoRouter.canPop() is false, and handles the case where GoRouter is not available (e.g., returns false and doesn't throw).

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added test coverage for canPopNavigation() in commit de783f8. The test verifies it returns false when GoRouter is not available (e.g., in test environment without router setup). All tests pass (552/552).

1 change: 1 addition & 0 deletions lib/utils/utils.dart
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ export 'string_comparison_helper.dart';
export 'string_formatting_helper.dart';
export 'style_description_helper.dart';
export 'url_launcher_helper.dart';
export 'widget_builders.dart';
102 changes: 102 additions & 0 deletions lib/utils/widget_builders.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
/// Common widget builders for reducing duplication across screens.
///
/// This file contains reusable widget builders that are used across multiple
/// screens to maintain consistency and reduce code duplication.
library;

import 'package:flutter/material.dart';
import 'package:go_router/go_router.dart';
import 'navigation_helpers.dart';

/// Builds a loading scaffold with standard appearance.
///
/// Used when data is being fetched to show a consistent loading state
/// across all screens.
///
/// Example:
/// ```dart
/// if (provider.isLoading) {
/// return buildLoadingScaffold();
/// }
/// ```
Widget buildLoadingScaffold() {
return Scaffold(
appBar: AppBar(title: const Text('Loading...')),
body: const Center(child: CircularProgressIndicator()),
);
}

/// Builds a home button for the AppBar leading position.
///
/// Shows a home button instead of the back button when navigation cannot pop.
/// This ensures users can always navigate back to the festival home.
///
/// The [festivalId] is used to navigate to the correct festival home page.
///
/// Example:
/// ```dart
/// AppBar(
/// leading: buildHomeLeadingButton(context, festivalId),
/// )
/// ```
Widget? buildHomeLeadingButton(BuildContext context, String festivalId) {
if (canPopNavigation(context)) {
return null; // Use default back button
}

return Semantics(
label: 'Go to home screen',
hint: 'Double tap to return to drinks list',
button: true,
child: IconButton(
icon: const Icon(Icons.home),
onPressed: () => context.go(buildFestivalHome(festivalId)),
tooltip: 'Home',
),
);
}

/// Builds a breadcrumb-style title for the AppBar.
///
/// Shows a primary title with the festival name as a subtitle for context.
/// This provides consistent navigation breadcrumbs across detail screens.
///
/// The [title] is the main heading (e.g., brewery name, style name, drink name).
/// The [festivalName] appears as a smaller subtitle below the title.
///
/// Example:
/// ```dart
/// AppBar(
/// title: buildBreadcrumbTitle(
/// context,
/// title: 'IPA',
/// festivalName: 'Cambridge Beer Festival 2025',
/// ),
/// )
/// ```
Widget buildBreadcrumbTitle(
BuildContext context, {
required String title,
required String festivalName,
}) {
final theme = Theme.of(context);

return Column(
mainAxisSize: MainAxisSize.min,
crossAxisAlignment: CrossAxisAlignment.start,
children: [
Text(
title,
style: theme.textTheme.titleLarge,
overflow: TextOverflow.ellipsis,
),
Text(
festivalName,
style: theme.textTheme.bodySmall?.copyWith(
color: theme.colorScheme.onSurfaceVariant,
),
overflow: TextOverflow.ellipsis,
),
],
);
}
Comment on lines +1 to +102

Copilot AI Jan 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new widget builder functions (buildLoadingScaffold, buildHomeLeadingButton, and buildBreadcrumbTitle) lack test coverage. Other utility functions in the codebase (e.g., navigation_helpers.dart) have comprehensive unit tests. Consider adding tests to verify the widget structure, semantics labels, and interaction behaviors, especially for buildHomeLeadingButton which has conditional logic and navigation behavior.

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added comprehensive test coverage for widget_builders.dart in commit de783f8. The tests verify:

  • buildLoadingScaffold(): widget structure, loading indicator, app bar title
  • buildHomeLeadingButton(): widget creation, semantics labels, tooltip, navigation behavior
  • buildBreadcrumbTitle(): title/subtitle display, text styles, overflow handling, theme colors

All tests pass (552/552).

Binary file modified test/goldens/drink_detail_screen_long_name_light.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file modified test/goldens/drink_detail_screen_medium_name_light.png
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
24 changes: 24 additions & 0 deletions test/utils/navigation_helpers_test.dart
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import 'package:cambridge_beer_festival/utils/utils.dart';
import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart';

void main() {
Expand Down Expand Up @@ -315,5 +316,28 @@ void main() {
);
});
});

group('canPopNavigation', () {
testWidgets('returns false when GoRouter is not available', (tester) async {
// In test environment with MaterialApp but without GoRouter
await tester.pumpWidget(
MaterialApp(
home: Builder(
builder: (context) {
final result = canPopNavigation(context);
return Scaffold(
body: Text('Can pop: $result'),
);
},
),
),
);

await tester.pumpAndSettle();

// Without GoRouter, canPopNavigation should return false
expect(find.text('Can pop: false'), findsOneWidget);
});
});
});
}
Loading
Loading