uno icon indicating copy to clipboard operation
uno copied to clipboard

fix: Match TextBox.VerticalContentAlignment with Windows

Open Youssef1313 opened this issue 2 years ago • 7 comments
trafficstars

GitHub Issue (If applicable): closes #4502.

This is a revert of https://github.com/unoplatform/uno/pull/4180

PR Type

What kind of change does this PR introduce?

What is the current behavior?

What is the new behavior?

Copilot Summary

🤖 Generated by Copilot at 5dd22d4

This pull request fixes a layout bug in TextBox controls and adds a unit test to verify the fix. It also improves the XAML loading logic by using .NET 7.0 source generators and a helper method LoadXaml.

PR Checklist

Please check if your PR fulfills the following requirements:

Other information

Internal Issue (If applicable):

Youssef1313 avatar Sep 25 '23 18:09 Youssef1313

/azp run

Youssef1313 avatar Sep 26 '23 06:09 Youssef1313

Azure Pipelines successfully started running 2 pipeline(s).

azure-pipelines[bot] avatar Sep 26 '23 06:09 azure-pipelines[bot]

There is a problem on Skia where the native Gtk.Entry (shown only on focus), has the text centered vertically, and Gtk.Entry provides no way to change this behavior.

So:

  • Existing behavior on Skia: On both focused & unfocused state, the text is centered vertically.
  • New behavior on Skia: On focus, it's centered vertically. On unfocus, the text is top-aligned.
  • Windows behavior: Text is top-aligned.

So while this PR gets closer to Windows behavior on Skia, I think it could be considered worse because of the inconsistency between focused and unfocused states.

Youssef1313 avatar Sep 27 '23 07:09 Youssef1313

I guess @ramezgerges work will probably help unblock this. Let's see :)

Youssef1313 avatar Oct 10 '23 05:10 Youssef1313

@Youssef1313 I think the new behavior is acceptable - it will work properly with rendered TextBox and it works well with WPF

MartinZikmund avatar Feb 19 '24 10:02 MartinZikmund

@MartinZikmund Yup it should work well with the new TextBox implementation, but not the old one which is still the "default":

https://github.com/unoplatform/uno/blob/efa47a5bb0a567e6f39dc44362d430507f744f2d/src/Uno.UI/FeatureConfiguration.cs#L505

Youssef1313 avatar Feb 19 '24 10:02 Youssef1313

@Youssef1313 but that is the default only for now and will "change" soon, and WPF works fine so I don't see this as a big issue

MartinZikmund avatar Feb 20 '24 08:02 MartinZikmund

@Youssef1313 I think this can be revived now after 5.2

MartinZikmund avatar Mar 25 '24 13:03 MartinZikmund

Yup. Sorry forgot about it earlier

Youssef1313 avatar Mar 25 '24 13:03 Youssef1313

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-13810/index.html

unodevops avatar Apr 07 '24 11:04 unodevops

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-13810/index.html

unodevops avatar Apr 07 '24 12:04 unodevops

🤖 Your WebAssembly Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-13810/index.html

unodevops avatar Apr 07 '24 12:04 unodevops

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-13810/index.html

unodevops avatar Apr 09 '24 14:04 unodevops

🤖 Your Docs stage site is ready! Visit it here: https://unodocsprstaging.z13.web.core.windows.net/pr-13810/index.html

unodevops avatar Apr 15 '24 13:04 unodevops

🤖 Your WebAssembly Sample App stage site is ready! Visit it here: https://unowasmprstaging.z20.web.core.windows.net/pr-13810/index.html

unodevops avatar Apr 15 '24 13:04 unodevops

Ready to be merged!

MartinZikmund avatar Apr 18 '24 08:04 MartinZikmund