unoplatform / Uno.Themes

This library is designed to help you use the Material, Fluent or Cupertino design system with the Uno Platform
https://platform.uno
Apache License 2.0
167 stars 28 forks source link

TextBox and PasswordBox Font styles are incorrect #1379

Open NVLudwig opened 1 month ago

NVLudwig commented 1 month ago

Current behavior

TextBox and PasswordBox currently use BodyMedium ressource. This causes font overrides to fail as the Figma file uses BodyLarge in line with current Material specs

image

Expected behavior

TextBox and PasswordBox should use BodyLarge ressource

How to reproduce it (as minimally and precisely as possible)

Try and apply a font override by selecting and editing the font style for a TextBox or PasswordBox

Environment

Nuget Package:

Package Version(s):

Affected platform(s):

Anything else we need to know?

All text input controls probably have this issue AutoSuggest etc

kazo0 commented 1 month ago

@NVLudwig

I believe they are already using BodyLarge typography values:

https://github.com/unoplatform/Uno.Themes/blob/aab5b0d108ce2438134de9855a427817498d2b50/src/library/Uno.Material/Styles/Controls/v2/TextBox.xaml#L82-L89

https://platform.uno/docs/articles/external/uno.themes/doc/styles/TextBox.html

https://github.com/unoplatform/Uno.Themes/blob/aab5b0d108ce2438134de9855a427817498d2b50/src/library/Uno.Material/Styles/Controls/v2/PasswordBox.xaml#L53-L56

https://platform.uno/docs/articles/external/uno.themes/doc/styles/PasswordBox.html

NVLudwig commented 1 month ago

hmmm...something is wrong: in Figma if I override the fontFamily for BodyMedium the render in the plugin of a textbox gets updated with the desired override. But if I override BodyLarge no change is applyed in the plugin preview. this is what led us to assume themes was not off...what else can this be then? CC @carldebilly

kazo0 commented 1 month ago

This is the problem I was mentioning before where we have intermediary aliases for the typography resources for some controls so overriding the origin resource won't affect aliases of that resource so we need to be referencing them directly

carldebilly commented 1 month ago

@kazo0, the override is working correctly... it's just not connect to the right resource.

Right now, in Figma, the FontFamily used for the TextBox is BodyLarge but overriding it doesn't affect the TextBox. But if we change the BodyMedium FontFamily, it will affect the TextBox.

That means the Material TextBox is wrongly connected to the BodyMedium resource when it should be connected to the BodyLarge one.

kazo0 commented 1 month ago

@carldebilly / @NVLudwig

Not sure I'm understanding, If I override the BodyLargeFontFamily to Ribeye, it is reflected in the TextBox, no?

image

image

NVLudwig commented 1 month ago

Not working here in Prod desktop with the following

Uno Figma Plugin v3.0.2 Uno.UI v5.1.56 Uno.Themes v4.1.1 Uno.Toolkit.WinUI v5.1.7 Uno.Extensions v4.0.5

image

And not working in QA desktop either Uno Figma Plugin v3.1.0-dev.66 Uno.UI v5.2.7 Uno.Themes v5.0.1 Uno.Toolkit.WinUI v6.0.2 Uno.Extensions v4.1.1 image

carldebilly commented 1 month ago

Ooohh @kazo0 is right! We were confusing the Label (Empty placeholder) with the content.

So that's ok, it works :-) But it looks like some variants in Figma are not using the right text style for labels.

image

kazo0 commented 1 month ago

Ahh, I see. Yeah because technically the "Label" when the field is empty is the header label just scaled up/down. Once this PR is finally unblocked, we can address this and use the proper font typography resource for the header/placeholder separately.

carldebilly commented 1 month ago

Temporary solution: