Unverified Commit 7d89617a authored by Taha Tesser's avatar Taha Tesser Committed by GitHub

Fix `TimePicker` defaults for `hourMinuteTextStyle` and `dayPeriodTextColor`...

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

<details> 
<summary>expand to view the code sample</summary> 

```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'),
        ),
      ),
    );
  }
}

``` 
	
</details>

### 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)
parent e4ce8e05
...@@ -737,6 +737,7 @@ md.comp.time-picker.period-selector.selected.label-text.color, ...@@ -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.selected.pressed.label-text.color,
md.comp.time-picker.period-selector.unselected.focus.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.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.unselected.pressed.label-text.color,
md.comp.time-picker.period-selector.vertical.container.height, md.comp.time-picker.period-selector.vertical.container.height,
md.comp.time-picker.period-selector.vertical.container.width, md.comp.time-picker.period-selector.vertical.container.width,
...@@ -746,7 +747,6 @@ md.comp.time-picker.time-selector.container.shape, ...@@ -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.container.width,
md.comp.time-picker.time-selector.focus.state-layer.opacity, 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.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.container.color,
md.comp.time-picker.time-selector.selected.focus.label-text.color, md.comp.time-picker.time-selector.selected.focus.label-text.color,
md.comp.time-picker.time-selector.selected.focus.state-layer.color, md.comp.time-picker.time-selector.selected.focus.state-layer.color,
......
...@@ -84,44 +84,28 @@ class _${blockName}DefaultsM3 extends _TimePickerDefaults { ...@@ -84,44 +84,28 @@ class _${blockName}DefaultsM3 extends _TimePickerDefaults {
@override @override
Color get dayPeriodTextColor { Color get dayPeriodTextColor {
return MaterialStateColor.resolveWith((Set<MaterialState> states) { return MaterialStateColor.resolveWith((Set<MaterialState> states) {
return _dayPeriodForegroundColor.resolve(states);
});
}
MaterialStateProperty<Color> get _dayPeriodForegroundColor {
return MaterialStateProperty.resolveWith((Set<MaterialState> states) {
Color? textColor;
if (states.contains(MaterialState.selected)) { if (states.contains(MaterialState.selected)) {
if (states.contains(MaterialState.pressed)) { if (states.contains(MaterialState.focused)) {
textColor = ${componentColor("$dayPeriodComponent.selected.pressed.label-text")}; return ${componentColor("$dayPeriodComponent.selected.focus.label-text")};
} else { }
// not pressed if (states.contains(MaterialState.hovered)) {
if (states.contains(MaterialState.hovered)) { return ${componentColor("$dayPeriodComponent.selected.hover.label-text")};
textColor = ${componentColor("$dayPeriodComponent.selected.hover.label-text")};
} else {
// not hovered
if (states.contains(MaterialState.focused)) {
textColor = ${componentColor("$dayPeriodComponent.selected.focus.label-text")};
}
}
} }
} else {
// unselected
if (states.contains(MaterialState.pressed)) { if (states.contains(MaterialState.pressed)) {
textColor = ${componentColor("$dayPeriodComponent.unselected.pressed.label-text")}; return ${componentColor("$dayPeriodComponent.selected.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")};
}
}
} }
return ${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 textColor ?? ${componentColor("$dayPeriodComponent.selected.label-text")}; return ${componentColor("$dayPeriodComponent.unselected.label-text")};
}); });
} }
...@@ -297,7 +281,10 @@ class _${blockName}DefaultsM3 extends _TimePickerDefaults { ...@@ -297,7 +281,10 @@ class _${blockName}DefaultsM3 extends _TimePickerDefaults {
@override @override
TextStyle get hourMinuteTextStyle { TextStyle get hourMinuteTextStyle {
return MaterialStateTextStyle.resolveWith((Set<MaterialState> states) { return MaterialStateTextStyle.resolveWith((Set<MaterialState> 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));
}); });
} }
......
...@@ -3393,44 +3393,28 @@ class _TimePickerDefaultsM3 extends _TimePickerDefaults { ...@@ -3393,44 +3393,28 @@ class _TimePickerDefaultsM3 extends _TimePickerDefaults {
@override @override
Color get dayPeriodTextColor { Color get dayPeriodTextColor {
return MaterialStateColor.resolveWith((Set<MaterialState> states) { return MaterialStateColor.resolveWith((Set<MaterialState> states) {
return _dayPeriodForegroundColor.resolve(states);
});
}
MaterialStateProperty<Color> get _dayPeriodForegroundColor {
return MaterialStateProperty.resolveWith((Set<MaterialState> states) {
Color? textColor;
if (states.contains(MaterialState.selected)) { if (states.contains(MaterialState.selected)) {
if (states.contains(MaterialState.pressed)) { if (states.contains(MaterialState.focused)) {
textColor = _colors.onTertiaryContainer; return _colors.onTertiaryContainer;
} else { }
// not pressed if (states.contains(MaterialState.hovered)) {
if (states.contains(MaterialState.hovered)) { return _colors.onTertiaryContainer;
textColor = _colors.onTertiaryContainer;
} else {
// not hovered
if (states.contains(MaterialState.focused)) {
textColor = _colors.onTertiaryContainer;
}
}
} }
} else {
// unselected
if (states.contains(MaterialState.pressed)) { if (states.contains(MaterialState.pressed)) {
textColor = _colors.onSurfaceVariant; return _colors.onTertiaryContainer;
} else {
// not pressed
if (states.contains(MaterialState.hovered)) {
textColor = _colors.onSurfaceVariant;
} else {
// not hovered
if (states.contains(MaterialState.focused)) {
textColor = _colors.onSurfaceVariant;
}
}
} }
return _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 textColor ?? _colors.onTertiaryContainer; return _colors.onSurfaceVariant;
}); });
} }
...@@ -3606,7 +3590,10 @@ class _TimePickerDefaultsM3 extends _TimePickerDefaults { ...@@ -3606,7 +3590,10 @@ class _TimePickerDefaultsM3 extends _TimePickerDefaults {
@override @override
TextStyle get hourMinuteTextStyle { TextStyle get hourMinuteTextStyle {
return MaterialStateTextStyle.resolveWith((Set<MaterialState> states) { return MaterialStateTextStyle.resolveWith((Set<MaterialState> 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));
}); });
} }
......
...@@ -249,8 +249,8 @@ void main() { ...@@ -249,8 +249,8 @@ void main() {
final RenderParagraph hourText = _textRenderParagraph(tester, '7'); final RenderParagraph hourText = _textRenderParagraph(tester, '7');
expect( expect(
hourText.text.style, hourText.text.style,
Typography.material2021().englishLike.displayLarge! Typography.material2021().englishLike.displayMedium!
.merge(Typography.material2021().black.displayLarge) .merge(Typography.material2021().black.displayMedium)
.copyWith( .copyWith(
color: defaultTheme.colorScheme.onPrimaryContainer, color: defaultTheme.colorScheme.onPrimaryContainer,
decorationColor: defaultTheme.colorScheme.onSurface decorationColor: defaultTheme.colorScheme.onSurface
...@@ -260,8 +260,8 @@ void main() { ...@@ -260,8 +260,8 @@ void main() {
final RenderParagraph minuteText = _textRenderParagraph(tester, '15'); final RenderParagraph minuteText = _textRenderParagraph(tester, '15');
expect( expect(
minuteText.text.style, minuteText.text.style,
Typography.material2021().englishLike.displayLarge! Typography.material2021().englishLike.displayMedium!
.merge(Typography.material2021().black.displayLarge) .merge(Typography.material2021().black.displayMedium)
.copyWith( .copyWith(
color: defaultTheme.colorScheme.onSurface, color: defaultTheme.colorScheme.onSurface,
decorationColor: defaultTheme.colorScheme.onSurface decorationColor: defaultTheme.colorScheme.onSurface
...@@ -275,7 +275,7 @@ void main() { ...@@ -275,7 +275,7 @@ void main() {
.merge(Typography.material2021().black.titleMedium) .merge(Typography.material2021().black.titleMedium)
.copyWith( .copyWith(
color: defaultTheme.colorScheme.onTertiaryContainer, color: defaultTheme.colorScheme.onTertiaryContainer,
decorationColor: defaultTheme.colorScheme.onSurface decorationColor: defaultTheme.colorScheme.onSurface,
), ),
); );
...@@ -285,8 +285,8 @@ void main() { ...@@ -285,8 +285,8 @@ void main() {
Typography.material2021().englishLike.titleMedium! Typography.material2021().englishLike.titleMedium!
.merge(Typography.material2021().black.titleMedium) .merge(Typography.material2021().black.titleMedium)
.copyWith( .copyWith(
color: defaultTheme.colorScheme.onTertiaryContainer, color: defaultTheme.colorScheme.onSurfaceVariant,
decorationColor: defaultTheme.colorScheme.onSurface decorationColor: defaultTheme.colorScheme.onSurface,
) )
); );
...@@ -297,7 +297,7 @@ void main() { ...@@ -297,7 +297,7 @@ void main() {
.merge(Typography.material2021().black.bodyMedium) .merge(Typography.material2021().black.bodyMedium)
.copyWith( .copyWith(
color: defaultTheme.colorScheme.onSurface, color: defaultTheme.colorScheme.onSurface,
decorationColor: defaultTheme.colorScheme.onSurface decorationColor: defaultTheme.colorScheme.onSurface,
), ),
); );
...@@ -312,7 +312,7 @@ void main() { ...@@ -312,7 +312,7 @@ void main() {
.merge(Typography.material2021().black.bodyLarge) .merge(Typography.material2021().black.bodyLarge)
.copyWith( .copyWith(
color: defaultTheme.colorScheme.onSurface, color: defaultTheme.colorScheme.onSurface,
decorationColor: defaultTheme.colorScheme.onSurface decorationColor: defaultTheme.colorScheme.onSurface,
), ),
); );
// ignore: avoid_dynamic_calls // ignore: avoid_dynamic_calls
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment