Skip to content

Commit dd7b213

Browse files
Fix ChipThemeData lerp for BorderSide (flutter#173160)
In flutter#171945 (comment) we realised that some `lerp` method for `BorderSide` had a bug. This PR - Fixes a wrong interpolation of `ChipThemeData.lerp` for `side` in the case of `b` being `null` - Do some cleaning of other `lerp` of `BorderSide?` Fixes flutter#173161 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
1 parent 34c2a3b commit dd7b213

File tree

8 files changed

+71
-61
lines changed

8 files changed

+71
-61
lines changed

packages/flutter/lib/src/material/button_style.dart

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -776,7 +776,7 @@ class ButtonStyle with Diagnosticable {
776776
iconColor: MaterialStateProperty.lerp<Color?>(a?.iconColor, b?.iconColor, t, Color.lerp),
777777
iconSize: MaterialStateProperty.lerp<double?>(a?.iconSize, b?.iconSize, t, lerpDouble),
778778
iconAlignment: t < 0.5 ? a?.iconAlignment : b?.iconAlignment,
779-
side: _lerpSides(a?.side, b?.side, t),
779+
side: WidgetStateBorderSide.lerp(a?.side, b?.side, t),
780780
shape: MaterialStateProperty.lerp<OutlinedBorder?>(
781781
a?.shape,
782782
b?.shape,
@@ -794,16 +794,4 @@ class ButtonStyle with Diagnosticable {
794794
foregroundBuilder: t < 0.5 ? a?.foregroundBuilder : b?.foregroundBuilder,
795795
);
796796
}
797-
798-
// Special case because BorderSide.lerp() doesn't support null arguments
799-
static MaterialStateProperty<BorderSide?>? _lerpSides(
800-
MaterialStateProperty<BorderSide?>? a,
801-
MaterialStateProperty<BorderSide?>? b,
802-
double t,
803-
) {
804-
if (a == null && b == null) {
805-
return null;
806-
}
807-
return MaterialStateBorderSide.lerp(a, b, t);
808-
}
809797
}

packages/flutter/lib/src/material/checkbox_theme.dart

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -235,19 +235,19 @@ class CheckboxThemeData with Diagnosticable {
235235

236236
// Special case because BorderSide.lerp() doesn't support null arguments
237237
static BorderSide? _lerpSides(BorderSide? a, BorderSide? b, double t) {
238-
if (a == null || b == null) {
238+
if (a == null && b == null) {
239239
return null;
240240
}
241-
if (identical(a, b)) {
242-
return a;
243-
}
244-
if (a is MaterialStateBorderSide) {
245-
a = a.resolve(<WidgetState>{});
241+
if (a is WidgetStateBorderSide) {
242+
a = a.resolve(const <WidgetState>{});
246243
}
247-
if (b is MaterialStateBorderSide) {
248-
b = b.resolve(<WidgetState>{});
244+
if (b is WidgetStateBorderSide) {
245+
b = b.resolve(const <WidgetState>{});
249246
}
250-
return BorderSide.lerp(a!, b!, t);
247+
a ??= BorderSide(width: 0, color: b!.color.withAlpha(0));
248+
b ??= BorderSide(width: 0, color: a.color.withAlpha(0));
249+
250+
return BorderSide.lerp(a, b, t);
251251
}
252252
}
253253

packages/flutter/lib/src/material/chip_theme.dart

Lines changed: 8 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -539,7 +539,7 @@ class ChipThemeData with Diagnosticable {
539539
labelPadding: EdgeInsetsGeometry.lerp(a?.labelPadding, b?.labelPadding, t),
540540
padding: EdgeInsetsGeometry.lerp(a?.padding, b?.padding, t),
541541
side: _lerpSides(a?.side, b?.side, t),
542-
shape: _lerpShapes(a?.shape, b?.shape, t),
542+
shape: OutlinedBorder.lerp(a?.shape, b?.shape, t),
543543
labelStyle: TextStyle.lerp(a?.labelStyle, b?.labelStyle, t),
544544
secondaryLabelStyle: TextStyle.lerp(a?.secondaryLabelStyle, b?.secondaryLabelStyle, t),
545545
brightness: t < 0.5 ? a?.brightness ?? Brightness.light : b?.brightness ?? Brightness.light,
@@ -566,27 +566,16 @@ class ChipThemeData with Diagnosticable {
566566
if (a == null && b == null) {
567567
return null;
568568
}
569-
if (a is MaterialStateBorderSide) {
570-
a = a.resolve(<WidgetState>{});
569+
if (a is WidgetStateBorderSide) {
570+
a = a.resolve(const <WidgetState>{});
571571
}
572-
if (b is MaterialStateBorderSide) {
573-
b = b.resolve(<WidgetState>{});
572+
if (b is WidgetStateBorderSide) {
573+
b = b.resolve(const <WidgetState>{});
574574
}
575-
if (a == null) {
576-
return BorderSide.lerp(BorderSide(width: 0, color: b!.color.withAlpha(0)), b, t);
577-
}
578-
if (b == null) {
579-
return BorderSide.lerp(BorderSide(width: 0, color: a.color.withAlpha(0)), a, t);
580-
}
581-
return BorderSide.lerp(a, b, t);
582-
}
575+
a ??= BorderSide(width: 0, color: b!.color.withAlpha(0));
576+
b ??= BorderSide(width: 0, color: a.color.withAlpha(0));
583577

584-
// TODO(perclasson): OutlinedBorder needs a lerp method - https://github.com/flutter/flutter/issues/60555.
585-
static OutlinedBorder? _lerpShapes(OutlinedBorder? a, OutlinedBorder? b, double t) {
586-
if (a == null && b == null) {
587-
return null;
588-
}
589-
return ShapeBorder.lerp(a, b, t) as OutlinedBorder?;
578+
return BorderSide.lerp(a, b, t);
590579
}
591580

592581
@override

packages/flutter/lib/src/material/search_bar_theme.dart

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,7 @@ class SearchBarThemeData with Diagnosticable {
155155
t,
156156
Color.lerp,
157157
),
158-
side: _lerpSides(a?.side, b?.side, t),
158+
side: MaterialStateBorderSide.lerp(a?.side, b?.side, t),
159159
shape: MaterialStateProperty.lerp<OutlinedBorder?>(
160160
a?.shape,
161161
b?.shape,
@@ -304,18 +304,6 @@ class SearchBarThemeData with Diagnosticable {
304304
),
305305
);
306306
}
307-
308-
// Special case because BorderSide.lerp() doesn't support null arguments
309-
static MaterialStateProperty<BorderSide?>? _lerpSides(
310-
MaterialStateProperty<BorderSide?>? a,
311-
MaterialStateProperty<BorderSide?>? b,
312-
double t,
313-
) {
314-
if (identical(a, b)) {
315-
return a;
316-
}
317-
return MaterialStateBorderSide.lerp(a, b, t);
318-
}
319307
}
320308

321309
/// Applies a search bar theme to descendant [SearchBar] widgets.

packages/flutter/lib/src/material/search_view_theme.dart

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -225,12 +225,18 @@ class SearchViewThemeData with Diagnosticable {
225225

226226
// Special case because BorderSide.lerp() doesn't support null arguments
227227
static BorderSide? _lerpSides(BorderSide? a, BorderSide? b, double t) {
228-
if (a == null || b == null) {
228+
if (a == null && b == null) {
229229
return null;
230230
}
231-
if (identical(a, b)) {
232-
return a;
231+
if (a is WidgetStateBorderSide) {
232+
a = a.resolve(const <WidgetState>{});
233233
}
234+
if (b is WidgetStateBorderSide) {
235+
b = b.resolve(const <WidgetState>{});
236+
}
237+
a ??= BorderSide(width: 0, color: b!.color.withAlpha(0));
238+
b ??= BorderSide(width: 0, color: a.color.withAlpha(0));
239+
234240
return BorderSide.lerp(a, b, t);
235241
}
236242
}

packages/flutter/lib/src/widgets/widget_state.dart

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -582,6 +582,9 @@ abstract class WidgetStateBorderSide extends BorderSide
582582
if (a == null && b == null) {
583583
return null;
584584
}
585+
if (identical(a, b)) {
586+
return a;
587+
}
585588
return _LerpSides(a, b, t);
586589
}
587590
}

packages/flutter/test/material/checkbox_theme_test.dart

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -458,8 +458,44 @@ void main() {
458458
lerped.shape,
459459
const RoundedRectangleBorder(borderRadius: BorderRadius.all(Radius.circular(2.0))),
460460
);
461-
// Returns null if either lerp value is null.
462-
expect(lerped.side, null);
461+
expect(lerped.side!.width, 2.0);
462+
expect(lerped.side!.color, isSameColorAs(const Color(0x80000000)));
463+
});
464+
465+
test('CheckboxThemeData lerp from null to populated parameters', () {
466+
final CheckboxThemeData theme = CheckboxThemeData(
467+
fillColor: MaterialStateProperty.all(const Color(0xfffffff0)),
468+
checkColor: MaterialStateProperty.all(const Color(0xfffffff1)),
469+
overlayColor: MaterialStateProperty.all(const Color(0xfffffff2)),
470+
splashRadius: 4.0,
471+
materialTapTargetSize: MaterialTapTargetSize.shrinkWrap,
472+
visualDensity: const VisualDensity(vertical: 1.0, horizontal: 1.0),
473+
shape: const RoundedRectangleBorder(borderRadius: BorderRadius.all(Radius.circular(4.0))),
474+
side: const BorderSide(width: 4.0),
475+
);
476+
final CheckboxThemeData lerped = CheckboxThemeData.lerp(null, theme, 0.25);
477+
478+
expect(
479+
lerped.fillColor!.resolve(const <MaterialState>{}),
480+
isSameColorAs(const Color(0x40fffff0)),
481+
);
482+
expect(
483+
lerped.checkColor!.resolve(const <MaterialState>{}),
484+
isSameColorAs(const Color(0x40fffff1)),
485+
);
486+
expect(
487+
lerped.overlayColor!.resolve(const <MaterialState>{}),
488+
isSameColorAs(const Color(0x40fffff2)),
489+
);
490+
expect(lerped.splashRadius, 1);
491+
expect(lerped.materialTapTargetSize, null);
492+
expect(lerped.visualDensity, null);
493+
expect(
494+
lerped.shape,
495+
const RoundedRectangleBorder(borderRadius: BorderRadius.all(Radius.circular(1.0))),
496+
);
497+
expect(lerped.side!.width, 1.0);
498+
expect(lerped.side!.color, isSameColorAs(const Color(0x40000000)));
463499
});
464500

465501
test('CheckboxThemeData lerp from populated parameters', () {

packages/flutter/test/material/chip_theme_test.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -738,7 +738,7 @@ void main() {
738738
expect(lerpBNull25.showCheckmark, equals(false));
739739
expect(lerpBNull25.labelPadding, equals(const EdgeInsets.only(left: 6.0, right: 6.0)));
740740
expect(lerpBNull25.padding, equals(const EdgeInsets.all(3.0)));
741-
expect(lerpBNull25.side!.color, isSameColorAs(Colors.black.withAlpha(0x3f)));
741+
expect(lerpBNull25.side!.color, isSameColorAs(Colors.black.withAlpha(0xbf)));
742742
expect(lerpBNull25.shape, isA<StadiumBorder>());
743743
expect(lerpBNull25.labelStyle?.color, isSameColorAs(Colors.white.withAlpha(0xa7)));
744744
expect(lerpBNull25.secondaryLabelStyle?.color, isSameColorAs(Colors.black.withAlpha(0xa7)));
@@ -760,7 +760,7 @@ void main() {
760760
expect(lerpBNull75.showCheckmark, equals(true));
761761
expect(lerpBNull75.labelPadding, equals(const EdgeInsets.only(left: 2.0, right: 2.0)));
762762
expect(lerpBNull75.padding, equals(const EdgeInsets.all(1.0)));
763-
expect(lerpBNull75.side!.color, isSameColorAs(Colors.black.withAlpha(0xbf)));
763+
expect(lerpBNull75.side!.color, isSameColorAs(Colors.black.withAlpha(0x3f)));
764764
expect(lerpBNull75.shape, isA<StadiumBorder>());
765765
expect(lerpBNull75.labelStyle?.color, isSameColorAs(Colors.white.withAlpha(0x38)));
766766
expect(lerpBNull75.secondaryLabelStyle?.color, isSameColorAs(Colors.black.withAlpha(0x38)));

0 commit comments

Comments
 (0)