Add currentColor support for SVG controls#518
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ec94d8e9b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| var baseCss = string.IsNullOrWhiteSpace(css) && string.IsNullOrWhiteSpace(currentCss) | ||
| ? source.Parameters?.Css ?? source.Css | ||
| : null; | ||
| var combinedCss = CombineCss(baseCss, css, currentCss); | ||
| var drawingColor = ToDrawingColor(currentColor) ?? source.Parameters?.CurrentColor ?? ToDrawingColor(source.CurrentColor); |
There was a problem hiding this comment.
Do not preserve stale SvgImage override parameters
When a SvgImage override is cleared, this falls back to source.Parameters, but source.Parameters is overwritten by the previous source.ReLoad(parameters) call. For example, after CurrentColor (or Css/CurrentCss) is set once and later becomes null/empty via an inherited binding, BuildParameters reuses the old parameter value, HasSameParameters sees no change, and the image keeps rendering with the stale tint/style instead of reverting to the source defaults.
Useful? React with 👍 / 👎.
PR Summary: Add host-provided currentColor support
Summary
This change adds a host-facing
CurrentColorAPI so Avalonia consumers can theme SVG assets that usefill="currentColor"orstroke="currentColor"without rewriting the SVG or injecting fill/stroke-specific CSS selectors.The new color value is passed through
SvgParametersand applied as the SVG rootcoloronly when the SVG document does not already define its own root color, or when the root explicitly asks to inherit/usecurrentColor. Explicit root colors such ascolor="red"orstyle="color:red"are preserved.Motivation
Discussion #440 requested a
Foreground-style mechanism for SVG controls so icon assets authored withcurrentColorcan follow application theme colors. Before this change, callers had to override concrete paint properties with CSS, which is awkward for reusable icon sets and does not map well to howcurrentColoris expected to work in SVG/CSS.Changes
Shared model and loading
CurrentColortoSvgParameters.SvgParameters(entities, css)constructor and deconstruction shape for source compatibility.SvgServiceload paths to applySvgParameters.CurrentColorafter SVG or VectorDrawable parsing.CurrentColordoes not override explicit SVG rootcolorvalues.SvgService.FromSvg(string, SvgParameters?)so inline SVG parsing can carry the same parameters as stream/path loading.Avalonia controls
CurrentColorattached/instance property toAvalonia.Svg.Skia.Svg.CurrentColorattached/instance property toAvalonia.Svg.Svg.CurrentColor.CurrentColor.Skia Avalonia image/resource APIs
SvgImage.CurrentColor.SvgSource.CurrentColorand preserved it when cloning or cloning parameters.SvgImageExtensionto read inheritedSvg.CurrentColorfrom the target control and bind future changes toSvgImage.CurrentColor.SvgResourceExtensionwith aCurrentColorproperty for resource/brush creation.SvgImageapplies additional style overrides.Uno preservation
SvgParameters.CurrentColorin UnoSvgSourceparameter cloning.CurrentColorin Uno cache keys.CurrentColorwhen Uno controls rebuild external sources with additional CSS.Documentation
CurrentColorusage with SVG assets that containfill="currentColor"orstroke="currentColor".Tests
Focused verification passed:
dotnet test tests/Svg.Skia.UnitTests/Svg.Skia.UnitTests.csproj -c Release --filter "FullyQualifiedName~SKSvgTests.Load_CurrentColorParameter"dotnet test tests/Svg.Controls.Skia.Avalonia.UnitTests/Svg.Controls.Skia.Avalonia.UnitTests.csproj -c Release --filter "FullyQualifiedName~SvgControlTests|FullyQualifiedName~SvgImageTests|FullyQualifiedName~SvgResourceExtensionTests"dotnet test tests/Svg.Controls.Skia.Uno.UnitTests/Svg.Controls.Skia.Uno.UnitTests.csproj -c Release --filter "FullyQualifiedName~SvgSourceTests.CacheKey|FullyQualifiedName~SvgSourceTests.BuildParameters"dotnet test tests/Svg.Controls.Avalonia.UnitTests/Svg.Controls.Avalonia.UnitTests.csproj -c Releasedotnet build src/Svg.Controls.Avalonia/Svg.Controls.Avalonia.csproj -c Release -f net10.0 --no-restoredotnet build src/Svg.Controls.Skia.Avalonia/Svg.Controls.Skia.Avalonia.csproj -c Release -f net10.0 --no-restoredotnet build src/Svg.Controls.Skia.Uno/Svg.Controls.Skia.Uno.csproj -c Release --no-restoregit diff --checkThe full solution build was attempted with:
dotnet build Svg.Skia.slnx -c Release --no-restoreIt still fails before completing because several sample/editor/benchmark projects do not have
project.assets.jsongenerated in this worktree, and somenet461editor targets require the .NET Framework 4.6.1 targeting pack.Notes For Review
CurrentColorintentionally does not override explicit SVG root color.color="currentColor"is treated as asking for the host current color.