Skip to content

Implement IFormattable on HslColor and HsvColor#20928

Closed
NathanDrake2406 wants to merge 4 commits intoAvaloniaUI:masterfrom
NathanDrake2406:feat/hsl-hsv-iformattable
Closed

Implement IFormattable on HslColor and HsvColor#20928
NathanDrake2406 wants to merge 4 commits intoAvaloniaUI:masterfrom
NathanDrake2406:feat/hsl-hsv-iformattable

Conversation

@NathanDrake2406
Copy link
Copy Markdown
Contributor

What does the pull request do?

Adds IFormattable support to HslColor and HsvColor, following the same convention established in #20919 for Color: uppercase includes alpha, lowercase excludes it.

Type Specifier Output Example
HslColor L hsl(h, s%, l%, a) hsl(240, 80%, 20%, 0.5)
HslColor l hsl(h, s%, l%) hsl(240, 80%, 20%)
HsvColor V hsv(h, s%, v%, a) hsv(240, 80%, 20%, 0.5)
HsvColor v hsv(h, s%, v%) hsv(240, 80%, 20%)

Default ToString() behavior is unchanged for both types.

What is the current behavior?

HslColor and HsvColor have a single ToString() that outputs a fixed full-precision format (hsla(h, s, l, a) / hsva(h, s, v, a)). There is no way to control alpha inclusion or get CSS-style percentage output.

What is the updated/expected behavior with this PR?

var hsl = new HslColor(0.5, 240, 0.8, 0.2);
hsl.ToString("L", null) // "hsl(240, 80%, 20%, 0.5)" — with alpha
hsl.ToString("l", null) // "hsl(240, 80%, 20%)"       — without alpha
hsl.ToString()          // unchanged default

How was the solution implemented (if it's not obvious)?

Same pattern as Color (#20919): IFormattable interface guarded by #if !BUILDTASK, format specifier selects output, IFormatProvider accepted but ignored (culture-invariant). Hue is rounded to integer degrees, saturation/lightness/value to integer percentages — standard CSS representation.

Checklist

Breaking changes

None. IFormattable is additive and default ToString() is unchanged.

Fixed issues

Fixes #20927

Add ToString(string?, IFormatProvider?) to both types:
- HslColor: L/l for hsl() with/without alpha
- HsvColor: V/v for hsv() with/without alpha

Follows same convention as Color: uppercase includes alpha,
lowercase excludes it.
C/c outputs the natural CSS format for any color type:
- On HslColor: alias for L/l (hsl output)
- On HsvColor: alias for V/v (hsv output)

Enables type-agnostic formatting when the caller doesn't
need to know which color model they're holding.
@NathanDrake2406 NathanDrake2406 force-pushed the feat/hsl-hsv-iformattable branch from 444de8c to bb666b0 Compare March 18, 2026 03:15
@avaloniaui-bot
Copy link
Copy Markdown

You can test this PR using the following package version. 12.0.999-cibuild0063583-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@MrJul MrJul added the feature label Mar 18, 2026

return format switch
{
"V" or "C" => string.Format(CultureInfo.InvariantCulture, "hsv({0}, {1}%, {2}%, {3})", hDeg, sPct, vPct, A),
Copy link
Copy Markdown
Contributor

@robloo robloo Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: The existing code will output hsva() if there is an alpha channel.

We should double check the CSS spec and see if that was a mistake originally and if that needs to change one way or the other.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

HSV was a natural extension of HSL which is part of the CSS standard: https://www.w3schools.com/css/css_colors_hsl.asp

HSV is how the ColorPicker code in upsteam UWP/WinUI worked and also aligned with the community toolkit's HsvColor so was added first. Then I added HSL to give us the full CSS color models support. Since HSL follows CSS, HSV follows the same formatting rules which does include hsv vs hsva.

Copy link
Copy Markdown
Contributor

@robloo robloo Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here is a copy/paste bug in the legacy code. This should be hsla

Since it is going to have to change and break backwards compatibility we can definitely decide if there should be an a or not based on what CSS does.

No one noticed this before anyway so it isn't used for anything I would think.

Pre-existing copy-paste bug: HslColor.ToString() was outputting
"hsva(" instead of "hsla(". Spotted by @robloo in review.
@avaloniaui-bot
Copy link
Copy Markdown

You can test this PR using the following package version. 12.0.999-cibuild0063641-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@NathanDrake2406
Copy link
Copy Markdown
Contributor Author

Superseded by #20919 which now implements the unified format specifier design across all three color types per @robloo's review feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement IFormattable on HslColor and HsvColor

4 participants