From e08177155e6aa9e2d16f38d93690696bed185817 Mon Sep 17 00:00:00 2001 From: xster Date: Mon, 9 Jul 2018 12:27:24 -0700 Subject: [PATCH] Put the nav bar padding inside the CustomMultiChildLayout for correct centering and allow custom padding (#19129) --- .../flutter/lib/src/cupertino/nav_bar.dart | 100 ++++++++++++++---- .../flutter/test/cupertino/nav_bar_test.dart | 97 +++++++++++++++++ 2 files changed, 174 insertions(+), 23 deletions(-) diff --git a/packages/flutter/lib/src/cupertino/nav_bar.dart b/packages/flutter/lib/src/cupertino/nav_bar.dart index dc91c614d5..4a5017db8d 100644 --- a/packages/flutter/lib/src/cupertino/nav_bar.dart +++ b/packages/flutter/lib/src/cupertino/nav_bar.dart @@ -26,9 +26,6 @@ const double _kNavBarShowLargeTitleThreshold = 10.0; const double _kNavBarEdgePadding = 16.0; -// The back chevron has a special padding in iOS. -const double _kNavBarBackButtonPadding = 0.0; - const double _kNavBarBackButtonTapWidth = 50.0; /// Title text transfer fade. @@ -87,6 +84,7 @@ class CupertinoNavigationBar extends StatelessWidget implements ObstructingPrefe this.trailing, this.border = _kDefaultNavBarBorder, this.backgroundColor = _kDefaultNavBarBackgroundColor, + this.padding, this.actionsForegroundColor = CupertinoColors.activeBlue, }) : assert(automaticallyImplyLeading != null), super(key: key); @@ -118,6 +116,19 @@ class CupertinoNavigationBar extends StatelessWidget implements ObstructingPrefe /// behind it. final Color backgroundColor; + /// Padding for the contents of the navigation bar. + /// + /// If null, the navigation bar will adopt the following defaults: + /// + /// * Vertically, contents will be sized to the same height as the navigation + /// bar itself minus the status bar. + /// * Horizontally, padding will be 16 pixels according to iOS specifications + /// unless the leading widget is an automatically inserted back button, in + /// which case the padding will be 0. + /// + /// Vertical padding won't change the height of the nav bar. + final EdgeInsetsDirectional padding; + /// The border of the navigation bar. By default renders a single pixel bottom border side. /// /// If a border is null, the navigation bar will not display a border. @@ -149,6 +160,7 @@ class CupertinoNavigationBar extends StatelessWidget implements ObstructingPrefe automaticallyImplyLeading: automaticallyImplyLeading, middle: new Semantics(child: middle, header: true), trailing: trailing, + padding: padding, actionsForegroundColor: actionsForegroundColor, ), ); @@ -197,6 +209,7 @@ class CupertinoSliverNavigationBar extends StatelessWidget { this.trailing, this.border = _kDefaultNavBarBorder, this.backgroundColor = _kDefaultNavBarBackgroundColor, + this.padding, this.actionsForegroundColor = CupertinoColors.activeBlue, }) : assert(largeTitle != null), assert(automaticallyImplyLeading != null), @@ -246,6 +259,19 @@ class CupertinoSliverNavigationBar extends StatelessWidget { /// This widget is visible in both collapsed and expanded states. final Widget trailing; + /// Padding for the contents of the navigation bar. + /// + /// If null, the navigation bar will adopt the following defaults: + /// + /// * Vertically, contents will be sized to the same height as the navigation + /// bar itself minus the status bar. + /// * Horizontally, padding will be 16 pixels according to iOS specifications + /// unless the leading widget is an automatically inserted back button, in + /// which case the padding will be 0. + /// + /// Vertical padding won't change the height of the nav bar. + final EdgeInsetsDirectional padding; + /// The border of the navigation bar. By default renders a single pixel bottom border side. /// /// If a border is null, the navigation bar will not display a border. @@ -277,6 +303,7 @@ class CupertinoSliverNavigationBar extends StatelessWidget { automaticallyImplyLeading: automaticallyImplyLeading, middle: middle, trailing: trailing, + padding: padding, border: border, backgroundColor: backgroundColor, actionsForegroundColor: actionsForegroundColor, @@ -332,6 +359,7 @@ class _CupertinoPersistentNavigationBar extends StatelessWidget implements Prefe this.automaticallyImplyLeading, this.middle, this.trailing, + this.padding, this.actionsForegroundColor, this.middleVisible, }) : super(key: key); @@ -344,6 +372,8 @@ class _CupertinoPersistentNavigationBar extends StatelessWidget implements Prefe final Widget trailing; + final EdgeInsetsDirectional padding; + final Color actionsForegroundColor; /// Whether the middle widget has a visible animated opacity. A null value @@ -362,15 +392,29 @@ class _CupertinoPersistentNavigationBar extends StatelessWidget implements Prefe color: actionsForegroundColor, ); - final Widget styledLeading = leading == null ? null : new DefaultTextStyle( - style: actionsStyle, - child: leading, - ); + final Widget styledLeading = leading == null + ? null + : new Padding( + padding: new EdgeInsetsDirectional.only( + start: padding?.start ?? _kNavBarEdgePadding, + ), + child: new DefaultTextStyle( + style: actionsStyle, + child: leading, + ), + ); - final Widget styledTrailing = trailing == null ? null : new DefaultTextStyle( - style: actionsStyle, - child: trailing, - ); + final Widget styledTrailing = trailing == null + ? null + : Padding( + padding: new EdgeInsetsDirectional.only( + end: padding?.end ?? _kNavBarEdgePadding, + ), + child: new DefaultTextStyle( + style: actionsStyle, + child: trailing, + ), + ); // Let the middle be black rather than `actionsForegroundColor` in case // it's a plain text title. @@ -413,6 +457,23 @@ class _CupertinoPersistentNavigationBar extends StatelessWidget implements Prefe } } + Widget paddedToolbar = new NavigationToolbar( + leading: styledLeading ?? backOrCloseButton, + middle: animatedStyledMiddle, + trailing: styledTrailing, + centerMiddle: true, + ); + + if (padding != null) { + paddedToolbar = new Padding( + padding: EdgeInsets.only( + top: padding.top, + bottom: padding.bottom, + ), + child: paddedToolbar, + ); + } + return new SizedBox( height: _kNavBarPersistentHeight + MediaQuery.of(context).padding.top, child: IconTheme.merge( @@ -422,18 +483,7 @@ class _CupertinoPersistentNavigationBar extends StatelessWidget implements Prefe ), child: new SafeArea( bottom: false, - child: new Padding( - padding: new EdgeInsetsDirectional.only( - start: useBackButton ? _kNavBarBackButtonPadding : _kNavBarEdgePadding, - end: _kNavBarEdgePadding, - ), - child: new NavigationToolbar( - leading: styledLeading ?? backOrCloseButton, - middle: animatedStyledMiddle, - trailing: styledTrailing, - centerMiddle: true, - ), - ), + child: paddedToolbar, ), ), ); @@ -449,6 +499,7 @@ class _CupertinoLargeTitleNavigationBarSliverDelegate this.automaticallyImplyLeading, this.middle, this.trailing, + this.padding, this.border, this.backgroundColor, this.actionsForegroundColor, @@ -466,6 +517,8 @@ class _CupertinoLargeTitleNavigationBarSliverDelegate final Widget trailing; + final EdgeInsetsDirectional padding; + final Color backgroundColor; final Border border; @@ -491,6 +544,7 @@ class _CupertinoLargeTitleNavigationBarSliverDelegate // If middle widget exists, always show it. Otherwise, show title // when collapsed. middleVisible: middle != null ? null : !showLargeTitle, + padding: padding, actionsForegroundColor: actionsForegroundColor, ); diff --git a/packages/flutter/test/cupertino/nav_bar_test.dart b/packages/flutter/test/cupertino/nav_bar_test.dart index 1d4f4b39fc..47e01fb5a8 100644 --- a/packages/flutter/test/cupertino/nav_bar_test.dart +++ b/packages/flutter/test/cupertino/nav_bar_test.dart @@ -28,6 +28,30 @@ void main() { expect(tester.getCenter(find.text('Title')).dx, 400.0); }); + testWidgets('Middle still in center with back button', (WidgetTester tester) async { + await tester.pumpWidget( + new CupertinoApp( + home: const CupertinoNavigationBar( + middle: const Text('Title'), + ), + ), + ); + + tester.state(find.byType(Navigator)).push(new CupertinoPageRoute( + builder: (BuildContext context) { + return const CupertinoNavigationBar( + middle: const Text('Page 2'), + ); + }, + )); + + await tester.pump(); + await tester.pump(const Duration(milliseconds: 500)); + + // Expect the middle of the title to be exactly in the middle of the screen. + expect(tester.getCenter(find.text('Page 2')).dx, 400.0); + }); + testWidgets('Opaque background does not add blur effects', (WidgetTester tester) async { await tester.pumpWidget( new CupertinoApp( @@ -51,6 +75,79 @@ void main() { expect(find.byType(BackdropFilter), findsOneWidget); }); + testWidgets('Can specify custom padding', (WidgetTester tester) async { + final Key middleBox = new GlobalKey(); + await tester.pumpWidget( + new CupertinoApp( + home: new Align( + alignment: Alignment.topCenter, + child: new CupertinoNavigationBar( + leading: const CupertinoButton(child: const Text('Cheetah'), onPressed: null), + // Let the box take all the vertical space to test vertical padding but let + // the nav bar position it horizontally. + middle: new Align( + key: middleBox, + alignment: Alignment.center, + widthFactor: 1.0, + child: const Text('Title') + ), + trailing: const CupertinoButton(child: const Text('Puma'), onPressed: null), + padding: const EdgeInsetsDirectional.only( + start: 10.0, + end: 20.0, + top: 3.0, + bottom: 4.0, + ), + ), + ), + ), + ); + + expect(tester.getRect(find.byKey(middleBox)).top, 3.0); + // 44 is the standard height of the nav bar. + expect( + tester.getRect(find.byKey(middleBox)).bottom, + // 44 is the standard height of the nav bar. + 44.0 - 4.0, + ); + + expect(tester.getTopLeft(find.widgetWithText(CupertinoButton, 'Cheetah')).dx, 10.0); + expect(tester.getTopRight(find.widgetWithText(CupertinoButton, 'Puma')).dx, 800.0 - 20.0); + + // Title is still exactly centered. + expect(tester.getCenter(find.text('Title')).dx, 400.0); + }); + + testWidgets('Padding works in RTL', (WidgetTester tester) async { + await tester.pumpWidget( + new CupertinoApp( + home: const Directionality( + textDirection: TextDirection.rtl, + child: const Align( + alignment: Alignment.topCenter, + child: const CupertinoNavigationBar( + leading: const CupertinoButton(child: const Text('Cheetah'), onPressed: null), + // Let the box take all the vertical space to test vertical padding but let + // the nav bar position it horizontally. + middle: const Text('Title'), + trailing: const CupertinoButton(child: const Text('Puma'), onPressed: null), + padding: const EdgeInsetsDirectional.only( + start: 10.0, + end: 20.0, + ), + ), + ), + ), + ), + ); + + expect(tester.getTopRight(find.widgetWithText(CupertinoButton, 'Cheetah')).dx, 800.0 - 10.0); + expect(tester.getTopLeft(find.widgetWithText(CupertinoButton, 'Puma')).dx, 20.0); + + // Title is still exactly centered. + expect(tester.getCenter(find.text('Title')).dx, 400.0); + }); + testWidgets('Verify styles of each slot', (WidgetTester tester) async { count = 0x000000; await tester.pumpWidget(