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

MUX DropDownButton/SplitButton/ToggleSplitButton Don't Show Chevron Glyph #6638

Closed
3 of 24 tasks
robloo opened this issue Aug 2, 2021 · 28 comments · Fixed by #6924
Closed
3 of 24 tasks

MUX DropDownButton/SplitButton/ToggleSplitButton Don't Show Chevron Glyph #6638

robloo opened this issue Aug 2, 2021 · 28 comments · Fixed by #6924
Labels
difficulty/tbd Categorizes an issue for which the difficulty level needs to be defined. kind/bug Something isn't working project/input ⌨️ Categorizes an issue or PR as relevant to input (Button, CheckBox, Toggle, Scroll, Map, Numeric,...) project/text 🔤 Categorizes an issue or PR as relevant to text (TextBox, PasswordBox, TextBlock, Fonts, …)

Comments

@robloo
Copy link
Contributor

robloo commented Aug 2, 2021

Current behavior

The Microsoft.UI.Xaml DropDownButton, SplitButton and ToggleSplitButton don't show the down chevron glyph. It instead falls back to a font besides Segoe MDL2 Assets (the Uno Equivalent).

image

I'm not sure where this issue happens as the code seems to be correct. This may be an issue with Resource lookup but then I wonder how any other controls work. Perhaps the glyph itself is missing from the Uno Font and we should change it to the ComboBox Unicode point.

FontFamily="{ThemeResource SymbolThemeFontFamily}"
FontSize="12"
Text=""

Expected behavior

The chevron should be shown.

How to reproduce it (as minimally and precisely as possible) / Test App

See the following test application.

DropDownButtonTest.zip

Workaround

Works on UWP/WinUI

Yes

Environment

Nuget Package Version(s): 3.9.1
Visual Studio 2019 Version 16.11.1
Windows 10 Pro 21H1 19043.1165
Android Emulator: Pixel 2 Q 10.0 - API 29

Nuget Package:

  • Uno.UI / Uno.UI.WebAssembly / Uno.UI.Skia
  • Uno.WinUI / Uno.WinUI.WebAssembly / Uno.WinUI.Skia
  • Uno.SourceGenerationTasks
  • Uno.UI.RemoteControl / Uno.WinUI.RemoteControl
  • Other:

Nuget Package Version(s): 3.9.1

Affected platform(s):

  • iOS
  • Android
  • WebAssembly
  • WebAssembly renderers for Xamarin.Forms
  • macOS
  • Skia
    • WPF
    • GTK (Linux)
    • Tizen
  • Windows
  • Build tasks
  • Solution Templates

IDE:

  • Visual Studio 2017 (version: )
  • Visual Studio 2019 (version: )
  • Visual Studio for Mac (version: )
  • Rider Windows (version: )
  • Rider macOS (version: )
  • Visual Studio Code (version: )

Relevant plugins:

  • Resharper (version: )

Anything else we need to know?

@robloo robloo added difficulty/tbd Categorizes an issue for which the difficulty level needs to be defined. kind/bug Something isn't working triage/untriaged Indicates an issue requires triaging or verification labels Aug 2, 2021
@jeromelaban jeromelaban added project/input ⌨️ Categorizes an issue or PR as relevant to input (Button, CheckBox, Toggle, Scroll, Map, Numeric,...) project/text 🔤 Categorizes an issue or PR as relevant to text (TextBox, PasswordBox, TextBlock, Fonts, …) and removed triage/untriaged Indicates an issue requires triaging or verification labels Aug 3, 2021
@jeromelaban
Copy link
Member

jeromelaban commented Aug 3, 2021

Adding the missing glyph in https://github.com/unoplatform/uno.fonts is likely the way to fix this.

GitHub
Uno Platform Open Source Fonts. Contribute to unoplatform/uno.fonts development by creating an account on GitHub.

@robloo
Copy link
Contributor Author

robloo commented Aug 3, 2021

My assumption was the glyph was the same as is used in the ComboBox. However, I couldn't find the ComboBox style when I first looked (very quickly).

