fix markup extension for avalonia

[StefanKoell]

• rename markup extension class to SymbolIconExt to avoid conflicts for style selectors
• remove default values in the markup extension (nullable props) to allow style selectors set property values
• remove default values in SymbolIcon ctor to allow style selectors set property values
• little bit of code cleanup to make analyzers happy

[davidxuang]

No, this UseSegoeMetricsDefaultValue implementation is wrong. It needs to supplied by a callback. like in

FluentIcons/FluentIcons.WinUI/SymbolIcon.cs

Line 24 in 52a57f6

DependencyProperty.Register(nameof(UseSegoeMetrics), typeof(bool), typeof(SymbolIcon), PropertyMetadata.Create(() => UseSegoeMetricsDefaultValue, OnSymbolPropertiesChanged));

, which Avalonia did not implement. It may be logical to allow global value only for this property.

Maybe make the property Lazy<> will also do, but I'm not sure, plus this solution has a racing issue.

[StefanKoell]

I did test this and for me it worked when I used the AppBuilderExtension to set the internal static UseSegoeMetricsDefaultValue to true. I double checked the results. Where do you see a potential racing condition?

[davidxuang]

I did test this and for me it worked when I used the AppBuilderExtension to set the internal static UseSegoeMetricsDefaultValue to true. I double checked the results. Where do you see a potential racing condition?

Weird. I suppose that all static fields has to be initialized before it can be changed, so the property would have been created before UseSegoeMetricsDefaultValue could be altered.

[StefanKoell]

I think it works because the AppBuilder extension is currently the only way to alter the default and this is executed before the whole Avalonia infrastructure is built. So at the time the styled properties are set, the static default value is already set. That's my guess at least. If it should be possible to alter the value afterwards, it has to be implemented differently but in that case, the ability to use style selectors to "override" the value will be gone.

A bit off topic but this setting is a mystery to me. Is there any documentation what value this should be? Is it encouraged to set it to true?

[davidxuang]

A bit off topic but this setting is a mystery to me. Is there any documentation what value this should be? Is it encouraged to set it to true?

It enables a modified version of fluentui-system-icons, so that it looks more similar to Segoe fonts. Check https://github.com/davidxuang/FluentIcons/blob/master/seagull-icons/README.md.

The most significant change is that Segoe uses a stroke width of 1/16 (of size of bounding square) while fluentui-system-icons uses 1/20 and has a large padding. This is done by removing padding and re-drawing some symbols which are out of bound. Re-drawing are done by hand (https://github.com/davidxuang/FluentIcons/tree/master/seagull-icons/override) or semi-automated composition (https://github.com/davidxuang/FluentIcons/blob/master/seagull-icons/transform.ts).

[StefanKoell]

It enables a modified version of fluentui-system-icons, so that it looks more similar to Segoe fonts. Check https://github.com/davidxuang/FluentIcons/blob/master/seagull-icons/README.md.

The most significant change is that Segoe uses a stroke width of 1/16 (of size of bounding square) while fluentui-system-icons uses 1/20 and has a large padding. This is done by removing padding and re-drawing some symbols which are out of bound. Re-drawing are done by hand (https://github.com/davidxuang/FluentIcons/tree/master/seagull-icons/override) or semi-automated composition (https://github.com/davidxuang/FluentIcons/blob/master/seagull-icons/transform.ts).

Thanks for the clarifications. This explains a couple of things. I have other icon sources as well and they are designed according to the fluent system guidelines (smaller appearance, padding around, etc.). So in this case I need to keep the UseSegoeMetricsDefaultValue set to false and compensate with the FontSize to align the icon sizes. The icons from my other source doesn't allow me to remove the padding/change the stroke width.

Regarding the PR:
Do you think the implementation as it is in the PR is OK or should set the value in the ctor again. It will not have any impact for me as I'm not using the UseSegoeMetricsDefaultValue = true

[davidxuang]

I didn't see it in the changelog, but is there any issue caused by inheriting MarkupExtension?

Regarding the PR: Do you think the implementation as it is in the PR is OK or should set the value in the ctor again. It will not have any impact for me as I'm not using the UseSegoeMetricsDefaultValue = true

I need more tests on this.

[StefanKoell]

I think the requirement to inherit from MarkupExtension is deprecated IIRC. The official docs show MarkupExtension as a standalone class: https://docs.avaloniaui.net/docs/concepts/markupextensions

I also briefly looked at the Avalonia source code and most MarkupExtensions are not inheriting from MarkupExtension:
https://github.com/AvaloniaUI/Avalonia/tree/f140033e420ee39b16dcc0621d5580f94d5bcb2c/src/Markup/Avalonia.Markup.Xaml/MarkupExtensions

[davidxuang]

Issues addressed.