From d7481882868b9948c9f0d17de404d9ee7df23908 Mon Sep 17 00:00:00 2001 From: Jorge Coca Date: Sun, 1 May 2022 17:58:03 -0500 Subject: [PATCH] refactor: improve HowToPlayDialog and reuse existing components (#281) --- lib/how_to_play/how_to_play.dart | 1 + .../widgets/how_to_play_dialog.dart | 130 +++++------------- .../widgets/widgets.dart | 0 .../view/character_selection_page.dart | 2 +- lib/start_game/start_game.dart | 1 - .../how_to_play_dialog_test.dart | 17 +-- .../view/character_selection_page_test.dart | 2 +- 7 files changed, 43 insertions(+), 110 deletions(-) create mode 100644 lib/how_to_play/how_to_play.dart rename lib/{start_game => how_to_play}/widgets/how_to_play_dialog.dart (68%) rename lib/{start_game => how_to_play}/widgets/widgets.dart (100%) rename test/{start_game/widgets => how_to_play}/how_to_play_dialog_test.dart (84%) diff --git a/lib/how_to_play/how_to_play.dart b/lib/how_to_play/how_to_play.dart new file mode 100644 index 00000000..e691bba3 --- /dev/null +++ b/lib/how_to_play/how_to_play.dart @@ -0,0 +1 @@ +export 'widgets/widgets.dart'; diff --git a/lib/start_game/widgets/how_to_play_dialog.dart b/lib/how_to_play/widgets/how_to_play_dialog.dart similarity index 68% rename from lib/start_game/widgets/how_to_play_dialog.dart rename to lib/how_to_play/widgets/how_to_play_dialog.dart index 5d8c68c3..766944b9 100644 --- a/lib/start_game/widgets/how_to_play_dialog.dart +++ b/lib/how_to_play/widgets/how_to_play_dialog.dart @@ -8,7 +8,6 @@ import 'package:pinball/l10n/l10n.dart'; import 'package:pinball_ui/pinball_ui.dart'; import 'package:platform_helper/platform_helper.dart'; -@visibleForTesting enum Control { left, right, @@ -51,18 +50,10 @@ extension on Control { } Future showHowToPlayDialog(BuildContext context) { - final height = MediaQuery.of(context).size.height * 0.5; return showDialog( context: context, - builder: (context) { - return Center( - child: SizedBox( - height: height, - width: height * 1.4, - child: HowToPlayDialog(), - ), - ); - }, + barrierDismissible: false, + builder: (_) => HowToPlayDialog(), ); } @@ -100,9 +91,11 @@ class _HowToPlayDialogState extends State { @override Widget build(BuildContext context) { final isMobile = widget.platformHelper.isMobile; - return PixelatedDecoration( - header: const _HowToPlayHeader(), - body: isMobile ? const _MobileBody() : const _DesktopBody(), + final l10n = context.l10n; + return PinballDialog( + title: l10n.howToPlay, + subtitle: l10n.tipsForFlips, + child: isMobile ? const _MobileBody() : const _DesktopBody(), ); } } @@ -137,26 +130,20 @@ class _MobileLaunchControls extends StatelessWidget { @override Widget build(BuildContext context) { final l10n = context.l10n; - final textStyle = Theme.of(context) + final headline3 = Theme.of(context) .textTheme .headline3! .copyWith(color: PinballColors.white); return Column( children: [ - Text( - l10n.tapAndHoldRocket, - style: textStyle, - ), + Text(l10n.tapAndHoldRocket, style: headline3), Text.rich( TextSpan( children: [ - TextSpan( - text: '${l10n.to} ', - style: textStyle, - ), + TextSpan(text: '${l10n.to} ', style: headline3), TextSpan( text: l10n.launch, - style: textStyle.copyWith(color: PinballColors.blue), + style: headline3.copyWith(color: PinballColors.blue), ), ], ), @@ -172,26 +159,20 @@ class _MobileFlipperControls extends StatelessWidget { @override Widget build(BuildContext context) { final l10n = context.l10n; - final textStyle = Theme.of(context) + final headline3 = Theme.of(context) .textTheme .headline3! .copyWith(color: PinballColors.white); return Column( children: [ - Text( - l10n.tapLeftRightScreen, - style: textStyle, - ), + Text(l10n.tapLeftRightScreen, style: headline3), Text.rich( TextSpan( children: [ - TextSpan( - text: '${l10n.to} ', - style: textStyle, - ), + TextSpan(text: '${l10n.to} ', style: headline3), TextSpan( text: l10n.flip, - style: textStyle.copyWith(color: PinballColors.orange), + style: headline3.copyWith(color: PinballColors.orange), ), ], ), @@ -206,55 +187,23 @@ class _DesktopBody extends StatelessWidget { @override Widget build(BuildContext context) { - const spacing = SizedBox(height: 16); return ListView( children: const [ - spacing, + SizedBox(height: 16), _DesktopLaunchControls(), - spacing, + SizedBox(height: 16), _DesktopFlipperControls(), ], ); } } -class _HowToPlayHeader extends StatelessWidget { - const _HowToPlayHeader({Key? key}) : super(key: key); - - @override - Widget build(BuildContext context) { - final l10n = context.l10n; - final textStyle = Theme.of(context).textTheme.headline3?.copyWith( - color: PinballColors.darkBlue, - ); - return FittedBox( - child: Column( - mainAxisAlignment: MainAxisAlignment.center, - children: [ - Text( - l10n.howToPlay, - style: textStyle?.copyWith( - fontWeight: FontWeight.bold, - ), - ), - Text( - l10n.tipsForFlips, - style: textStyle, - ), - ], - ), - ); - } -} - class _DesktopLaunchControls extends StatelessWidget { const _DesktopLaunchControls({Key? key}) : super(key: key); @override Widget build(BuildContext context) { final l10n = context.l10n; - const spacing = SizedBox(width: 10); - return Column( children: [ Text( @@ -264,11 +213,11 @@ class _DesktopLaunchControls extends StatelessWidget { const SizedBox(height: 10), Wrap( children: const [ - KeyButton(control: Control.down), - spacing, - KeyButton(control: Control.space), - spacing, - KeyButton(control: Control.s), + _KeyButton(control: Control.down), + SizedBox(width: 10), + _KeyButton(control: Control.space), + SizedBox(width: 10), + _KeyButton(control: Control.s), ], ) ], @@ -282,8 +231,6 @@ class _DesktopFlipperControls extends StatelessWidget { @override Widget build(BuildContext context) { final l10n = context.l10n; - const rowSpacing = SizedBox(width: 20); - return Column( children: [ Text( @@ -297,17 +244,17 @@ class _DesktopFlipperControls extends StatelessWidget { mainAxisSize: MainAxisSize.min, mainAxisAlignment: MainAxisAlignment.center, children: const [ - KeyButton(control: Control.left), - rowSpacing, - KeyButton(control: Control.right), + _KeyButton(control: Control.left), + SizedBox(width: 20), + _KeyButton(control: Control.right), ], ), const SizedBox(height: 8), Wrap( children: const [ - KeyButton(control: Control.a), - rowSpacing, - KeyButton(control: Control.d), + _KeyButton(control: Control.a), + SizedBox(width: 20), + _KeyButton(control: Control.d), ], ) ], @@ -317,29 +264,24 @@ class _DesktopFlipperControls extends StatelessWidget { } } -@visibleForTesting -class KeyButton extends StatelessWidget { - const KeyButton({ - Key? key, - required Control control, - }) : _control = control, - super(key: key); +class _KeyButton extends StatelessWidget { + const _KeyButton({Key? key, required this.control}) : super(key: key); - final Control _control; + final Control control; @override Widget build(BuildContext context) { final textTheme = Theme.of(context).textTheme; final textStyle = - _control.isArrow ? textTheme.headline1 : textTheme.headline3; + control.isArrow ? textTheme.headline1 : textTheme.headline3; const height = 60.0; - final width = _control.isSpace ? height * 2.83 : height; + final width = control.isSpace ? height * 2.83 : height; return DecoratedBox( decoration: BoxDecoration( image: DecorationImage( fit: BoxFit.fill, image: AssetImage( - _control.isSpace + control.isSpace ? Assets.images.components.space.keyName : Assets.images.components.key.keyName, ), @@ -350,9 +292,9 @@ class KeyButton extends StatelessWidget { height: height, child: Center( child: RotatedBox( - quarterTurns: _control.isDown ? 1 : 0, + quarterTurns: control.isDown ? 1 : 0, child: Text( - _control.getCharacter(context), + control.getCharacter(context), style: textStyle?.copyWith(color: PinballColors.white), ), ), diff --git a/lib/start_game/widgets/widgets.dart b/lib/how_to_play/widgets/widgets.dart similarity index 100% rename from lib/start_game/widgets/widgets.dart rename to lib/how_to_play/widgets/widgets.dart diff --git a/lib/select_character/view/character_selection_page.dart b/lib/select_character/view/character_selection_page.dart index 864d1b41..1df01ad7 100644 --- a/lib/select_character/view/character_selection_page.dart +++ b/lib/select_character/view/character_selection_page.dart @@ -1,9 +1,9 @@ import 'package:flutter/material.dart'; import 'package:flutter_bloc/flutter_bloc.dart'; +import 'package:pinball/how_to_play/how_to_play.dart'; import 'package:pinball/l10n/l10n.dart'; import 'package:pinball/select_character/cubit/character_theme_cubit.dart'; import 'package:pinball/select_character/select_character.dart'; -import 'package:pinball/start_game/start_game.dart'; import 'package:pinball_theme/pinball_theme.dart'; import 'package:pinball_ui/pinball_ui.dart'; diff --git a/lib/start_game/start_game.dart b/lib/start_game/start_game.dart index 1556b533..7171c66d 100644 --- a/lib/start_game/start_game.dart +++ b/lib/start_game/start_game.dart @@ -1,2 +1 @@ export 'bloc/start_game_bloc.dart'; -export 'widgets/widgets.dart'; diff --git a/test/start_game/widgets/how_to_play_dialog_test.dart b/test/how_to_play/how_to_play_dialog_test.dart similarity index 84% rename from test/start_game/widgets/how_to_play_dialog_test.dart rename to test/how_to_play/how_to_play_dialog_test.dart index cf8b4971..24c683a4 100644 --- a/test/start_game/widgets/how_to_play_dialog_test.dart +++ b/test/how_to_play/how_to_play_dialog_test.dart @@ -1,13 +1,11 @@ -// ignore_for_file: prefer_const_constructors - import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:mocktail/mocktail.dart'; +import 'package:pinball/how_to_play/how_to_play.dart'; import 'package:pinball/l10n/l10n.dart'; -import 'package:pinball/start_game/start_game.dart'; import 'package:platform_helper/platform_helper.dart'; -import '../../helpers/helpers.dart'; +import '../helpers/helpers.dart'; class MockPlatformHelper extends Mock implements PlatformHelper {} @@ -17,7 +15,7 @@ void main() { late PlatformHelper platformHelper; setUp(() async { - l10n = await AppLocalizations.delegate.load(Locale('en')); + l10n = await AppLocalizations.delegate.load(const Locale('en')); platformHelper = MockPlatformHelper(); }); @@ -40,7 +38,7 @@ void main() { expect(find.text(l10n.tipsForFlips), findsOneWidget); expect(find.text(l10n.launchControls), findsOneWidget); expect(find.text(l10n.flipperControls), findsOneWidget); - expect(find.byType(KeyButton), findsNWidgets(7)); + expect(find.byType(RotatedBox), findsNWidgets(7)); // controls }); testWidgets('displays content for mobile', (tester) async { @@ -76,11 +74,4 @@ void main() { expect(find.byType(HowToPlayDialog), findsNothing); }); }); - - group('KeyButton', () { - testWidgets('renders correctly', (tester) async { - await tester.pumpApp(KeyButton(control: Control.a)); - expect(find.text('A'), findsOneWidget); - }); - }); } diff --git a/test/select_character/view/character_selection_page_test.dart b/test/select_character/view/character_selection_page_test.dart index 9a94fdbd..b9c95f7f 100644 --- a/test/select_character/view/character_selection_page_test.dart +++ b/test/select_character/view/character_selection_page_test.dart @@ -2,8 +2,8 @@ import 'package:bloc_test/bloc_test.dart'; import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:mocktail/mocktail.dart'; +import 'package:pinball/how_to_play/how_to_play.dart'; import 'package:pinball/select_character/select_character.dart'; -import 'package:pinball/start_game/start_game.dart'; import 'package:pinball_theme/pinball_theme.dart'; import 'package:pinball_ui/pinball_ui.dart';