For my own custom controls, I use the newer 0xE70D Unicode point which Uno includes in the default symbol font. The easiest would be to switch to this newer Unicode point as nearly all chevrons are the same. That would make it a XAML-only change but then any style updates from upstream would clear the change again.

@jeromelaban
Copy link
Member

Here's the glyph used for combobox:

It's probably already modified somehow from the original version. We'll certainly need to readjust for the new WinUI 2.6 styles in #6374.

@robloo
Copy link
Contributor Author

robloo commented Aug 4, 2021

Thanks for pointing me to the style. Both DropDownButton and ComboBox are using the same Unicode point. Only DropDownBox is using a TextBlock instead of FontIcon. Why is ComboBox switching to a TextBlock only for Android? This is strange to me.

Reguardless, this proves it isnt a font issue.

I'm going to try a few other things but someone hopefully can confirm they see the same issue. This may be a strange case on my end.

@robloo
Copy link
Contributor Author

robloo commented Aug 5, 2021

I reproduced this issue with a brand-new, empty app so it's isn't something caused as a secondary issue. It really confuses me:

  1. Both DropDownButton and ComboBox are using the exact same Unicode point 0xE0E5 so it isn't a font issue
  2. The correct font family is specified: SymbolThemeFontFamily and I double checked spelling
  3. ComboBox is switching to a TextBlock ONLY for Android. This is suspicious and something I can't explain. Regardless, DropDownButton is already using a TextBlock.

It's like the DropDownButton style can't find the SymbolThemeFontFamily resource.

DropDownButton:

<TextBlock
x:Name="ChevronTextBlock"
Grid.Column="1"
FontFamily="{ThemeResource SymbolThemeFontFamily}"
FontSize="12"
Text="&#xE0E5;"
VerticalAlignment="Center"
Margin="6,0,0,0"
IsTextScaleFactorEnabled="False"
AutomationProperties.AccessibilityView="Raw"/>

ComboBox:

<not_android:FontIcon x:Name="DropDownGlyph"
Grid.Row="1"
Grid.Column="1"
IsHitTestVisible="False"
Margin="0,10,10,10"
Foreground="{ThemeResource SystemControlForegroundBaseMediumHighBrush}"
FontFamily="{ThemeResource SymbolThemeFontFamily}"
FontSize="12"
Glyph="&#xE0E5;"
HorizontalAlignment="Right"
VerticalAlignment="Center"
AutomationProperties.AccessibilityView="Raw" />
<android:TextBlock x:Name="DropDownGlyph"
Grid.Row="1"
Grid.Column="1"
IsHitTestVisible="False"
Margin="0,10,10,10"
Foreground="{ThemeResource SystemControlForegroundBaseMediumHighBrush}"
FontFamily="{ThemeResource SymbolThemeFontFamily}"
FontSize="12"
Text="&#xE0E5;"
HorizontalAlignment="Right"
VerticalAlignment="Center"
AutomationProperties.AccessibilityView="Raw" />

@jeromelaban
Copy link
Member

It's like the DropDownButton style can't find the SymbolThemeFontFamily resource.

@davidjohnoliver does this sound familiar ?

@davidjohnoliver
Copy link
Contributor

It's like the DropDownButton style can't find the SymbolThemeFontFamily resource.

@davidjohnoliver does this sound familiar ?

Not really... I'm not sure what would be going wrong here.

@robloo robloo changed the title MUX DropDownButton Doesn't Show Chevron Glyph MUX DropDownButton/SplitButton/ToggleSplitButton Don't Show Chevron Glyph Aug 9, 2021
@robloo
Copy link
Contributor Author

robloo commented Aug 9, 2021

This affects MUX SplitButton and ToggleSplitButton as well, added to the description and title

@Youssef1313
Copy link
Member

Youssef1313 commented Aug 29, 2021

@robloo Which platform are you testing against?

The glyph shows correctly for me in SamplesApp on Android.

image

image

@robloo
Copy link
Contributor Author

robloo commented Aug 29, 2021

