Skip to content

Commit

Permalink
[web] Improve null safety for color->css (#41699)
Browse files Browse the repository at this point in the history 
Turning `colorToCssString` into an extension method on `ui.Color` improves null safety and developer ergonomics.
  • Loading branch information
@mdebbar
mdebbar committed May 3, 2023
1 parent 7df8bfb commit 92a16fa
Show file tree
Hide file tree
Showing 12 changed files with 44 additions and 49 deletions.
6 changes: 3 additions & 3 deletions lib/web_ui/lib/src/engine/canvas_pool.dart
View file
Expand Up @@ -541,7 +541,7 @@ class CanvasPool extends _SaveStackTracking {
void drawColor(ui.Color color, ui.BlendMode blendMode) {
final DomCanvasRenderingContext2D ctx = context;
contextHandle.blendMode = blendMode;
contextHandle.fillStyle = colorToCssString(color);
contextHandle.fillStyle = color.toCssString();
contextHandle.strokeStyle = '';
ctx.beginPath();
// Fill a virtually infinite rect with the color.
Expand Down Expand Up @@ -997,7 +997,7 @@ class ContextStateHandle {
}
}
} else {
final String? colorString = colorValueToCssString(paint.color);
final String colorString = colorValueToCssString(paint.color);
fillStyle = colorString;
strokeStyle = colorString;
}
Expand All @@ -1019,7 +1019,7 @@ class ContextStateHandle {
context.save();
context.shadowBlur = convertSigmaToRadius(maskFilter.webOnlySigma);
// Shadow color must be fully opaque.
context.shadowColor = colorToCssString(ui.Color(paint.color).withAlpha(255));
context.shadowColor = ui.Color(paint.color).withAlpha(255).toCssString();

// On the web a shadow must always be painted together with the shape
// that casts it. In order to paint just the shadow, we offset the shape
Expand Down
12 changes: 6 additions & 6 deletions lib/web_ui/lib/src/engine/html/bitmap_canvas.dart
View file
Expand Up @@ -572,7 +572,7 @@ class BitmapCanvas extends EngineCanvas {
void _applyFilter(DomElement element, SurfacePaintData paint) {
if (paint.maskFilter != null) {
final bool isStroke = paint.style == ui.PaintingStyle.stroke;
final String cssColor = colorValueToCssString(paint.color)!;
final String cssColor = colorValueToCssString(paint.color);
final double sigma = paint.maskFilter!.webOnlySigma;
if (browserEngine == BrowserEngine.webkit && !isStroke) {
// A bug in webkit leaves artifacts when this element is animated
Expand Down Expand Up @@ -808,7 +808,7 @@ class BitmapCanvas extends EngineCanvas {
case ui.BlendMode.srcOver:
style
..position = 'absolute'
..backgroundColor = colorToCssString(filterColor)!;
..backgroundColor = filterColor!.toCssString();
case ui.BlendMode.dst:
case ui.BlendMode.dstIn:
style
Expand All @@ -820,7 +820,7 @@ class BitmapCanvas extends EngineCanvas {
..backgroundImage = "url('${image.imgElement.src}')"
..backgroundBlendMode =
blendModeToCssMixBlendMode(colorFilterBlendMode) ?? ''
..backgroundColor = colorToCssString(filterColor)!;
..backgroundColor = filterColor!.toCssString();
break;
}
return imgElement;
Expand All @@ -839,7 +839,7 @@ class BitmapCanvas extends EngineCanvas {
final DomHTMLElement imgElement = _reuseOrCreateImage(image);
imgElement.style.filter = 'url(#${svgFilter.id})';
if (colorFilterBlendMode == ui.BlendMode.saturation) {
imgElement.style.backgroundColor = colorToCssString(filterColor)!;
imgElement.style.backgroundColor = filterColor!.toCssString();
}
return imgElement;
}
Expand Down Expand Up @@ -900,7 +900,7 @@ class BitmapCanvas extends EngineCanvas {
if (shadows != null) {
ctx.save();
for (final ui.Shadow shadow in shadows) {
ctx.shadowColor = colorToCssString(shadow.color);
ctx.shadowColor = shadow.color.toCssString();
ctx.shadowBlur = shadow.blurRadius;
ctx.shadowOffsetX = shadow.offset.dx;
ctx.shadowOffsetY = shadow.offset.dy;
Expand Down Expand Up @@ -1009,7 +1009,7 @@ class BitmapCanvas extends EngineCanvas {
final ui.Color color = ui.Color(paint.color);
_canvasPool.contextHandle
..fillStyle = null
..strokeStyle = colorToCssString(color);
..strokeStyle = color.toCssString();
glRenderer!.drawHairline(ctx, positions);
restore();
return;
Expand Down
2 changes: 1 addition & 1 deletion lib/web_ui/lib/src/engine/html/clip.dart
View file
Expand Up @@ -231,7 +231,7 @@ class PersistedPhysicalShape extends PersistedContainerSurface
}

void _applyColor() {
rootElement!.style.backgroundColor = colorToCssString(color)!;
rootElement!.style.backgroundColor = color.toCssString();
}

@override
Expand Down
12 changes: 6 additions & 6 deletions lib/web_ui/lib/src/engine/html/color_filter.dart
View file
Expand Up @@ -352,7 +352,7 @@ SvgFilter _srcInColorFilterToSvg(ui.Color? color) {
result: 'destalpha',
);
builder.setFeFlood(
floodColor: colorToCssString(color) ?? '',
floodColor: color?.toCssString() ?? '',
floodOpacity: '1',
result: 'flood',
);
Expand All @@ -374,7 +374,7 @@ SvgFilter _srcInColorFilterToSvg(ui.Color? color) {
SvgFilter _dstATopColorFilterToSvg(ui.Color? color) {
final SvgFilterBuilder builder = SvgFilterBuilder();
builder.setFeFlood(
floodColor: colorToCssString(color) ?? '',
floodColor: color?.toCssString() ?? '',
floodOpacity: '1',
result: 'flood',
);
Expand All @@ -390,7 +390,7 @@ SvgFilter _dstATopColorFilterToSvg(ui.Color? color) {
SvgFilter _srcOutColorFilterToSvg(ui.Color? color) {
final SvgFilterBuilder builder = SvgFilterBuilder();
builder.setFeFlood(
floodColor: colorToCssString(color) ?? '',
floodColor: color?.toCssString() ?? '',
floodOpacity: '1',
result: 'flood',
);
Expand All @@ -406,7 +406,7 @@ SvgFilter _srcOutColorFilterToSvg(ui.Color? color) {
SvgFilter _xorColorFilterToSvg(ui.Color? color) {
final SvgFilterBuilder builder = SvgFilterBuilder();
builder.setFeFlood(
floodColor: colorToCssString(color) ?? '',
floodColor: color?.toCssString() ?? '',
floodOpacity: '1',
result: 'flood',
);
Expand All @@ -425,7 +425,7 @@ SvgFilter _compositeColorFilterToSvg(
ui.Color? color, double k1, double k2, double k3, double k4) {
final SvgFilterBuilder builder = SvgFilterBuilder();
builder.setFeFlood(
floodColor: colorToCssString(color) ?? '',
floodColor: color?.toCssString() ?? '',
floodOpacity: '1',
result: 'flood',
);
Expand Down Expand Up @@ -478,7 +478,7 @@ SvgFilter _blendColorFilterToSvg(ui.Color? color, SvgBlendMode svgBlendMode,
{bool swapLayers = false}) {
final SvgFilterBuilder builder = SvgFilterBuilder();
builder.setFeFlood(
floodColor: colorToCssString(color) ?? '',
floodColor: color?.toCssString() ?? '',
floodOpacity: '1',
result: 'flood',
);
Expand Down
10 changes: 5 additions & 5 deletions lib/web_ui/lib/src/engine/html/dom_canvas.dart
View file
Expand Up @@ -60,7 +60,7 @@ class DomCanvas extends EngineCanvas with SaveElementStackTracking {
..right = '0'
..bottom = '0'
..left = '0'
..backgroundColor = colorToCssString(color)!;
..backgroundColor = color.toCssString();
currentElement.append(box);
}

Expand Down Expand Up @@ -257,15 +257,15 @@ DomHTMLElement buildDrawRectElement(
..transformOrigin = '0 0 0'
..transform = effectiveTransform;

String cssColor = colorValueToCssString(paint.color)!;
String cssColor = colorValueToCssString(paint.color);

if (paint.maskFilter != null) {
final double sigma = paint.maskFilter!.webOnlySigma;
if (browserEngine == BrowserEngine.webkit && !isStroke) {
// A bug in webkit leaves artifacts when this element is animated
// with filter: blur, we use boxShadow instead.
style.boxShadow = '0px 0px ${sigma * 2.0}px $cssColor';
cssColor = colorToCssString(blurColor(ui.Color(paint.color), sigma))!;
cssColor = blurColor(ui.Color(paint.color), sigma).toCssString();
} else {
style.filter = 'blur(${sigma}px)';
}
Expand Down Expand Up @@ -345,14 +345,14 @@ SVGSVGElement pathToSvgElement(SurfacePath path, SurfacePaintData paint) {
(paint.style != ui.PaintingStyle.fill &&
paint.strokeWidth != 0 &&
paint.strokeWidth != null)) {
svgPath.setAttribute('stroke', colorValueToCssString(paint.color)!);
svgPath.setAttribute('stroke', colorValueToCssString(paint.color));
svgPath.setAttribute('stroke-width', '${paint.strokeWidth ?? 1.0}');
if (paint.strokeCap != null) {
svgPath.setAttribute('stroke-linecap', '${stringForStrokeCap(paint.strokeCap)}');
}
svgPath.setAttribute('fill', 'none');
} else {
svgPath.setAttribute('fill', colorValueToCssString(paint.color)!);
svgPath.setAttribute('fill', colorValueToCssString(paint.color));
}
if (path.fillType == ui.PathFillType.evenOdd) {
svgPath.setAttribute('fill-rule', 'evenodd');
Expand Down
2 changes: 1 addition & 1 deletion lib/web_ui/lib/src/engine/html/painting.dart
View file
Expand Up @@ -268,7 +268,7 @@ class SurfacePaintData {
if (strokeJoin != null) {
buffer.write('strokeJoin = $strokeJoin; ');
}
buffer.write('color = ${colorToCssString(ui.Color(color))}; ');
buffer.write('color = ${ui.Color(color).toCssString()}; ');
if (shader != null) {
buffer.write('shader = $shader; ');
}
Expand Down
8 changes: 4 additions & 4 deletions lib/web_ui/lib/src/engine/html/shaders/shader.dart
View file
Expand Up @@ -407,13 +407,13 @@ void _addColorStopsToCanvasGradient(DomCanvasGradient gradient,
}
if (colorStops == null) {
assert(colors.length == 2);
gradient.addColorStop(offset, colorToCssString(colors[0])!);
gradient.addColorStop(1 - offset, colorToCssString(colors[1])!);
gradient.addColorStop(offset, colors[0].toCssString());
gradient.addColorStop(1 - offset, colors[1].toCssString());
} else {
for (int i = 0; i < colors.length; i++) {
final double colorStop = colorStops[i].clamp(0.0, 1.0);
gradient.addColorStop(
colorStop * scale + offset, colorToCssString(colors[i])!);
colorStop * scale + offset, colors[i].toCssString());
}
}
if (isDecal) {
Expand Down Expand Up @@ -841,7 +841,7 @@ class ModeHtmlColorFilter extends EngineHtmlColorFilter {
if (blendMode == ui.BlendMode.saturation ||
blendMode == ui.BlendMode.multiply ||
blendMode == ui.BlendMode.modulate) {
filterElement!.style.backgroundColor = colorToCssString(color)!;
filterElement!.style.backgroundColor = color.toCssString();
}
return svgFilter.element;
}
Expand Down
10 changes: 5 additions & 5 deletions lib/web_ui/lib/src/engine/text/paragraph.dart
View file
Expand Up @@ -791,13 +791,13 @@ void applyTextStyleToElement({
final double adaptedWidth = strokeWidth != null && strokeWidth > 0
? strokeWidth
: 1.0 / ui.window.devicePixelRatio;
cssStyle.textStroke = '${adaptedWidth}px ${colorToCssString(color)}';
cssStyle.textStroke = '${adaptedWidth}px ${color?.toCssString()}';
} else if (color != null) {
cssStyle.color = colorToCssString(color)!;
cssStyle.color = color.toCssString();
}
final ui.Color? background = style.background?.color;
if (background != null) {
cssStyle.backgroundColor = colorToCssString(background)!;
cssStyle.backgroundColor = background.toCssString();
}
final double? fontSize = style.fontSize;
if (fontSize != null) {
Expand Down Expand Up @@ -843,7 +843,7 @@ void applyTextStyleToElement({
}
final ui.Color? decorationColor = style.decorationColor;
if (decorationColor != null) {
cssStyle.textDecorationColor = colorToCssString(decorationColor)!;
cssStyle.textDecorationColor = decorationColor.toCssString();
}
}
}
Expand Down Expand Up @@ -878,7 +878,7 @@ String _shadowListToCss(List<ui.Shadow> shadows) {
}
final ui.Shadow shadow = shadows[i];
sb.write('${shadow.offset.dx}px ${shadow.offset.dy}px '
'${shadow.blurRadius}px ${colorToCssString(shadow.color)}');
'${shadow.blurRadius}px ${shadow.color.toCssString()}');
}
return sb.toString();
}
Expand Down
17 changes: 6 additions & 11 deletions lib/web_ui/lib/src/engine/util.dart
View file
Expand Up @@ -339,20 +339,15 @@ bool rectContainsOther(ui.Rect rect, ui.Rect other) {
rect.bottom >= other.bottom;
}

/// Converts color to a css compatible attribute value.
String? colorToCssString(ui.Color? color) {
if (color == null) {
return null;
extension CssColor on ui.Color {
/// Converts color to a css compatible attribute value.
String toCssString() {
return colorValueToCssString(value);
}
final int value = color.value;
return colorValueToCssString(value);
}

// Converts a color value (as an int) into a CSS-compatible value.
String? colorValueToCssString(int? value) {
if (value == null) {
return null;
}
String colorValueToCssString(int value) {
if (value == 0xFF000000) {
return '#000000';
}
Expand Down Expand Up @@ -690,7 +685,7 @@ void setThemeColor(ui.Color? color) {
..name = 'theme-color';
domDocument.head!.append(theme);
}
theme.content = colorToCssString(color)!;
theme.content = color.toCssString();
} else {
theme?.remove();
}
Expand Down
8 changes: 4 additions & 4 deletions lib/web_ui/test/engine/platform_dispatcher/application_switcher_description_test.dart
View file
Expand Up @@ -47,7 +47,7 @@ Future<void> testMain() async {
const ui.Color expectedPrimaryColor = ui.Color(0xFF00FF00);

expect(domDocument.title, 'Title Test');
expect(getCssThemeColor(), colorToCssString(expectedPrimaryColor));
expect(getCssThemeColor(), expectedPrimaryColor.toCssString());

ui.window.sendPlatformMessage(
'flutter/platform',
Expand All @@ -64,7 +64,7 @@ Future<void> testMain() async {
const ui.Color expectedNewPrimaryColor = ui.Color(0xFFFABADA);

expect(domDocument.title, 'Different title');
expect(getCssThemeColor(), colorToCssString(expectedNewPrimaryColor));
expect(getCssThemeColor(), expectedNewPrimaryColor.toCssString());
});

test('supports null title and primaryColor', () {
Expand All @@ -89,7 +89,7 @@ Future<void> testMain() async {
);

expect(domDocument.title, '');
expect(getCssThemeColor(), colorToCssString(expectedNullColor));
expect(getCssThemeColor(), expectedNullColor.toCssString());

domDocument.title = 'Something Else';
expect(domDocument.title, 'Something Else');
Expand All @@ -104,7 +104,7 @@ Future<void> testMain() async {
);

expect(domDocument.title, '');
expect(getCssThemeColor(), colorToCssString(expectedNullColor));
expect(getCssThemeColor(), expectedNullColor.toCssString());
});
});
}
2 changes: 1 addition & 1 deletion lib/web_ui/test/engine/platform_dispatcher/system_ui_overlay_style_test.dart
View file
Expand Up @@ -44,7 +44,7 @@ void testMain() {

const ui.Color statusBarColor = ui.Color(0xFFF44336);
sendSetSystemUIOverlayStyle(statusBarColor: statusBarColor);
expect(getCssThemeColor(), colorToCssString(statusBarColor));
expect(getCssThemeColor(), statusBarColor.toCssString());

sendSetSystemUIOverlayStyle();
expect(getCssThemeColor(), null);
Expand Down
4 changes: 2 additions & 2 deletions lib/web_ui/test/html/path_to_svg_golden_test.dart
View file
Expand Up @@ -194,14 +194,14 @@ DomElement pathToSvgElement(Path path, Paint paint, bool enableFill) {
root.append(pathElement);
if (paint.style == PaintingStyle.stroke ||
paint.strokeWidth != 0.0) {
pathElement.setAttribute('stroke', colorToCssString(paint.color)!);
pathElement.setAttribute('stroke', paint.color.toCssString());
pathElement.setAttribute('stroke-width', paint.strokeWidth);
if (!enableFill) {
pathElement.setAttribute('fill', 'none');
}
}
if (paint.style == PaintingStyle.fill) {
pathElement.setAttribute('fill', colorToCssString(paint.color)!);
pathElement.setAttribute('fill', paint.color.toCssString());
}
pathElement.setAttribute('d', pathToSvg((path as SurfacePath).pathRef)); // This is what we're testing!
return root;
Expand Down

0 comments on commit 92a16fa

Please sign in to comment.