diff --git a/lib/widgets/content.dart b/lib/widgets/content.dart index ff671389f2..4ac92abaa3 100644 --- a/lib/widgets/content.dart +++ b/lib/widgets/content.dart @@ -39,6 +39,7 @@ class ContentTheme extends ThemeExtension { color: const HSLColor.fromAHSL(1, 0, 0, 0.15).toColor(), fontSize: kBaseFontSize, letterSpacing: 0, + textBaseline: localizedTextBaseline(context), height: (22 / kBaseFontSize), leadingDistribution: TextLeadingDistribution.even, decoration: TextDecoration.none, @@ -345,7 +346,7 @@ class ListItemWidget extends StatelessWidget { return Row( mainAxisAlignment: MainAxisAlignment.start, crossAxisAlignment: CrossAxisAlignment.baseline, - textBaseline: TextBaseline.alphabetic, + textBaseline: localizedTextBaseline(context), children: [ SizedBox( width: 20, // TODO handle long numbers in
    , like https://github.com/zulip/zulip/pull/25063 diff --git a/lib/widgets/message_list.dart b/lib/widgets/message_list.dart index 4131e9644e..b0783202a1 100644 --- a/lib/widgets/message_list.dart +++ b/lib/widgets/message_list.dart @@ -120,7 +120,10 @@ class MessageListAppBarTitle extends StatelessWidget { final Narrow narrow; - Widget _buildStreamRow(ZulipStream? stream, String text) { + Widget _buildStreamRow(BuildContext context, { + ZulipStream? stream, + required String text, + }) { // A null [Icon.icon] makes a blank space. final icon = (stream != null) ? iconDataForStream(stream) : null; return Row( @@ -128,11 +131,10 @@ class MessageListAppBarTitle extends StatelessWidget { // TODO(design): The vertical alignment of the stream privacy icon is a bit ad hoc. // For screenshots of some experiments, see: // https://github.com/zulip/zulip-flutter/pull/219#discussion_r1281024746 - crossAxisAlignment: CrossAxisAlignment.baseline, - textBaseline: TextBaseline.alphabetic, + crossAxisAlignment: CrossAxisAlignment.center, children: [ Icon(size: 16, icon), - const SizedBox(width: 8), + const SizedBox(width: 4), Flexible(child: Text(text)), ]); } @@ -149,13 +151,13 @@ class MessageListAppBarTitle extends StatelessWidget { final store = PerAccountStoreWidget.of(context); final stream = store.streams[streamId]; final streamName = stream?.name ?? '(unknown channel)'; - return _buildStreamRow(stream, streamName); + return _buildStreamRow(context, stream: stream, text: streamName); case TopicNarrow(:var streamId, :var topic): final store = PerAccountStoreWidget.of(context); final stream = store.streams[streamId]; final streamName = stream?.name ?? '(unknown channel)'; - return _buildStreamRow(stream, "$streamName > $topic"); + return _buildStreamRow(context, stream: stream, text: "$streamName > $topic"); case DmNarrow(:var otherRecipientIds): final store = PerAccountStoreWidget.of(context); @@ -899,7 +901,7 @@ class MessageWithPossibleSender extends StatelessWidget { senderRow = Row( mainAxisAlignment: MainAxisAlignment.spaceBetween, crossAxisAlignment: CrossAxisAlignment.baseline, - textBaseline: TextBaseline.alphabetic, + textBaseline: localizedTextBaseline(context), children: [ Flexible( child: GestureDetector( diff --git a/lib/widgets/profile.dart b/lib/widgets/profile.dart index 80a08b952e..afadc289b0 100644 --- a/lib/widgets/profile.dart +++ b/lib/widgets/profile.dart @@ -195,7 +195,7 @@ class _ProfileDataTable extends StatelessWidget { items.add(Row( crossAxisAlignment: CrossAxisAlignment.baseline, - textBaseline: TextBaseline.alphabetic, + textBaseline: localizedTextBaseline(context), children: [ SizedBox(width: 100, child: Text(style: _TextStyles.customProfileFieldLabel(context), diff --git a/lib/widgets/text.dart b/lib/widgets/text.dart index 3f4389e592..08957170e3 100644 --- a/lib/widgets/text.dart +++ b/lib/widgets/text.dart @@ -12,12 +12,13 @@ import 'package:flutter/material.dart'; /// an [AppBar]'s title, of an [ElevatedButton]'s label, and so on. /// /// As of writing, it turns out that these styles also flow naturally into -/// most of our own widgets' text styles. +/// many of our own widgets' text styles. /// We often see this in the child of a [Material], for example, /// since by default [Material] applies an [AnimatedDefaultTextStyle] /// with the [TextTheme.bodyMedium] that gets its value from here. -/// A notable exception is the base style for message content. -/// That style is self-contained and is not meant to inherit from this; +/// There are exceptions, having `inherit: false`, that are self-contained +/// and not meant to inherit from this. +/// For example, the base style for message content; /// see [ContentTheme.textStylePlainParagraph]. /// /// Applies [kDefaultFontFamily] and [kDefaultFontFamilyFallback], @@ -373,3 +374,24 @@ double proportionalLetterSpacing( final effectiveTextScaler = textScaler ?? MediaQuery.textScalerOf(context); return effectiveTextScaler.scale(baseFontSize) * proportion; } + +/// The most suitable [TextBaseline] for the current language. +/// +/// The result is chosen based on information from [MaterialLocalizations] +/// about the language's script. +/// If [MaterialLocalizations] doesn't have that information, +/// gives [TextBaseline.alphabetic]. +// Adapted from [Theme.of], which localizes text styles according to the +// locale's [ScriptCategory]. With M3 defaults, this just means varying +// [TextStyle.textBaseline] the way we do here. +TextBaseline localizedTextBaseline(BuildContext context) { + final materialLocalizations = + Localizations.of(context, MaterialLocalizations); + final scriptCategory = materialLocalizations?.scriptCategory + ?? ScriptCategory.englishLike; + return switch (scriptCategory) { + ScriptCategory.dense => TextBaseline.ideographic, + ScriptCategory.englishLike => TextBaseline.alphabetic, + ScriptCategory.tall => TextBaseline.alphabetic, + }; +} diff --git a/lib/widgets/theme.dart b/lib/widgets/theme.dart index d7aad5cac2..c2e61cca98 100644 --- a/lib/widgets/theme.dart +++ b/lib/widgets/theme.dart @@ -4,20 +4,45 @@ import 'content.dart'; import 'text.dart'; ThemeData zulipThemeData(BuildContext context) { + final designVariables = DesignVariables(); return ThemeData( typography: zulipTypography(context), - extensions: [ContentTheme(context)], - appBarTheme: const AppBarTheme( + extensions: [ContentTheme(context), designVariables], + appBarTheme: AppBarTheme( // Set these two fields to prevent a color change in [AppBar]s when // there is something scrolled under it. If an app bar hasn't been // given a backgroundColor directly or by theme, it uses // ColorScheme.surfaceContainer for the scrolled-under state and // ColorScheme.surface otherwise, and those are different colors. scrolledUnderElevation: 0, - backgroundColor: Color(0xfff5f5f5), // `bg-top-bar` in Figma + backgroundColor: designVariables.bgTopBar, + + // TODO match layout to Figma + actionsIconTheme: IconThemeData( + color: designVariables.icon, + ), + + titleTextStyle: TextStyle( + inherit: false, + color: designVariables.title, + fontSize: 20, + letterSpacing: 0.0, + height: (30 / 20), + textBaseline: localizedTextBaseline(context), + leadingDistribution: TextLeadingDistribution.even, + decoration: TextDecoration.none, + fontFamily: kDefaultFontFamily, + fontFamilyFallback: defaultFontFamilyFallback, + ) + .merge(weightVariableTextStyle(context, wght: 600)), + titleSpacing: 4, + + // TODO Figma has height 42; we should try `toolbarHeight: 42` and test + // that it looks reasonable with different system text-size settings. + // Also the back button will look too big and need adjusting. shape: Border(bottom: BorderSide( - color: Color(0x33000000), // `border-bar` in Figma + color: designVariables.borderBar, strokeAlign: BorderSide.strokeAlignInside, // (default restated for explicitness) )), ), @@ -32,7 +57,7 @@ ThemeData zulipThemeData(BuildContext context) { colorScheme: ColorScheme.fromSeed( seedColor: kZulipBrandColor, ), - scaffoldBackgroundColor: const Color(0xfff0f0f0), // `bg-main` in Figma + scaffoldBackgroundColor: designVariables.bgMain, tooltipTheme: const TooltipThemeData(preferBelow: false), ); } @@ -42,3 +67,71 @@ ThemeData zulipThemeData(BuildContext context) { /// This is chosen as the sRGB midpoint of the Zulip logo's gradient. // As computed by Anders: https://github.com/zulip/zulip-mobile/pull/4467 const kZulipBrandColor = Color.fromRGBO(0x64, 0x92, 0xfe, 1); + +/// Variables from the Figma design. +/// +/// For how to export these from the Figma, see: +/// https://www.figma.com/design/1JTNtYo9memgW7vV6d0ygq/Zulip-Mobile?node-id=2945-49492&t=MEb4vtp7S26nntxm-0 +class DesignVariables extends ThemeExtension { + DesignVariables() : + bgMain = const Color(0xfff0f0f0), + bgTopBar = const Color(0xfff5f5f5), + borderBar = const Color(0x33000000), + icon = const Color(0xff666699), + title = const Color(0xff1a1a1a); + + DesignVariables._({ + required this.bgMain, + required this.bgTopBar, + required this.borderBar, + required this.icon, + required this.title, + }); + + /// The [DesignVariables] from the context's active theme. + /// + /// The [ThemeData] must include [DesignVariables] in [ThemeData.extensions]. + static DesignVariables of(BuildContext context) { + final theme = Theme.of(context); + final extension = theme.extension(); + assert(extension != null); + return extension!; + } + + final Color bgMain; + final Color bgTopBar; + final Color borderBar; + final Color icon; + final Color title; + + @override + DesignVariables copyWith({ + Color? bgMain, + Color? bgTopBar, + Color? borderBar, + Color? icon, + Color? title, + }) { + return DesignVariables._( + bgMain: bgMain ?? this.bgMain, + bgTopBar: bgTopBar ?? this.bgTopBar, + borderBar: borderBar ?? this.borderBar, + icon: icon ?? this.icon, + title: title ?? this.title, + ); + } + + @override + DesignVariables lerp(DesignVariables? other, double t) { + if (identical(this, other)) { + return this; + } + return DesignVariables._( + bgMain: Color.lerp(bgMain, other?.bgMain, t)!, + bgTopBar: Color.lerp(bgTopBar, other?.bgTopBar, t)!, + borderBar: Color.lerp(borderBar, other?.borderBar, t)!, + icon: Color.lerp(icon, other?.icon, t)!, + title: Color.lerp(title, other?.title, t)!, + ); + } +} diff --git a/test/widgets/text_test.dart b/test/widgets/text_test.dart index 824bc574eb..837f515124 100644 --- a/test/widgets/text_test.dart +++ b/test/widgets/text_test.dart @@ -2,9 +2,12 @@ import 'package:checks/checks.dart'; import 'package:collection/collection.dart'; import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; +import 'package:zulip/widgets/app.dart'; +import 'package:zulip/widgets/page.dart'; import 'package:zulip/widgets/text.dart'; import '../flutter_checks.dart'; +import '../model/binding.dart'; // From trying the options on an iPhone 13 Pro running iOS 16.6.1: const kTextScaleFactors = [ @@ -15,6 +18,8 @@ const kTextScaleFactors = [ ]; void main() { + TestZulipBinding.ensureInitialized(); + group('zulipTypography', () { Future getZulipTypography(WidgetTester tester, { required bool platformRequestsBold, @@ -382,4 +387,34 @@ void main() { expected: clampingTextScaler.scale(14) * 0.01); } }); + + group('localizedTextBaseline', () { + Future testLocalizedTextBaseline(Locale locale, TextBaseline expected) async { + testWidgets('gives $expected for $locale', (tester) async { + addTearDown(testBinding.reset); + tester.platformDispatcher.localeTestValue = locale; + tester.platformDispatcher.localesTestValue = [locale]; + addTearDown(tester.platformDispatcher.clearLocaleTestValue); + addTearDown(tester.platformDispatcher.clearLocalesTestValue); + + await tester.pumpWidget(const ZulipApp()); + await tester.pump(); + + final navigator = await ZulipApp.navigator; + navigator.push(MaterialWidgetRoute(page: Builder(builder: (context) => + Text('123', style: TextStyle(textBaseline: localizedTextBaseline(context)))))); + await tester.pumpAndSettle(); + + final TextStyle? style = tester.widget(find.text('123')).style; + final actualTextBaseline = style!.textBaseline!; + check(actualTextBaseline).equals(expected); + }); + } + + testLocalizedTextBaseline(const Locale('en'), TextBaseline.alphabetic); + testLocalizedTextBaseline(const Locale('ja'), TextBaseline.ideographic); + + // "und" is a special language code meaning undefined; see [Locale] + testLocalizedTextBaseline(const Locale('und'), TextBaseline.alphabetic); + }); }