@Youssef1313 Thanks for taking a look. I've updated the environment section of the issue description adding more details. This is reproducible in a blank app. I never noticed this in the SamplesApp.

image

@robloo
Copy link
Contributor Author

robloo commented Aug 29, 2021

@Youssef1313 I added a test application for this as well. See the issue description for the link.

@Youssef1313
Copy link
Member

After spending the last 3 hours on this. I think I (partially) figured out things. (Looks a bit embarrassing to take that too much time for the small finding 😄 ):

The confusing part to me is that how using the MUXC.DropDownButton partially works in Android (ie, only the glyph doesn't show correctly) while it's completely not implemented.

@jeromelaban
Copy link
Member

jeromelaban commented Aug 31, 2021

The MUX DropDownButton is implemented in Uno https://github.com/unoplatform/uno/blob/master/src/Uno.UI/UI/Xaml/Controls/DropDownButton/DropDownButton.cs (from @MartinZikmund), it is the one you're referring to?

And no worries about the time ;)

GitHub
Build Mobile, Desktop and WebAssembly apps with C# and XAML. Today. Open source and professionally supported. - uno/DropDownButton.cs at master · unoplatform/uno

@Youssef1313
Copy link
Member

@jeromelaban Then there is some gap I'm not understanding here. I thought it should be in Microsoft.UI.Xaml.Controls namespace as opposed to Windows.UI.Xaml.Controls?

@Youssef1313
Copy link
Member

By the way, here is a workaround (and weird behavior at the same time):

image

@jeromelaban Does the behavior described in the above image points to any possibilities?

@robloo
Copy link
Contributor Author

robloo commented Aug 31, 2021

Yea, it's always something seemingly simple that takes the most time. This one is really strange to me and it's entirely possible I'm overlooking something simple myself.

Apologies for confusion about Windows.UI.Xaml vs Microsoft.UI.Xaml namespaces. (I had momentarily forgotten these were implemented in both). In this issue I am always referring to Microsoft.UI.Xaml.Controls which includes the latest pre-2.6 version 1 styles as well. The repro test app is also using Microsoft.UI.Xaml.Controls I refer to as "MUX" and others may call "MUXC".

@robloo
Copy link
Contributor Author

robloo commented Aug 31, 2021

After looking at the code I agree this is very strange that it works anyway. DropDownButton is missing from the MUX controls. I thought for sure Martin ported this though. Perhaps he used the MUX source but put it in the WUX namespace?

https://github.com/unoplatform/uno/tree/master/src/Uno.UI/Microsoft/UI/Xaml/Controls

How does this work at all? Is code generation automatically adding it to the MUX namespace?

All of this said, SplitButton has the same issue but is implemented as expected: https://github.com/unoplatform/uno/tree/master/src/Uno.UI/Microsoft/UI/Xaml/Controls/SplitButton

GitHub
Build Mobile, Desktop and WebAssembly apps with C# and XAML. Today. Open source and professionally supported. - uno/src/Uno.UI/Microsoft/UI/Xaml/Controls at master · unoplatform/uno
GitHub
Build Mobile, Desktop and WebAssembly apps with C# and XAML. Today. Open source and professionally supported. - uno/src/Uno.UI/Microsoft/UI/Xaml/Controls/SplitButton at master · unoplatform/uno

@Youssef1313
Copy link
Member

