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
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
22 changes: 22 additions & 0 deletions src/Controls/tests/TestCases.HostApp/Issues/Issue25973.xaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<?xml version="1.0" encoding="utf-8" ?>
<ContentPage xmlns="http://schemas.microsoft.com/dotnet/2021/maui"
xmlns:x="http://schemas.microsoft.com/winfx/2009/xaml"
x:Class="Maui.Controls.Sample.Issues.Issue25973">

<ScrollView>
<VerticalStackLayout>
<Editor
x:Name="editor"
AutomationId="Editor"
Text="Editor Text in Vertical End"
IsVisible="False"
VerticalTextAlignment="End"
HeightRequest="100"/>
<Button
x:Name="toggleButton"
AutomationId="ToggleButton"
Text="Change Visibility"
Clicked="VisibilityButtonClicked"/>
</VerticalStackLayout>
</ScrollView>
</ContentPage>
16 changes: 16 additions & 0 deletions src/Controls/tests/TestCases.HostApp/Issues/Issue25973.xaml.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
namespace Maui.Controls.Sample.Issues
{
[Issue(IssueTracker.Github, 25973, "Editor vertical text alignment not working after toggling IsVisible", PlatformAffected.UWP)]
public partial class Issue25973 : ContentPage
{
public Issue25973()
{
InitializeComponent();
}

private void VisibilityButtonClicked(object sender, EventArgs e)
{
editor.IsVisible = true;
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
using NUnit.Framework;
using NUnit.Framework.Legacy;
using UITest.Appium;
using UITest.Core;

namespace Microsoft.Maui.TestCases.Tests.Issues
{
internal class Issue25973 : _IssuesUITest
{
public override string Issue => "Editor vertical text alignment not working after toggling IsVisible";

public Issue25973(TestDevice device) : base(device)
{
}

[Test]
[Category(UITestCategories.Editor)]
public void VerifyEditorVerticalTextAlignmentWhenVisibilityToggled()
{
App.WaitForElement("ToggleButton");
App.Click("ToggleButton");
VerifyScreenshot();
}
}
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
6 changes: 3 additions & 3 deletions src/Core/src/Handlers/Editor/EditorHandler.Windows.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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.

}

protected override void DisconnectHandler(TextBox platformView)
{
platformView.Loaded -= OnPlatformLoaded;
platformView.SizeChanged -= OnPlatformViewSizeChanged;
platformView.TextChanged -= OnTextChanged;
platformView.LostFocus -= OnLostFocus;

Expand Down Expand Up @@ -99,7 +99,7 @@ void OnTextChanged(object sender, TextChangedEventArgs args) =>
void OnLostFocus(object? sender, RoutedEventArgs e) =>
VirtualView?.Completed();

void OnPlatformLoaded(object sender, RoutedEventArgs e) =>
void OnPlatformViewSizeChanged(object sender, RoutedEventArgs e) =>
MauiTextBox.InvalidateAttachedProperties(PlatformView);

private void OnSelectionChanged(object sender, RoutedEventArgs e)
Expand Down