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 [Windows] MAUI Entry in Windows always shows ClearButton despite ClearButtonVisibility set to 'Never' #25567

Merged
merged 19 commits into from
Dec 18, 2024

Conversation

NirmalKumarYuvaraj
Copy link
Contributor

@NirmalKumarYuvaraj NirmalKumarYuvaraj commented Oct 29, 2024

Issue Details

Clear Button Visibility Issue in Entry Control (Windows): Even when ClearButtonVisibility is set to "Never," the Clear button appears within the Entry control on Windows. This issue is specifically observed when MinimumWidthRequest is set; without this property, the Clear button behaves as expected and remains hidden.

Root Cause

The issue stems from the layout behaviour of WinUI, where setting ClearButtonVisibility to Never and adjusting the delete button's column width to zero does not completely eliminate the button's effect on the layout. This remaining effect on the layout causes the delete button to appear occasionally or shift within the Entry control, particularly during layout adjustments.

Description of Change

This update resolves the issue by explicitly setting IsEnabled on the delete button. When ClearButtonVisibility is set to Never, the button is disabled, ensuring it remains non-interactive and hidden, regardless of layout changes. When visibility is allowed, the button is re-enabled. This approach eliminates reliance on WinUI’s opacity handling, resulting in consistent behaviour for the Entry control across different layouts.

Validated the behaviour in the following platforms

  • Android
  • Windows
  • iOS
  • Mac

Issues Fixed

Fixes #25473

Before After
BeforeFix.mp4
AfterFix.mp4

Copy link
Contributor

Hey there @NirmalKumarYuvaraj! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Oct 29, 2024
@NirmalKumarYuvaraj NirmalKumarYuvaraj changed the title Fixed MAUI Entry in Windows always shows ClearButton despite ClearButtonVisibility set to 'Never' Fixed [Windows] MAUI Entry in Windows always shows ClearButton despite ClearButtonVisibility set to 'Never' Oct 29, 2024
@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

App.WaitForElement("EntryField");
App.Tap("EntryField");
App.EnterText("EntryField", "Entry");
VerifyScreenshot();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test is failing on Android and iOS. There are slightly differences in the snapshot, mostly related with the keyboard.
image
Recommendation, close the keyboard before verify the screenshot or unfocus the Entry. The cursor animation or the keyboard could impact in the screenshot validation.

Copy link
Contributor

@jsuarezruiz jsuarezruiz Nov 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are a couple of test failing on iOS:
image

The visual differences are related with the keyboard. Could you close the keyboard?

App.DismissKeyboard();

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jsuarezruiz To accurately ensure ClearButtonVisibility, the entry must stay focused. Since the entry with focused state causes the keyboards to remain open in the view, resulting in failures, I changed the test case to exclude other platforms because the actual fix only relates to Windows.

@NirmalKumarYuvaraj NirmalKumarYuvaraj marked this pull request as ready for review October 30, 2024 10:21
@NirmalKumarYuvaraj NirmalKumarYuvaraj requested a review from a team as a code owner October 30, 2024 10:21
@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).


[Test]
[Category(UITestCategories.Entry)]
public void VerifyEntryClearButtonVisibilitySetToNever()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, the test is failing on Android and iOS because the reference snapshot is pending. Could you include them?

@rmarinho
Copy link
Member

rmarinho commented Nov 1, 2024

/rebase

@rmarinho
Copy link
Member

rmarinho commented Nov 1, 2024

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

xmlns:local="clr-namespace:Maui.Controls.Sample.Issues"
x:Class="Maui.Controls.Sample.Issues.Issue25473">

<VerticalStackLayout>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but, could the test include two Entries, using the two https://learn.microsoft.com/es-es/dotnet/api/microsoft.maui.clearbuttonvisibility?view=net-maui-8.0 enum values?

Will require to validate each one with a snapshot but in this way we have a test verifying all the ClearButtonVisibility possibility.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jsuarezruiz ,Thank you for the feedback! Based on your suggestion, I have added the additional test cases to include both ClearButtonVisibility enum values. Each entry is now validated with a snapshot, ensuring we have comprehensive coverage for all ClearButtonVisibility scenarios. Please let me know if there are any further adjustments needed.

@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@dotnet dotnet deleted a comment from azure-pipelines bot Nov 8, 2024
@jsuarezruiz
Copy link
Contributor

/rebase

@jsuarezruiz
Copy link
Contributor

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

Copy link
Contributor

@jsuarezruiz jsuarezruiz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Failing tests on iOS on CI:
image

@jsuarezruiz
Copy link
Contributor

jsuarezruiz commented Nov 13, 2024

/azp run

Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jsuarezruiz
Copy link
Contributor

/rebase

@NirmalKumarYuvaraj NirmalKumarYuvaraj added the partner/syncfusion Issues / PR's with Syncfusion collaboration label Dec 6, 2024
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@jfversluis jfversluis dismissed jsuarezruiz’s stale review December 18, 2024 09:32

Feedback addressed

@jfversluis jfversluis merged commit b8c7efc into dotnet:main Dec 18, 2024
104 checks passed
@jfversluis jfversluis added this to the .NET 9 SR3 milestone Dec 18, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jan 18, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-controls-entry Entry community ✨ Community Contribution partner/syncfusion Issues / PR's with Syncfusion collaboration platform/windows 🪟
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MAUI Entry in Windows always shows ClearButton despite ClearButtonVisibility set to 'Never'
6 participants