-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Fixed Editor vertical text alignment not working after toggling IsVisible #26194
base: main
Are you sure you want to change the base?
Conversation
Hey there @NanthiniMahalingam! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
@@ -30,12 +30,12 @@ protected override void ConnectHandler(TextBox platformView) | |||
{ | |||
platformView.TextChanged += OnTextChanged; | |||
platformView.LostFocus += OnLostFocus; | |||
platformView.Loaded += OnPlatformLoaded; | |||
platformView.SizeChanged += OnPlatformViewSizeChanged; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mnnn, I am concerned about the performance impact. Each invalidation that impacts the size of the control will cause the VerticalTextAlignment
and DeleteButton
to be updated, which in both cases will look for Descendants or apply changes to the Layout, sometimes without being necessary. For example, a page with several Editors, and resizing the Window will perform the previous updates several times (without being necessary).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mnnn, I am concerned about the performance impact. Each invalidation that impacts the size of the control will cause the
VerticalTextAlignment
andDeleteButton
to be updated, which in both cases will look for Descendants or apply changes to the Layout, sometimes without being necessary. For example, a page with several Editors, and resizing the Window will perform the previous updates several times (without being necessary).
Hi @jsuarezruiz ,
The Loaded event does not trigger when the Editor visibility changes, which prevents the alignment from being set in this case.
To address this, we used the SizeChanged event instead of the Loaded event. This approach is consistent with the method followed in the Entry control to update properties like VerticalTextAlignment and DeleteButton.
Refer to the Entry code: https://github.com/dotnet/maui/blob/main/src/Core/src/Handlers/Entry/EntryHandler.Windows.cs#L37
As this approach sets the respective native property, such as assigning the VerticalTextAlignment value to the native text box's VerticalAlignment property, we believe that resizing the window will only reapply the same value to the native property. It will not trigger a property change or affect any layout operations.
If you feel this approach is not appropriate for both Entry and Editor, we will explore other alternatives.
Issue Details
Editor is initially configured with VerticalTextAlignment set to End or Center and Visibility set to False, toggling the Visibility causes the vertical text alignment to unexpectedly reset to the default Start alignment.
Root Cause:
The VerticalTextAlignment value is updated during the Loaded event. However, toggling the Visibility does not re-trigger the Loaded event, resulting in the attached properties not being invalidated.
Description of Change:
Invalidate the MauiTextBox attached properties on size changes, During the Loaded or when Visibility is toggled, This ensures the VerticalTextAlignment is correctly reapplied to maintain the updated state.
Fixes #25973
Validated the behaviour in the following platforms
Output
Before-Fix.mp4
After-Fix.mp4