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

Fixed Editor vertical text alignment not working after toggling IsVisible #26194

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

NanthiniMahalingam
Copy link
Contributor

@NanthiniMahalingam NanthiniMahalingam commented Nov 28, 2024

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

  • Android
  • Windows
  • iOS
  • Mac

Output

Before After
Before-Fix.mp4
After-Fix.mp4

@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Nov 28, 2024
Copy link
Contributor

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.

@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

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;
Copy link
Contributor

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).

Copy link
Contributor Author

@NanthiniMahalingam NanthiniMahalingam Nov 28, 2024

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).

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.

@NanthiniMahalingam NanthiniMahalingam marked this pull request as ready for review November 29, 2024 13:08
@NanthiniMahalingam NanthiniMahalingam requested a review from a team as a code owner November 29, 2024 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-controls-editor Editor community ✨ Community Contribution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Editor vertical text alignment not working after toggling IsVisible
3 participants