forked from zulip/zulip-flutter
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
ui: Set letter spacing to 1% in buttons
It's a bit annoying that we have to repeat all of elevatedButtonTheme filledButtonTheme menuButtonTheme outlinedButtonTheme segmentedButtonTheme textButtonTheme but anyway, here we are; I don't think I've left any out. [ThemeData.buttonTheme] *sounds* like it might be one central place to define button text's letter spacing...but on a closer look, it doesn't allow setting letter spacing, and moreover it's deprecated, with these various FooButtonThemes meant to replace it. Empirically, the mark-all-as-read button (a [FilledButton.icon]) ignores the TextStyle in the ambient FilledButtonThemeData unless explicitly told to use it; that's because we use the `textStyle` param. So, explicitly merge that in, and adjust some tests (action_sheet_test, autocomplete_test, message_list_test) that weren't providing the FilledButtonThemeData so that they do provide it. ...hmm, but for that specific button, the Figma asks for a specific font size, which differs from the default `labelLarge`. That means we need to set `letterSpacing` right alongside `fontSize`, because the letter spacing should be proportional to the font size. Having done so, the restatement of the TextStyle in FilledButtonThemeData is now completely redundant. Shrug...I guess... if we add to those theme styles, we'll want them to be merged in, as done here, and that seems really easy to forget to do. Fixes: zulip#548
- Loading branch information
1 parent
dd97594
commit 3f822eb
Showing
8 changed files
with
190 additions
and
39 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,112 @@ | ||
| import 'package:checks/checks.dart'; | ||
| import 'package:flutter/material.dart'; | ||
| import 'package:flutter/rendering.dart'; | ||
| import 'package:flutter_test/flutter_test.dart'; | ||
| import 'package:zulip/widgets/text.dart'; | ||
| import 'package:zulip/widgets/theme.dart'; | ||
|
|
||
| import '../flutter_checks.dart'; | ||
|
|
||
| void main() { | ||
| group('innerThemeData', () { | ||
| testWidgets('smoke', (tester) async { | ||
| ThemeData? outerThemeDataValue; | ||
| ThemeData? value; | ||
| await tester.pumpWidget( | ||
| Builder(builder: (context) => MaterialApp( | ||
| theme: outerThemeData(context), | ||
| home: Builder(builder: (context) { | ||
| outerThemeDataValue = Theme.of(context); | ||
| return Theme( | ||
| data: innerThemeData(context), | ||
| child: Builder(builder: (context) { | ||
| value = Theme.of(context); | ||
| return const Placeholder(); | ||
| })); | ||
| })))); | ||
|
|
||
| final outerElevatedButtonLetterSpacing = outerThemeDataValue | ||
| !.elevatedButtonTheme.style?.textStyle?.resolve({})?.letterSpacing; | ||
| final elevatedButtonLetterSpacing = value | ||
| !.elevatedButtonTheme.style?.textStyle?.resolve({})?.letterSpacing; | ||
| check(outerElevatedButtonLetterSpacing).isNull(); | ||
| check(elevatedButtonLetterSpacing).isNotNull(); | ||
|
|
||
| // innerThemeData should extend outerThemeData using [ThemeData.copyWith] | ||
| // (at least for now, lacking a ThemeData.merge method). Pick a value that | ||
| // we set in outerThemeData and check that it hasn't been dropped. | ||
| check(value!.scaffoldBackgroundColor) | ||
| ..equals(outerThemeDataValue!.scaffoldBackgroundColor) | ||
| ..equals(const Color(0xfff6f6f6)); | ||
| }); | ||
|
|
||
| group('button text size and letter spacing', () { | ||
| Future<void> testButtonLetterSpacing( | ||
| String description, { | ||
| required Widget Function(BuildContext context, String text) buttonBuilder, | ||
| double? ambientTextScaleFactor, | ||
| }) async { | ||
| testWidgets(description, (WidgetTester tester) async { | ||
| if (ambientTextScaleFactor != null) { | ||
| tester.platformDispatcher.textScaleFactorTestValue = ambientTextScaleFactor; | ||
| addTearDown(tester.platformDispatcher.clearTextScaleFactorTestValue); | ||
| } | ||
| const buttonText = 'Zulip'; | ||
| double? expectedFontSize; | ||
| double? expectedLetterSpacing; | ||
| await tester.pumpWidget( | ||
| Builder(builder: (context) => MaterialApp( | ||
| theme: outerThemeData(context), | ||
| home: Builder(builder: (context) { | ||
| expectedFontSize = Theme.of(context).textTheme.labelLarge!.fontSize!; | ||
| expectedLetterSpacing = proportionalLetterSpacing(context, | ||
| 0.01, baseFontSize: expectedFontSize!); | ||
| return Theme(data: innerThemeData(context), | ||
| child: Builder(builder: (context) => | ||
| buttonBuilder(context, buttonText))); | ||
| })))); | ||
|
|
||
| final text = tester.renderObject<RenderParagraph>(find.text(buttonText)).text; | ||
| check(text.style!) | ||
| ..fontSize.equals(expectedFontSize) | ||
| ..letterSpacing.equals(expectedLetterSpacing); | ||
| }); | ||
| } | ||
|
|
||
| testButtonLetterSpacing('with device text size adjusted', | ||
| ambientTextScaleFactor: 2.0, | ||
| buttonBuilder: (context, text) => ElevatedButton(onPressed: () {}, | ||
| child: Text(text))); | ||
|
|
||
| testButtonLetterSpacing('ElevatedButton', | ||
| buttonBuilder: (context, text) => ElevatedButton(onPressed: () {}, | ||
| child: Text(text))); | ||
|
|
||
| testButtonLetterSpacing('FilledButton', | ||
| buttonBuilder: (context, text) => FilledButton(onPressed: () {}, | ||
| child: Text(text))); | ||
|
|
||
| // IconButton can't have text; skip | ||
|
|
||
| testButtonLetterSpacing('MenuItemButton', | ||
| buttonBuilder: (context, text) => MenuItemButton(onPressed: () {}, | ||
| child: Text(text))); | ||
|
|
||
| testButtonLetterSpacing('SubmenuButton', | ||
| buttonBuilder: (context, text) => SubmenuButton(menuChildren: const [], | ||
| child: Text(text))); | ||
|
|
||
| testButtonLetterSpacing('OutlinedButton', | ||
| buttonBuilder: (context, text) => OutlinedButton(onPressed: () {}, | ||
| child: Text(text))); | ||
|
|
||
| testButtonLetterSpacing('SegmentedButton', | ||
| buttonBuilder: (context, text) => SegmentedButton(selected: const {1}, | ||
| segments: [ButtonSegment(value: 1, label: Text(text))])); | ||
|
|
||
| testButtonLetterSpacing('TextButton', | ||
| buttonBuilder: (context, text) => TextButton(onPressed: () {}, | ||
| child: Text(text))); | ||
| }); | ||
| }); | ||
| } |