@robloo The repro app is and is not using Microsoft.UI.Xaml.Controls. You specifiy Microsoft in XAML, the source generator generates Windows, which seems like a separate bug (probably it just incorrectly falls back to any control with the name if it's not found in the given namespace).

Using Windows instead of Microsoft still reproduces the issue (which is expected since the generated code didn't change).

@robloo
Copy link
Contributor Author

robloo commented Aug 31, 2021

You specifiy Microsoft in XAML, the source generator generates Windows, which seems like a separate bug (probably it just incorrectly falls back to any control with the name if it's not found in the given namespace).

Ok, that answers the big question of how this even works to begin with. DropDownButton is not implemented in Microsoft.UI.Xaml.Controls as we both noted. My guess is this is an intentional fallback though -- however unexpected it might be. I certainly think this needs to change long term: we should do what was finally decided for NavigationView and put it in the MUX directory (even though those same sources were used for WUX version).

@Youssef1313
Copy link
Member

however unexpected it might be

Yes it's very unexpected. At minimum, I think the generator should produce a warning. So the compilation doesn't fail but the developer is informed that the control doesn't exist in namespace X and we'll use namespace Y.

This is, however, a little bit separate from the issue here.

I think the questions are:

  • How does this work correctly in SamplesApp, but not outside of it.
  • How does adding a dummy empty class that derives from DropDownButton fixes the issue?

I was never able to debug an external app following these instructions, so it's hard to debug for me without reproducing in samples app 😕

@Youssef1313
Copy link
Member

I'm very suspicious about this:

<TextBlock
x:Name="ChevronTextBlock"
Grid.Column="1"
FontFamily="Segoe MDL2 Assets"
FontSize="12"
Text="&#xE70D;"
VerticalAlignment="Center"
Margin="6,0,0,0"
IsTextScaleFactorEnabled="False"
AutomationProperties.AccessibilityView="Raw"/>

@robloo
Copy link
Contributor Author

robloo commented Aug 31, 2021

I was never able to debug an external app following these instructions, so it's hard to debug for me without reproducing in samples app 😕

I haven't been able to build Uno for quite some time for some reason. So I won't be able to test this locally and step into Uno source. I've had the setup working in the past on some other issues though. When I finish work on performance and work-arounds I'll have bugs like this to address. So eventually I will circle back to issues I file and spend more time on them. I decided just to file them so they are known and discussed. That said, issues like this are outside my area of knowledge and I'm not sure where to start.

@robloo
Copy link
Contributor Author

robloo commented Aug 31, 2021

@Youssef1313 That's almost certainly it! I was never sure about those styles and they are organized strangely to me.

#4435
#6374 (comment)

Note that all occurrences of FontFamily="Segoe MDL2 Assets" were fixed upstream.

microsoft/microsoft-ui-xaml#3745
microsoft/microsoft-ui-xaml#3923

That needs to be changed here as well to FontFamily="{ThemeResource SymbolThemeFontFamily}" . The resources are definitely very confusing here and likely incorrect. The priority folders really need to be removed IMO.

@jeromelaban
Copy link
Member

jeromelaban commented Sep 1, 2021

@robloo take a look at this video I made yesterday for building uno: https://www.twitch.tv/videos/1135290868

Twitch
jeromelaban went live on Twitch. Catch up on their Science & Technology VOD now.

@robloo
Copy link
Contributor Author

robloo commented Sep 1, 2021

@jeromelaban Thanks, I did have the full setup working last year -- including debugging an app with Uno source. However, every few months the build breaks and I have to spend a lot of time figuring out what went wrong. Last time it broke I wasn't able to fix it within a few hours. It was at the time related to the VS issue you filed that would just silently refuse to build some projects. I'm not sure why it's so fragile.

@jeromelaban
Copy link
Member

If you encounter this again, open an issue with the build errors, we can certainly enhance the build validations for the errors to be more explicit.

@robloo
Copy link
Contributor Author

robloo commented Sep 1, 2021

If you encounter this again, open an issue with the build errors, we can certainly enhance the build validations for the errors to be more explicit.

Will do. In the issue I'm thinking of it was Visual Studio and there was nothing you could do. You filed an issue for it but I can't find it at the moment to see if it was closed.

@robloo
Copy link
Contributor Author

robloo commented Sep 20, 2021

Confirmed to work in Uno.UI version 3.10.7!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty/tbd Categorizes an issue for which the difficulty level needs to be defined. kind/bug Something isn't working project/input ⌨️ Categorizes an issue or PR as relevant to input (Button, CheckBox, Toggle, Scroll, Map, Numeric,...) project/text 🔤 Categorizes an issue or PR as relevant to text (TextBox, PasswordBox, TextBlock, Fonts, …)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants