From f7cca2211275e411359823e9913fd87c7c1818cc Mon Sep 17 00:00:00 2001 From: Jaime <52668514+jsgalarraga@users.noreply.github.com> Date: Thu, 30 May 2024 19:02:37 +0200 Subject: [PATCH] fix: leaderboard button (#545) * refactor: improve app bar layout * fix: leaderboard button not shown * fix: vertical position in app bar --- lib/crossword/view/crossword_page.dart | 14 +- lib/how_to_play/view/how_to_play_page.dart | 2 +- lib/initials/view/initials_page.dart | 2 +- lib/leaderboard/view/leaderboard_page.dart | 4 +- .../view/team_selection_page.dart | 2 +- lib/welcome/view/welcome_page.dart | 4 +- .../lib/src/theme/io_crossword_theme.dart | 5 +- .../lib/src/widgets/io_app_bar.dart | 121 ++++++++++++++---- .../test/src/widgets/io_app_bar_test.dart | 64 ++++----- 9 files changed, 135 insertions(+), 83 deletions(-) diff --git a/lib/crossword/view/crossword_page.dart b/lib/crossword/view/crossword_page.dart index e715f7c81..47a1668fa 100644 --- a/lib/crossword/view/crossword_page.dart +++ b/lib/crossword/view/crossword_page.dart @@ -143,15 +143,11 @@ class _CrosswordViewState extends State appBar: IoAppBar( title: const PlayerRankingInformation(), crossword: l10n.crossword, - actions: (context) { - return const Row( - children: [ - MuteButton(), - SizedBox(width: 7), - EndDrawerButton(), - ], - ); - }, + actions: const [ + MuteButton(), + SizedBox(width: 8), + EndDrawerButton(), + ], ), body: Stack( children: [ diff --git a/lib/how_to_play/view/how_to_play_page.dart b/lib/how_to_play/view/how_to_play_page.dart index 325d0f89f..fc21aa5e1 100644 --- a/lib/how_to_play/view/how_to_play_page.dart +++ b/lib/how_to_play/view/how_to_play_page.dart @@ -53,7 +53,7 @@ class HowToPlayView extends StatelessWidget { return Scaffold( appBar: IoAppBar( crossword: l10n.crossword, - actions: (context) => const MuteButton(), + actions: const [MuteButton()], ), body: switch (layout) { IoLayoutData.small => const _HowToPlaySmall(), diff --git a/lib/initials/view/initials_page.dart b/lib/initials/view/initials_page.dart index 6fff446d7..5c1fa7614 100644 --- a/lib/initials/view/initials_page.dart +++ b/lib/initials/view/initials_page.dart @@ -73,7 +73,7 @@ class _InitialsViewState extends State { child: Scaffold( appBar: IoAppBar( crossword: l10n.crossword, - actions: (context) => const MuteButton(), + actions: const [MuteButton()], ), body: SelectionArea( child: SingleChildScrollView( diff --git a/lib/leaderboard/view/leaderboard_page.dart b/lib/leaderboard/view/leaderboard_page.dart index 5d98cdb6a..50e20ae04 100644 --- a/lib/leaderboard/view/leaderboard_page.dart +++ b/lib/leaderboard/view/leaderboard_page.dart @@ -51,9 +51,7 @@ class LeaderboardView extends StatelessWidget { style: IoCrosswordTextStyles.mobile.h2, ), crossword: l10n.crossword, - actions: (context) { - return const CloseButton(); - }, + actions: const [CloseButton()], ), body: BlocBuilder( buildWhen: (previous, current) => previous.status != current.status, diff --git a/lib/team_selection/view/team_selection_page.dart b/lib/team_selection/view/team_selection_page.dart index 8342df240..59efb8b87 100644 --- a/lib/team_selection/view/team_selection_page.dart +++ b/lib/team_selection/view/team_selection_page.dart @@ -49,7 +49,7 @@ class TeamSelectionView extends StatelessWidget { return Scaffold( appBar: IoAppBar( crossword: l10n.crossword, - actions: (context) => const MuteButton(), + actions: const [MuteButton()], ), body: BlocConsumer( listenWhen: (previous, current) => previous.index != current.index, diff --git a/lib/welcome/view/welcome_page.dart b/lib/welcome/view/welcome_page.dart index 95b064445..b5a741958 100644 --- a/lib/welcome/view/welcome_page.dart +++ b/lib/welcome/view/welcome_page.dart @@ -55,7 +55,7 @@ class WelcomeLarge extends StatelessWidget { return Scaffold( appBar: IoAppBar( crossword: l10n.crossword, - actions: (context) => const MuteButton(), + actions: const [MuteButton()], ), body: SelectionArea( child: Container( @@ -93,7 +93,7 @@ class WelcomeSmall extends StatelessWidget { appBar: IoAppBar( crossword: l10n.crossword, bottom: const WelcomeHeaderImage(), - actions: (context) => const MuteButton(), + actions: const [MuteButton()], ), body: const SelectionArea( child: SingleChildScrollView( diff --git a/packages/io_crossword_ui/lib/src/theme/io_crossword_theme.dart b/packages/io_crossword_ui/lib/src/theme/io_crossword_theme.dart index ef71a0886..ab95df529 100644 --- a/packages/io_crossword_ui/lib/src/theme/io_crossword_theme.dart +++ b/packages/io_crossword_ui/lib/src/theme/io_crossword_theme.dart @@ -330,11 +330,10 @@ class IoCrosswordTheme { ), ), simpleBorder: OutlinedButton.styleFrom( - minimumSize: const Size(171, 56), + minimumSize: const Size(168, 56), foregroundColor: IoCrosswordColors.seedWhite, padding: const EdgeInsets.symmetric( - vertical: 17, - horizontal: 18, + horizontal: 16, ), ).copyWith( shape: WidgetStateProperty.resolveWith( diff --git a/packages/io_crossword_ui/lib/src/widgets/io_app_bar.dart b/packages/io_crossword_ui/lib/src/widgets/io_app_bar.dart index 2e60e7748..cef53806d 100644 --- a/packages/io_crossword_ui/lib/src/widgets/io_app_bar.dart +++ b/packages/io_crossword_ui/lib/src/widgets/io_app_bar.dart @@ -1,3 +1,5 @@ +import 'dart:math' as math; + import 'package:flutter/material.dart'; import 'package:io_crossword_ui/io_crossword_ui.dart'; @@ -20,7 +22,7 @@ class IoAppBar extends StatelessWidget implements PreferredSizeWidget { final String crossword; /// Display actions based on the [IoLayoutData] layout. - final WidgetBuilder? actions; + final List? actions; /// The title of the app bar. /// @@ -62,33 +64,47 @@ class IoAppBar extends StatelessWidget implements PreferredSizeWidget { child: switch (layout) { IoLayoutData.small => Padding( padding: const EdgeInsets.symmetric(horizontal: 32), - child: Row( - mainAxisAlignment: MainAxisAlignment.spaceBetween, + child: CustomMultiChildLayout( + delegate: AppBarLayout(), children: [ - if (title == null) - _IoCrosswordLogo( - crossword: crossword, - ) - else - titleWidget, - if (actions != null) actions(context), + LayoutId( + id: _AppBarAlignment.start, + child: title != null + ? titleWidget + : _IoCrosswordLogo(crossword: crossword), + ), + if (actions != null) + LayoutId( + id: _AppBarAlignment.end, + child: Row( + mainAxisSize: MainAxisSize.min, + children: actions, + ), + ), ], ), ), IoLayoutData.large => Padding( padding: const EdgeInsets.symmetric(horizontal: 32), - child: Stack( + child: CustomMultiChildLayout( + delegate: AppBarLayout(), children: [ - Center(child: titleWidget), - Row( - mainAxisAlignment: MainAxisAlignment.spaceBetween, - children: [ - _IoCrosswordLogo( - crossword: crossword, - ), - if (actions != null) actions(context), - ], + LayoutId( + id: _AppBarAlignment.start, + child: _IoCrosswordLogo(crossword: crossword), + ), + LayoutId( + id: _AppBarAlignment.center, + child: titleWidget, ), + if (actions != null) + LayoutId( + id: _AppBarAlignment.end, + child: Row( + mainAxisSize: MainAxisSize.min, + children: actions, + ), + ), ], ), ), @@ -127,16 +143,75 @@ class _IoCrosswordLogo extends StatelessWidget { @override Widget build(BuildContext context) { return Row( + mainAxisSize: MainAxisSize.min, children: [ const IoLogo(), const SizedBox(width: 5), Text( crossword, - style: Theme.of(context).io.textStyles.h2.copyWith( - fontWeight: FontWeight.w700, - ), + style: Theme.of(context).io.textStyles.h2.bold, ), ], ); } } + +enum _AppBarAlignment { + start, + center, + end, +} + +/// Layout delegate that positions 3 widgets along a horizontal axis in order to +/// keep the middle widget centered and leading and trailing in the left and +/// right side of the screen respectively, independently of which of the 3 +/// widgets are present. +@visibleForTesting +class AppBarLayout extends MultiChildLayoutDelegate { + /// The default spacing around the middle widget. + static const double kMiddleSpacing = 16; + + @override + void performLayout(Size size) { + var leadingWidth = 0.0; + var trailingWidth = 0.0; + + if (hasChild(_AppBarAlignment.start)) { + final constraints = BoxConstraints.loose(size); + final leadingSize = layoutChild(_AppBarAlignment.start, constraints); + const leadingX = 0.0; + final leadingY = (size.height - leadingSize.height) / 2; + leadingWidth = leadingSize.width; + positionChild(_AppBarAlignment.start, Offset(leadingX, leadingY)); + } + + if (hasChild(_AppBarAlignment.end)) { + final constraints = BoxConstraints.loose(size); + final trailingSize = layoutChild(_AppBarAlignment.end, constraints); + + final trailingX = size.width - trailingSize.width; + final trailingY = (size.height - trailingSize.height) / 2; + + trailingWidth = trailingSize.width; + positionChild(_AppBarAlignment.end, Offset(trailingX, trailingY)); + } + + if (hasChild(_AppBarAlignment.center)) { + final double maxWidth = math.max( + size.width - leadingWidth - trailingWidth - kMiddleSpacing * 2, + 0, + ); + final constraints = + BoxConstraints.loose(size).copyWith(maxWidth: maxWidth); + final middleSize = layoutChild(_AppBarAlignment.center, constraints); + + final middleX = (size.width - middleSize.width) / 2; + final middleY = (size.height - middleSize.height) / 2; + + positionChild(_AppBarAlignment.center, Offset(middleX, middleY)); + } + } + + @override + bool shouldRelayout(AppBarLayout oldDelegate) => false; +} diff --git a/packages/io_crossword_ui/test/src/widgets/io_app_bar_test.dart b/packages/io_crossword_ui/test/src/widgets/io_app_bar_test.dart index 3004e1439..c1c4bb2c9 100644 --- a/packages/io_crossword_ui/test/src/widgets/io_app_bar_test.dart +++ b/packages/io_crossword_ui/test/src/widgets/io_app_bar_test.dart @@ -26,7 +26,7 @@ void main() { expect( IoAppBar( crossword: 'Crossword', - actions: (_) => SizedBox(), + actions: const [SizedBox()], title: Text('Title'), ).preferredSize, equals(Size(double.infinity, 80)), @@ -40,7 +40,7 @@ void main() { expect( IoAppBar( crossword: 'Crossword', - actions: (_) => SizedBox(), + actions: const [SizedBox()], bottom: _BottomWidget(), title: Text('Title'), ).preferredSize, @@ -55,7 +55,7 @@ void main() { await tester.pumpApp( IoAppBar( crossword: 'Crossword', - actions: (_) => SizedBox(), + actions: const [SizedBox()], title: Text('Title'), ), layout: IoLayoutData.small, @@ -71,7 +71,7 @@ void main() { await tester.pumpSubject( IoAppBar( crossword: 'Crossword', - actions: (_) => SizedBox(), + actions: const [SizedBox()], title: null, ), layout: IoLayoutData.small, @@ -87,7 +87,7 @@ void main() { await tester.pumpSubject( IoAppBar( crossword: 'Crossword', - actions: (_) => SizedBox(), + actions: const [SizedBox()], title: Text('Title'), ), layout: IoLayoutData.large, @@ -104,7 +104,7 @@ void main() { await tester.pumpSubject( IoAppBar( crossword: 'Crossword', - actions: (_) => SizedBox(), + actions: const [SizedBox()], title: Text('Title'), ), layout: layout, @@ -122,7 +122,7 @@ void main() { await tester.pumpSubject( IoAppBar( crossword: 'Crossword', - actions: (_) => SizedBox(), + actions: const [SizedBox()], title: Text('Title'), bottom: _BottomWidget(), ), @@ -135,52 +135,36 @@ void main() { } testWidgets( - 'display actions based on large layout', + 'display all actions', (tester) async { await tester.pumpSubject( IoAppBar( crossword: 'Crossword', - actions: (context) { - final layout = IoLayout.of(context); - - return switch (layout) { - IoLayoutData.small => Text('Small'), - IoLayoutData.large => Text('Large'), - }; - }, + actions: const [ + Text('Action 1'), + Text('Action 2'), + ], title: Text('Title'), ), layout: IoLayoutData.large, ); - expect(find.text('Large'), findsOneWidget); - expect(find.text('Small'), findsNothing); + expect(find.text('Action 1'), findsOneWidget); + expect(find.text('Action 2'), findsOneWidget); }, ); + }); - testWidgets( - 'display actions based on large layout', - (tester) async { - await tester.pumpSubject( - IoAppBar( - crossword: 'Crossword', - actions: (context) { - final layout = IoLayout.of(context); - - return switch (layout) { - IoLayoutData.small => Text('Small'), - IoLayoutData.large => Text('Large'), - }; - }, - title: Text('Title'), - ), - layout: IoLayoutData.small, - ); + group('AppBarLayout', () { + testWidgets('shouldRelayout method returns false', (tester) async { + final delegate = AppBarLayout(); + final delegate2 = AppBarLayout(); + await tester.pumpWidget( + CustomMultiChildLayout(delegate: delegate), + ); - expect(find.text('Small'), findsOneWidget); - expect(find.text('Large'), findsNothing); - }, - ); + expect(delegate.shouldRelayout(delegate2), isFalse); + }); }); }