From 7d89617a92b7c008dbee6914cf04f67f36912b40 Mon Sep 17 00:00:00 2001 From: Taha Tesser Date: Fri, 28 Jul 2023 17:11:23 +0300 Subject: [PATCH] Fix `TimePicker` defaults for `hourMinuteTextStyle` and `dayPeriodTextColor` for Material 3 (#131253) fixes [`TimePicker` color and visual issues](https://github.com/flutter/flutter/issues/127035) ## Description - fixes default text style for `TimePicker`s `hourMinuteTextStyle` and added a todo for https://github.com/flutter/flutter/issues/131247 - fixes correct default color not being accessed for `dayPeriodTextColor` - Updates tests ### Code sample
expand to view the code sample ```dart import 'package:flutter/material.dart'; void main() => runApp(const MyApp()); class MyApp extends StatelessWidget { const MyApp({super.key}); @override Widget build(BuildContext context) { return MaterialApp( debugShowCheckedModeBanner: false, theme: ThemeData(useMaterial3: true), home: const Example(), ); } } class Example extends StatelessWidget { const Example({super.key}); @override Widget build(BuildContext context) { return Scaffold( appBar: AppBar( title: const Text('Sample'), ), body: Center( child: ElevatedButton( onPressed: () { showTimePicker( context: context, orientation: Orientation.portrait, initialEntryMode: TimePickerEntryMode.input, initialTime: TimeOfDay.now(), builder: (BuildContext context, Widget? child) { return MediaQuery( data: MediaQuery.of(context) .copyWith(alwaysUse24HourFormat: true), child: child!, ); }, ); }, child: const Text('Open Time Picker'), ), ), ); } } ```
### Before ![ezgif com-video-to-gif](https://github.com/flutter/flutter/assets/48603081/b791501f-aed3-44f3-8f75-70a1e28038c6) ### After ![ezgif com-video-to-gif (1)](https://github.com/flutter/flutter/assets/48603081/1bb32064-a9b1-416d-8290-7d22b0d4fdb9) --- .../gen_defaults/generated/used_tokens.csv | 2 +- .../lib/time_picker_template.dart | 57 +++++++------------ .../flutter/lib/src/material/time_picker.dart | 57 +++++++------------ .../test/material/time_picker_theme_test.dart | 18 +++--- 4 files changed, 54 insertions(+), 80 deletions(-) diff --git a/dev/tools/gen_defaults/generated/used_tokens.csv b/dev/tools/gen_defaults/generated/used_tokens.csv index 28ec7c57f4..c7eb2a390b 100644 --- a/dev/tools/gen_defaults/generated/used_tokens.csv +++ b/dev/tools/gen_defaults/generated/used_tokens.csv @@ -737,6 +737,7 @@ md.comp.time-picker.period-selector.selected.label-text.color, md.comp.time-picker.period-selector.selected.pressed.label-text.color, md.comp.time-picker.period-selector.unselected.focus.label-text.color, md.comp.time-picker.period-selector.unselected.hover.label-text.color, +md.comp.time-picker.period-selector.unselected.label-text.color, md.comp.time-picker.period-selector.unselected.pressed.label-text.color, md.comp.time-picker.period-selector.vertical.container.height, md.comp.time-picker.period-selector.vertical.container.width, @@ -746,7 +747,6 @@ md.comp.time-picker.time-selector.container.shape, md.comp.time-picker.time-selector.container.width, md.comp.time-picker.time-selector.focus.state-layer.opacity, md.comp.time-picker.time-selector.hover.state-layer.opacity, -md.comp.time-picker.time-selector.label-text.text-style, md.comp.time-picker.time-selector.selected.container.color, md.comp.time-picker.time-selector.selected.focus.label-text.color, md.comp.time-picker.time-selector.selected.focus.state-layer.color, diff --git a/dev/tools/gen_defaults/lib/time_picker_template.dart b/dev/tools/gen_defaults/lib/time_picker_template.dart index a10e5b35c5..af359acf6c 100644 --- a/dev/tools/gen_defaults/lib/time_picker_template.dart +++ b/dev/tools/gen_defaults/lib/time_picker_template.dart @@ -84,44 +84,28 @@ class _${blockName}DefaultsM3 extends _TimePickerDefaults { @override Color get dayPeriodTextColor { return MaterialStateColor.resolveWith((Set states) { - return _dayPeriodForegroundColor.resolve(states); - }); - } - - MaterialStateProperty get _dayPeriodForegroundColor { - return MaterialStateProperty.resolveWith((Set states) { - Color? textColor; if (states.contains(MaterialState.selected)) { - if (states.contains(MaterialState.pressed)) { - textColor = ${componentColor("$dayPeriodComponent.selected.pressed.label-text")}; - } else { - // not pressed - if (states.contains(MaterialState.hovered)) { - textColor = ${componentColor("$dayPeriodComponent.selected.hover.label-text")}; - } else { - // not hovered - if (states.contains(MaterialState.focused)) { - textColor = ${componentColor("$dayPeriodComponent.selected.focus.label-text")}; - } - } + if (states.contains(MaterialState.focused)) { + return ${componentColor("$dayPeriodComponent.selected.focus.label-text")}; } - } else { - // unselected - if (states.contains(MaterialState.pressed)) { - textColor = ${componentColor("$dayPeriodComponent.unselected.pressed.label-text")}; - } else { - // not pressed - if (states.contains(MaterialState.hovered)) { - textColor = ${componentColor("$dayPeriodComponent.unselected.hover.label-text")}; - } else { - // not hovered - if (states.contains(MaterialState.focused)) { - textColor = ${componentColor("$dayPeriodComponent.unselected.focus.label-text")}; - } - } + if (states.contains(MaterialState.hovered)) { + return ${componentColor("$dayPeriodComponent.selected.hover.label-text")}; } + if (states.contains(MaterialState.pressed)) { + return ${componentColor("$dayPeriodComponent.selected.pressed.label-text")}; + } + return ${componentColor("$dayPeriodComponent.selected.label-text")}; } - return textColor ?? ${componentColor("$dayPeriodComponent.selected.label-text")}; + if (states.contains(MaterialState.focused)) { + return ${componentColor("$dayPeriodComponent.unselected.focus.label-text")}; + } + if (states.contains(MaterialState.hovered)) { + return ${componentColor("$dayPeriodComponent.unselected.hover.label-text")}; + } + if (states.contains(MaterialState.pressed)) { + return ${componentColor("$dayPeriodComponent.unselected.pressed.label-text")}; + } + return ${componentColor("$dayPeriodComponent.unselected.label-text")}; }); } @@ -297,7 +281,10 @@ class _${blockName}DefaultsM3 extends _TimePickerDefaults { @override TextStyle get hourMinuteTextStyle { return MaterialStateTextStyle.resolveWith((Set states) { - return ${textStyle('$hourMinuteComponent.label-text')}!.copyWith(color: _hourMinuteTextColor.resolve(states)); + // TODO(tahatesser): Update this when https://github.com/flutter/flutter/issues/127035 is fixed. + // This is using the correct text style from Material 3 spec. + // https://m3.material.io/components/time-pickers/specs#fd0b6939-edab-4058-82e1-93d163945215 + return _textTheme.displayMedium!.copyWith(color: _hourMinuteTextColor.resolve(states)); }); } diff --git a/packages/flutter/lib/src/material/time_picker.dart b/packages/flutter/lib/src/material/time_picker.dart index 86ebd1f3da..7a4b526eb8 100644 --- a/packages/flutter/lib/src/material/time_picker.dart +++ b/packages/flutter/lib/src/material/time_picker.dart @@ -3393,44 +3393,28 @@ class _TimePickerDefaultsM3 extends _TimePickerDefaults { @override Color get dayPeriodTextColor { return MaterialStateColor.resolveWith((Set states) { - return _dayPeriodForegroundColor.resolve(states); - }); - } - - MaterialStateProperty get _dayPeriodForegroundColor { - return MaterialStateProperty.resolveWith((Set states) { - Color? textColor; if (states.contains(MaterialState.selected)) { - if (states.contains(MaterialState.pressed)) { - textColor = _colors.onTertiaryContainer; - } else { - // not pressed - if (states.contains(MaterialState.hovered)) { - textColor = _colors.onTertiaryContainer; - } else { - // not hovered - if (states.contains(MaterialState.focused)) { - textColor = _colors.onTertiaryContainer; - } - } + if (states.contains(MaterialState.focused)) { + return _colors.onTertiaryContainer; } - } else { - // unselected - if (states.contains(MaterialState.pressed)) { - textColor = _colors.onSurfaceVariant; - } else { - // not pressed - if (states.contains(MaterialState.hovered)) { - textColor = _colors.onSurfaceVariant; - } else { - // not hovered - if (states.contains(MaterialState.focused)) { - textColor = _colors.onSurfaceVariant; - } - } + if (states.contains(MaterialState.hovered)) { + return _colors.onTertiaryContainer; } + if (states.contains(MaterialState.pressed)) { + return _colors.onTertiaryContainer; + } + return _colors.onTertiaryContainer; } - return textColor ?? _colors.onTertiaryContainer; + if (states.contains(MaterialState.focused)) { + return _colors.onSurfaceVariant; + } + if (states.contains(MaterialState.hovered)) { + return _colors.onSurfaceVariant; + } + if (states.contains(MaterialState.pressed)) { + return _colors.onSurfaceVariant; + } + return _colors.onSurfaceVariant; }); } @@ -3606,7 +3590,10 @@ class _TimePickerDefaultsM3 extends _TimePickerDefaults { @override TextStyle get hourMinuteTextStyle { return MaterialStateTextStyle.resolveWith((Set states) { - return _textTheme.displayLarge!.copyWith(color: _hourMinuteTextColor.resolve(states)); + // TODO(tahatesser): Update this when https://github.com/flutter/flutter/issues/127035 is fixed. + // This is using the correct text style from Material 3 spec. + // https://m3.material.io/components/time-pickers/specs#fd0b6939-edab-4058-82e1-93d163945215 + return _textTheme.displayMedium!.copyWith(color: _hourMinuteTextColor.resolve(states)); }); } diff --git a/packages/flutter/test/material/time_picker_theme_test.dart b/packages/flutter/test/material/time_picker_theme_test.dart index 9f07d5e38f..f9a291a4c6 100644 --- a/packages/flutter/test/material/time_picker_theme_test.dart +++ b/packages/flutter/test/material/time_picker_theme_test.dart @@ -249,8 +249,8 @@ void main() { final RenderParagraph hourText = _textRenderParagraph(tester, '7'); expect( hourText.text.style, - Typography.material2021().englishLike.displayLarge! - .merge(Typography.material2021().black.displayLarge) + Typography.material2021().englishLike.displayMedium! + .merge(Typography.material2021().black.displayMedium) .copyWith( color: defaultTheme.colorScheme.onPrimaryContainer, decorationColor: defaultTheme.colorScheme.onSurface @@ -260,8 +260,8 @@ void main() { final RenderParagraph minuteText = _textRenderParagraph(tester, '15'); expect( minuteText.text.style, - Typography.material2021().englishLike.displayLarge! - .merge(Typography.material2021().black.displayLarge) + Typography.material2021().englishLike.displayMedium! + .merge(Typography.material2021().black.displayMedium) .copyWith( color: defaultTheme.colorScheme.onSurface, decorationColor: defaultTheme.colorScheme.onSurface @@ -275,7 +275,7 @@ void main() { .merge(Typography.material2021().black.titleMedium) .copyWith( color: defaultTheme.colorScheme.onTertiaryContainer, - decorationColor: defaultTheme.colorScheme.onSurface + decorationColor: defaultTheme.colorScheme.onSurface, ), ); @@ -285,8 +285,8 @@ void main() { Typography.material2021().englishLike.titleMedium! .merge(Typography.material2021().black.titleMedium) .copyWith( - color: defaultTheme.colorScheme.onTertiaryContainer, - decorationColor: defaultTheme.colorScheme.onSurface + color: defaultTheme.colorScheme.onSurfaceVariant, + decorationColor: defaultTheme.colorScheme.onSurface, ) ); @@ -297,7 +297,7 @@ void main() { .merge(Typography.material2021().black.bodyMedium) .copyWith( color: defaultTheme.colorScheme.onSurface, - decorationColor: defaultTheme.colorScheme.onSurface + decorationColor: defaultTheme.colorScheme.onSurface, ), ); @@ -312,7 +312,7 @@ void main() { .merge(Typography.material2021().black.bodyLarge) .copyWith( color: defaultTheme.colorScheme.onSurface, - decorationColor: defaultTheme.colorScheme.onSurface + decorationColor: defaultTheme.colorScheme.onSurface, ), ); // ignore: avoid_dynamic_calls