-
Notifications
You must be signed in to change notification settings - Fork 6k
[web] Implement TextStyle.shadows #13769
Conversation
| if (shadows.length > 1) { | ||
| throw UnsupportedError('Multiple shadows on text not supported'); | ||
| } | ||
| cssStyle.textShadow = ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be assigned to _shadowListToCss(style._shadows)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
| if (style._fontFamily != previousStyle._fontFamily) { | ||
| cssStyle.fontFamily = quoteFontFamily(style._fontFamily); | ||
| } | ||
| if (style._shadows != previousStyle._shadows) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This covers the else case but not the if case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
| if (style._fontFamily != previousStyle._fontFamily) { | ||
| cssStyle.fontFamily = quoteFontFamily(style._fontFamily); | ||
| } | ||
| if (style._shadows != previousStyle._shadows) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comparing List<ui.Shadow> using != won't work because List does not implement operator== properly. We probably need a listsEqual utility that compares lists element-wise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately no ImmutableList there but TextStyle is not mutable itself, so keeping identity check.
| @required ui.TextAlign textAlign, | ||
| @required ui.TextDirection textDirection, | ||
| @required ui.Paint background, | ||
| @required List<ui.Shadow> shadows, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This also needs to be added to ui.ParagraphStyle.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done.
|
This commit causes a breakage in the framework tests. They test for the exact output of ParagraphStyle.toString() in packages/flutter/test/painting/text_style_test.dart |
Fixes: flutter/flutter#44458