Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ColorHelper.ToDisplayName still appears to be missing since before 1.0? #8287

Closed
Tracked by #2
michael-hawker opened this issue Mar 15, 2023 · 10 comments
Closed
Tracked by #2
Labels
feature proposal New feature proposal fix-released The fix has been in a release (experimental, preview, stable, or servicing). team-Markup Issue for the Markup team wct
Milestone

Comments

@michael-hawker
Copy link
Collaborator

michael-hawker commented Mar 15, 2023

@bpulliam ToDisplayName still appear to be missing in the latest release, this never seemed resolved here on how we're supposed to upgrade these code paths. Also see: MicrosoftDocs/winrt-api#2120

This effects our ColorToDisplayNameConverter in the toolkit, so not sure what to do with our comment here:

        // return global::Microsoft.UI.Xaml.ColorDisplayNameHelper.ToDisplayName(color);
        // Removed on Reunion 0.8-RC. Not sure what to replace with.

Originally posted by @michael-hawker in #5260 (comment)

Someone mentioned using the original Windows.UI one, but that doesn't seem to work either.

I get red squiggles trying to use the Windows.UI version explicitly Windows.UI.ColorHelper.ToDisplayName(color):

image

@DarranRowe
Copy link

@michael-hawker
I know it is a silly question, but for C#, wouldn't Color.ToString be how you would do this?
The documentation for ColorHelper itself does state that you should be using members on the Color class due to how C# wraps it.

@jaigak
Copy link
Contributor

jaigak commented Mar 16, 2023

@DarranRowe Color.ToString returns the hex representation of the colour, not the display name.

From the docs:

For C# and Microsoft Visual Basic, the language support for the Color structure provides a behavior for ToString that serializes the values of the ARGB data properties into a single string. The string representations of Color values are similar to XAML attribute string syntax for specifying Color values in markup. It is the syntax that is most commonly used by designer tools to specify a (non-named) Color. The string is in the form #AARRGGBB, where each letter pair represents one of the color channels as a value between 00 and FF. Each letter pair is interpreted as if it were a hex value and thus represents a value between 0 and 255. The string always starts with a hash (#). For example, the string form of the color where A=255, R=0, G=128, B=255 is "#FF0080FF". For named colors you get the serialized string and not the constant name; for example calling ToString on Colors.Blue gives "#FF0000FF".

@DarranRowe
Copy link

Hmm, well, I have to admit that I'm not exactly that familiar with the C# projection.
Since I am more knowledgeable of the C++ projection, I was messing around with this a little. Interestingly, ToDisplayName seems to just outright crash. Digging into the disassembly a little, this seems to call into Windows.UI.Xaml.dll, and for some reason, at least on Windows 11 22H2, DirectUI::DXamlCore::GetLocalizedResourceString tries to access a null pointer.
So part of me is saying that there is also currently a bug in Windows that would get in the way.

@jaigak
Copy link
Contributor

jaigak commented Mar 16, 2023

@DarranRowe Are you running as packaged or unpackaged. Maybe this has to do with it using old MRT (Windows.ApplicationModel.Resources) instead of MRT Core from Windows App SDK (Microsoft.Windows.ApplicationModel.Resources). Or maybe it's dependent on the UWP XAML framework to be initialized, even though the ColorHelper class itself is not under the Windows.UI.Xaml namespace.

@DarranRowe
Copy link

@JaiganeshKumaran
I tried both packaged and unpackaged, it had the same behaviour. I also tried it in a plain Windows API application, so that completely ruled out any Windows App SDK interference.
But since Windows Runtime APIs not supported in desktop apps doesn't give any indicator that it shouldn't work, I am inclined to go with bug.

@jaigak
Copy link
Contributor

jaigak commented Mar 16, 2023

@DarranRowe Probably the UWP XAML framework needs to be initialised first then.

@DarranRowe
Copy link

@JaiganeshKumaran By the looks of it. I dug in a little more and it was getting a null pointer from a TLS slot. I then had the brilliant idea to initialise Xaml Islands for the UI thread and it worked.
But as I stated the documentation doesn't give any indicator that it shouldn't work. The ToDisplayName documentation also doesn't state that UWP Xaml needs to be initialised in order for it to work. If it does require Xaml to be initialised then it needs to be documented. If the intention is to not require it to be initialised then this issue needs to be fixed.

michael-hawker added a commit to CommunityToolkit/Windows that referenced this issue Mar 22, 2023
- Updated to `CommunityToolkit.WinUI.Converters` namespace
- Changed `FormatStringConverter` to `IFormattableToStringConverter` to differentiate from `StringFormatConverter` (too confusing to have such a similar name)
- Merged differences between UWP and WinAppSDK branches of main toolkit, _have proper updates from both_
- Imported original monolith doc from our doc repo as a starting point, converted one sample as a starting point for `StringFormatConverter`
- Brought over existing unit tests from main repo (**did not** compare or merge with WinUI branch **nor** update to use our helpers), but passing for UWP and WASDK
- Found platform gap for ColorToDisplayNameConverter, see microsoft/microsoft-ui-xaml#8287
@michael-hawker
Copy link
Collaborator Author

I think this function just needs to be ported/exposed to the new Microsoft.UI.ColorHelper version of the class that does exist in WASDK/WinUI 3?

@bpulliam bpulliam added team-Framework team-Markup Issue for the Markup team feature proposal New feature proposal and removed team-Framework labels Aug 22, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the needs-triage Issue needs to be triaged by the area owners label Aug 22, 2023
@bpulliam bpulliam removed the needs-triage Issue needs to be triaged by the area owners label Oct 18, 2023
@Arlodotexe
Copy link

@duncanmacmichael
Copy link
Member

Fixed in 1.6.0; closing issue.

@codendone codendone added this to the WinAppSDK 1.6 milestone Sep 6, 2024
@codendone codendone added the fix-released The fix has been in a release (experimental, preview, stable, or servicing). label Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature proposal New feature proposal fix-released The fix has been in a release (experimental, preview, stable, or servicing). team-Markup Issue for the Markup team wct
Projects
None yet
Development

No branches or pull requests

7 participants