app_bar [nfc]: Centralize _getEffectiveCenterTitle in our wrapper

chrisbobbe · chrisbobbe · commit 79ada37de72c · 2025-01-23T13:10:32.000-08:00
This logic is sort of complicated, and duplicated from upstream.
Better to put it centrally in our ZulipAppBar wrapper, instead of in
message_list.dart.

As a bonus, we also have it handle `actions` instead of assuming
there are none.
diff --git a/lib/widgets/app_bar.dart b/lib/widgets/app_bar.dart
@@ -6,14 +6,66 @@ import 'store.dart';
 ///
 /// This should be used for most of the pages with access to [PerAccountStore].
 class ZulipAppBar extends AppBar {
+  /// Creates our Zulip custom app bar based on [AppBar].
+  ///
+  /// [buildTitle] is passed a boolean `willCenterTitle` that answers
+  /// whether the underlying [AppBar] will decide to center [title]
+  /// based on [centerTitle], the theme, the platform, and [actions].
+  /// Useful if [title] is a container whose children should align the same way,
+  /// such as a [Column] with multiple lines of text.
+  // TODO(upstream) send a PR to replace our `willCenterTitle` code
   ZulipAppBar({
     super.key,
     super.titleSpacing,
-    required super.title,
+    Widget? title,
+    Widget Function(bool willCenterTitle)? buildTitle,
+    super.centerTitle,
     super.backgroundColor,
     super.shape,
     super.actions,
-  }) : super(bottom: _ZulipAppBarBottom(backgroundColor: backgroundColor));
+  }) :
+    assert((title == null) != (buildTitle == null)),
+    super(
+      bottom: _ZulipAppBarBottom(backgroundColor: backgroundColor),
+      title: title ?? _Title(centerTitle: centerTitle, actions: actions, buildTitle: buildTitle!)
+    );
+}
+
+class _Title extends StatelessWidget {
+  const _Title({
+    required this.centerTitle,
+    required this.actions,
+    required this.buildTitle,
+  });
+
+  final bool? centerTitle;
+  final List<Widget>? actions;
+  final Widget Function(bool centerTitle) buildTitle;
+
+  // A copy of [AppBar._getEffectiveCenterTitle].
+  bool _getEffectiveCenterTitle(ThemeData theme) {
+    bool platformCenter() {
+      switch (theme.platform) {
+        case TargetPlatform.android:
+        case TargetPlatform.fuchsia:
+        case TargetPlatform.linux:
+        case TargetPlatform.windows:
+          return false;
+        case TargetPlatform.iOS:
+        case TargetPlatform.macOS:
+          return actions == null || actions!.length < 2;
+      }
+    }
+
+    return centerTitle ?? theme.appBarTheme.centerTitle ?? platformCenter();
+  }
+
+  @override
+  Widget build(BuildContext context) {
+    final theme = Theme.of(context);
+    final willCenterTitle = _getEffectiveCenterTitle(theme);
+    return buildTitle(willCenterTitle);
+  }
 }
 
 class _ZulipAppBarBottom extends StatelessWidget implements PreferredSizeWidget {
diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart
@@ -269,8 +269,6 @@ class _MessageListPageState extends State<MessageListPage> implements MessageLis
 
     List<Widget>? actions;
     if (narrow case TopicNarrow(:final streamId)) {
-      // The helper [_getEffectiveCenterTitle] relies on the fact that we
-      // have at most one action here.
       (actions ??= []).add(IconButton(
         icon: const Icon(ZulipIcons.message_feed),
         tooltip: zulipLocalizations.channelFeedButtonTooltip,
@@ -281,7 +279,8 @@ class _MessageListPageState extends State<MessageListPage> implements MessageLis
 
     return Scaffold(
       appBar: ZulipAppBar(
-        title: MessageListAppBarTitle(narrow: narrow),
+        buildTitle: (willCenterTitle) =>
+          MessageListAppBarTitle(narrow: narrow, willCenterTitle: willCenterTitle),
         actions: actions,
         backgroundColor: appBarBackgroundColor,
         shape: removeAppBarBottomBorder
@@ -321,9 +320,14 @@ class _MessageListPageState extends State<MessageListPage> implements MessageLis
 }
 
 class MessageListAppBarTitle extends StatelessWidget {
-  const MessageListAppBarTitle({super.key, required this.narrow});
+  const MessageListAppBarTitle({
+    super.key,
+    required this.narrow,
+    required this.willCenterTitle,
+  });
 
   final Narrow narrow;
+  final bool willCenterTitle;
 
   Widget _buildStreamRow(BuildContext context, {
     ZulipStream? stream,
@@ -367,29 +371,6 @@ class MessageListAppBarTitle extends StatelessWidget {
       ]);
   }
 
-  // TODO(upstream): provide an API for this
-  // Adapted from [AppBar._getEffectiveCenterTitle].
-  bool _getEffectiveCenterTitle(ThemeData theme) {
-    bool platformCenter() {
-      switch (theme.platform) {
-        case TargetPlatform.android:
-        case TargetPlatform.fuchsia:
-        case TargetPlatform.linux:
-        case TargetPlatform.windows:
-          return false;
-        case TargetPlatform.iOS:
-        case TargetPlatform.macOS:
-        // We rely on the fact that there is at most one action
-        // on the message list app bar, so that the expression returned
-        // in the original helper, `actions == null || actions!.length < 2`,
-        // always evaluates to `true`:
-          return true;
-      }
-    }
-
-    return theme.appBarTheme.centerTitle ?? platformCenter();
-  }
-
   @override
   Widget build(BuildContext context) {
     final zulipLocalizations = ZulipLocalizations.of(context);
@@ -410,19 +391,17 @@ class MessageListAppBarTitle extends StatelessWidget {
         return _buildStreamRow(context, stream: stream);
 
       case TopicNarrow(:var streamId, :var topic):
-        final theme = Theme.of(context);
         final store = PerAccountStoreWidget.of(context);
         final stream = store.streams[streamId];
-        final centerTitle = _getEffectiveCenterTitle(theme);
         return SizedBox(
           width: double.infinity,
           child: GestureDetector(
             behavior: HitTestBehavior.translucent,
             onLongPress: () => showTopicActionSheet(context,
               channelId: streamId, topic: topic),
             child: Column(
-              crossAxisAlignment: centerTitle ? CrossAxisAlignment.center
-                                              : CrossAxisAlignment.start,
+              crossAxisAlignment: willCenterTitle ? CrossAxisAlignment.center
+                                                  : CrossAxisAlignment.start,
               children: [
                 _buildStreamRow(context, stream: stream),
                 _buildTopicRow(context, stream: stream, topic: topic),
diff --git a/test/widgets/app_bar_test.dart b/test/widgets/app_bar_test.dart
@@ -1,5 +1,7 @@
 import 'package:checks/checks.dart';
+import 'package:flutter/foundation.dart';
 import 'package:flutter/material.dart';
+import 'package:flutter_checks/flutter_checks.dart';
 import 'package:flutter_test/flutter_test.dart';
 import 'package:zulip/widgets/app_bar.dart';
 import 'package:zulip/widgets/profile.dart';
@@ -35,4 +37,124 @@ void main() {
     check(tester.getRect(find.byType(ZulipAppBar))).equals(rectBefore);
     check(finder.evaluate()).single;
   });
+
+  group("buildTitle's willCenterTitle agrees with Material AppBar", () {
+    /// Build an [AppBar]; inspect and return whether it decided to center.
+    Future<bool> material(WidgetTester tester, {
+      required bool? paramValue,
+      required bool? themeValue,
+      required List<Widget>? actions,
+    }) async {
+      testBinding.reset();
+
+      final themeData = ThemeData(appBarTheme: AppBarTheme(centerTitle: themeValue));
+      final widget = TestZulipApp(
+        child: Theme(data: themeData,
+          child: AppBar(
+            centerTitle: paramValue,
+            actions: actions,
+            title: const Text('a'))));
+
+      await tester.pumpWidget(widget);
+      await tester.pump();
+
+      // test assumes LTR text direction
+      check(tester.platformDispatcher.locale).equals(const Locale('en', 'US'));
+      assert(actions == null || actions.isNotEmpty);
+      final titleAreaRightEdgeOffset = actions == null
+        ? (tester.view.physicalSize / tester.view.devicePixelRatio).width
+        : tester.getTopLeft(find.byWidget(actions.first)).dx;
+      final titlePosition = tester.getTopLeft(find.text('a')).dx;
+      final isCentered = titlePosition > ((1 / 3) * titleAreaRightEdgeOffset);
+      check(titlePosition).isLessThan((2 / 3) * titleAreaRightEdgeOffset);
+
+      return isCentered;
+    }
+
+    /// Build a [ZulipAppBar]; return willCenterTitle from the buildTitle call.
+    Future<bool> ours(WidgetTester tester, {
+      required bool? paramValue,
+      required bool? themeValue,
+      required List<Widget>? actions,
+    }) async {
+      testBinding.reset();
+      await testBinding.globalStore.add(eg.selfAccount, eg.initialSnapshot());
+
+      bool? result;
+
+      final widget = TestZulipApp(
+        // ZulipAppBar expects a per-account context (for the loading indicator)
+        accountId: eg.selfAccount.id,
+        child: Builder(builder: (context) => Theme(
+          data: Theme.of(context).copyWith(appBarTheme: AppBarTheme(centerTitle: themeValue)),
+          child: ZulipAppBar(
+            centerTitle: paramValue,
+            actions: actions,
+            buildTitle: (willCenterTitle) {
+              result = willCenterTitle;
+              return const Text('a');
+            }))));
+
+      await tester.pumpWidget(widget);
+      await tester.pump(); // global store
+      await tester.pump(); // per-account store
+      check(find.widgetWithText(ZulipAppBar, 'a')).findsOne();
+
+      check(result).isNotNull();
+      return result!;
+    }
+
+    void doTest(String description, bool expectedWillCenter, {
+      bool? paramValue,
+      bool? themeValue,
+      TargetPlatform? platform,
+      List<Widget>? actions,
+    }) {
+      testWidgets(description, (tester) async {
+        addTearDown(testBinding.reset);
+        debugDefaultTargetPlatformOverride = platform;
+
+        check(
+          await ours(tester, paramValue: paramValue, themeValue: themeValue, actions: actions)
+        )..equals(
+          await material(tester, paramValue: paramValue, themeValue: themeValue, actions: actions)
+        )..equals(expectedWillCenter);
+
+        // TODO(upstream) Do this in an addTearDown, once we can:
+        //   https://github.com/flutter/flutter/issues/123189
+        debugDefaultTargetPlatformOverride = null;
+      });
+    }
+
+    const iOS = TargetPlatform.iOS;
+    const android = TargetPlatform.android;
+
+    Widget button() => IconButton(icon: const Icon(Icons.add), onPressed: () {});
+    final oneButton =    [button()];
+    final twoButtons =   [button(), button()];
+    final threeButtons = [button(), button(), button()];
+
+    doTest('ios',     true,  platform: iOS);
+    doTest('android', false, platform: android);
+
+    doTest('ios, theme false',    false, platform: iOS,     themeValue: false);
+    doTest('android, theme true', true,  platform: android, themeValue: true);
+
+    doTest('ios, param false',    false, platform: iOS,     paramValue: false);
+    doTest('android, param true', true,  platform: android, paramValue: true);
+
+    doTest('ios, theme true, param false', false,     platform: iOS,     themeValue: true,  paramValue: false);
+    doTest('ios, theme false, param true', true,      platform: iOS,     themeValue: false, paramValue: true);
+
+    doTest('android, theme true, param false', false, platform: android, themeValue: true,  paramValue: false);
+    doTest('android, theme false, param true', true,  platform: android, themeValue: false, paramValue: true);
+
+    doTest('ios, no actions',    true,  platform: iOS, actions: null);
+    doTest('ios, one action',    true,  platform: iOS, actions: oneButton);
+    doTest('ios, two actions' ,  false, platform: iOS, actions: twoButtons);
+    doTest('ios, three actions', false, platform: iOS, actions: threeButtons);
+
+    doTest('ios, two actions but param true', true, platform: iOS, paramValue: true, actions: twoButtons);
+    doTest('ios, two actions but theme true', true, platform: iOS, themeValue: true, actions: twoButtons);
+  });
 }
