[Android, iOS, Mac] Fixed Entry ClearButton not visible on dark theme Change#32889
[Android, iOS, Mac] Fixed Entry ClearButton not visible on dark theme Change#32889jfversluis merged 5 commits intodotnet:mainfrom
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where the Entry control's clear button doesn't update its color properly when the app theme switches between light and dark modes when TextColor is not explicitly set.
Key changes:
- iOS: Sets
clearButton.TintColor = nullwhen Entry.TextColor is null to allow automatic theme-based coloring - Android: Retrieves the system's TextColorPrimary attribute and applies it to the clear button when Entry.TextColor is null
- Adds UI tests to verify clear button visibility in both light and dark themes
Reviewed changes
Copilot reviewed 4 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Core/src/Platform/iOS/TextFieldExtensions.cs | Updates iOS clear button handling to set TintColor to null when TextColor is not specified, enabling automatic theme adaptation |
| src/Core/src/Platform/Android/EditTextExtensions.cs | Updates Android clear button handling to use system TextColorPrimary attribute when TextColor is not specified |
| src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue32886.cs | Adds UI tests to verify clear button visibility in both light and dark themes |
| src/Controls/tests/TestCases.HostApp/Issues/Issue32886.cs | Adds test page that demonstrates the clear button behavior with theme switching |
| src/Controls/tests/TestCases.iOS.Tests/snapshots/ios/EntryClearButtonShouldBeVisibleOnLightTheme.png | Snapshot for iOS light theme test verification |
| src/Controls/tests/TestCases.iOS.Tests/snapshots/ios/EntryClearButtonShouldBeVisibleOnDarkTheme.png | Snapshot for iOS dark theme test verification |
| src/Controls/tests/TestCases.Android.Tests/snapshots/android/EntryClearButtonShouldBeVisibleOnLightTheme.png | Snapshot for Android light theme test verification |
| src/Controls/tests/TestCases.Android.Tests/snapshots/android/EntryClearButtonShouldBeVisibleOnDarkTheme.png | Snapshot for Android dark theme test verification |
| int[] EnabledState = [global::Android.Resource.Attribute.StateEnabled]; | ||
| var color = new global::Android.Graphics.Color(cs.GetColorForState(EnabledState, Colors.Black.ToPlatform())); |
There was a problem hiding this comment.
The local variable EnabledState should use camelCase naming convention (enabledState) to follow C# coding standards. Local variables should not use PascalCase.
| int[] EnabledState = [global::Android.Resource.Attribute.StateEnabled]; | |
| var color = new global::Android.Graphics.Color(cs.GetColorForState(EnabledState, Colors.Black.ToPlatform())); | |
| int[] enabledState = [global::Android.Resource.Attribute.StateEnabled]; | |
| var color = new global::Android.Graphics.Color(cs.GetColorForState(enabledState, Colors.Black.ToPlatform())); |
jfversluis
left a comment
There was a problem hiding this comment.
Test is failing on Android and needs screenshots on Windows and macOS. Please also address the feedback issue about the variable name. Thanks!
|
Hi @@TamilarasanSF4853. We have added the "s/pr-needs-author-input" label to this issue, which indicates that we have an open question/action for you before we can take further action. This PRwill be closed automatically in 14 days if we do not hear back from you by then - please feel free to re-open it if you come back to this PR after that time. |
@jfversluis I addressed the issue and made the necessary changes. |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
🤖 AI Summary📊 Expand Full Review🔍 Pre-Flight — Context & Validation📝 Review Session — added snaps ·
|
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #32889 | Android: Query TextColorPrimary from theme; iOS: TintColor= PENDING (Gate) | EditTextExtensions.cs, TextFieldExtensions.cs |
Original PR | null |
🚦 Gate — Test Verification
📝 Review Session — added snaps · 42ab1ce
Result PASSED:
Platform: android
Mode: Full Verification
Test Filter: Issue32886
- Tests FAIL without fix
- Tests PASS with fix
Conclusion
The tests correctly validate the fix and catch the bug when it's present. The test suite properly reproduces issue #32886, and the fix successfully resolves the issue on Android.
🔧 Fix — Analysis & Comparison
📝 Review Session — added snaps · 42ab1ce
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix (claude-sonnet-4.5) | Use on Android; on iOS | PASS | 2 files | Uses hint color as proxy; semantically less precise |
| 2 | try-fix (claude-opus-4.6) | Use directly | PASS | 1 file | Minimal (3 lines, no API check); may have timing edge case on theme change |
| 3 | try-fix (gpt-5.2) | Use attr + | PASS | 1 file | Good, but more complex, API 21 check, different semantic |
| 4 | try-fix (gpt-5.2-codex) | Use (ColorStateList) + | PASS | 1 file | State-aware, clean; API 21 check |
| 5 | try-fix (gemini-3- | Rate limit error after 2 retries | SKIPPED | pro- | preview) |
| PR | PR #32889 | Query via , get enabled-state color, apply as | PASS (Gate) | 2 files | Consistent with existing UpdateTextColor pattern in same file |
Exhausted: Yes (Round 2 - all 4 available models confirmed NO NEW IDEAS)
Selected Fix: PR's fix
Reason: The PR's approach is the most consistent with existing code patterns in EditTextExtensions. the nearly-identical UpdateTextColormethod uses the sameObtainStyledAttributes(TextColorPrimary) pattern. It explicitly queries the theme (not runtime state that might be stale on first call), handles the enabled state deliberately, and includes a proper fallback for Android < API 23. While Attempt 2 (CurrentTextColor) is simpler, it relies on the view's runtime text color being already resolved, which may have subtle timing edge cases when the theme changes.cs
Root Cause Analysis
Android: UpdateClearButtonColor was calling clearButtonDrawable?.ClearColorFilter() when textColor is null, which removed any tint, leaving the drawable using its inherent colors. The original clear button drawable is dark (designed for light backgrounds), so it became invisible on dark theme backgrounds.
iOS: clearButton.SetImage(defaultClearImage, ...) was reusing the same image captured in light theme. The image had baked-in colors, so iOS's automatic dark/light adaptation couldn't update it. Setting clearButton.TintColor = null instead allows the system's normal button color logic to apply, which automatically adapts to the current theme.
Alternative Analysis
Multiple alternative approaches pass the tests (Attempts 1-4). The PR's fix is preferred because:
- Consistent with existing codebase pattern (
UpdateTextColorin same file) - More explicit intent (not relying on stale runtime state)
- Has proper API level fallback
📋 Report — Final Recommendation
📝 Review Session — added snaps · 42ab1ce
Final Recommendation: APPROVE
Summary
PR #32889 fixes the Entry clear button not updating its color when the app switches between light and dark themes (when TextColor is not explicitly set). The fix is correct, well-tested, and consistent with existing code patterns in the repository.
Root Cause
Android: UpdateClearButtonColor in EditTextExtensions.cs was calling clearButtonDrawable?.ClearColorFilter() when textColor is removing any tint from the drawable. The clear button drawable has inherent dark colors (designed for light backgrounds), so it became invisible on dark theme backgrounds.null
iOS: UpdateClearButtonColor in TextFieldExtensions.cs was calling clearButton.SetImage(defaultClearImage, ...) with an image captured in light theme. This baked-in the light-theme colors, preventing iOS's automatic dark/light adaptation.
Fix Quality
The PR's approach is correct and optimal:
Android: Queries TextColorPrimary from the theme via ObtainStyledAttributes and applies the enabled-state color as a SetColorFilter. This is exactly the same pattern used in the existing UpdateTextColor method in the same highly consistent. Includes proper fallback for Android < API 23.file
iOS: Setting clearButton.TintColor = null allows the system to apply the appropriate tint based on the current theme. Clean, minimal, and idiomatic iOS.
Alternative Analysis
4 alternative approaches were tested (all pass):
editText. uses hint color proxy (semantically less precise)CurrentHintTextColoreditText. simpler (3 lines, no API check) but relies on runtime state that may be stale on first renderCurrentTextColorcolorControlNormal+different semantic (control chrome vs text color)SetTintListeditText.TextColors+state-aware but requires API 21SetTintList
The PR's fix is preferred: more explicit, consistent with codebase patterns, and doesn't depend on the view's runtime-resolved state.
Code Review
**
-
Missing
ClearColorFilter()fallback whencsis If 23 but the theme has noTextColorPrimaryColorStateList (unusual but possible), the code falls through without clearing any stale filter. Addingelse { clearButtonDrawable?.ClearColorFilter(); }after theif (cs is not null)block would be defensive. However, all standard Android themes includeTextColorPrimary, so this is an academic edge case.Android null -
Colors.Black.ToPlatform()as the fallback incs.GetColorForState(enabledState, Colors.Black. In dark mode, a black button on a black background would be invisible. The fallback is only used if the state doesn't match any ColorStateList entry, which won't occur withStateEnabledin standard themes. ButColors.White.ToPlatform()would be slightly safer, or simply usingClearColorFilter()as fallback.ToPlatform()) -
EntryClearButtonShouldBeVisibleOnDarkThemeOn Android, the test doesn't tapTestEntryafter theme change before callingVerifyScreenshot(). The test relies onTestEntrybeing focused from the Order(1) test. If run in isolation, it would not show the clear button. AddingApp.Tap("TestEntry");beforeVerifyScreenshot()on Android would make the test independent.test
** What Looks Good:**
- iOS fix is clean and minimal (2 lines changed)
- Android fix is consistent with the existing
UpdateTextColorpattern in the same file - camelCase
enabledStatevariable follows C# conventions - Explanatory comment added to iOS change
- Tests cover both light and dark themes
- Snapshots provided for all 4 platforms
- Platform validation checklist completed
Title Suggestion
Current: [Android, iOS, Mac] Fixed Entry ClearButton not visible on dark theme Change
Recommended: [Android, iOS, Mac] Entry: Fix ClearButton color not updating on theme switch
(Remove "Fixed ... Change" phrasing; use "Fix" imperative form; make behavior description precise)
PR Description
Good structure - keep as-is. Has NOTE block, root cause, description of change, platform validation, issue link, and before/after output images. Minor fix: remove stray comma after "Windows ,".
📋 Expand PR Finalization Review
Title: ✅ Good
Current: [Android, iOS, Mac] Fixed Entry ClearButton not visible on dark theme Change
Description: ✅ Good
Description needs updates. See details below.
✨ Suggested PR Description
[!NOTE]
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Issue Details
When TextColor is not specified and the app theme switches between dark and light mode, the ClearButton on Entry does not update its color correctly — remaining invisible on dark backgrounds.
Root Cause
Android: When entry.TextColor is null, the previous code called ClearColorFilter() on the clear button drawable, which removed any tint and left the icon using its raw drawable color (white on white in dark theme, invisible). No theme-appropriate color was applied.
iOS/MacCatalyst: When entry.TextColor is null, the previous code set the clear button image with SetImage(defaultClearImage, ...), reusing the same image created in the light theme. This bypassed iOS's automatic theme-adaptive rendering, freezing the button in its original color regardless of subsequent theme changes.
Description of Change
Android (EditTextExtensions.cs):
When TextColor is null, now reads TextColorPrimary from the current theme's styled attributes and applies it as a color filter on the clear button drawable. This correctly adapts to both light and dark themes. Requires API 23+ (Android 6.0 Marshmallow); on older API levels the original ClearColorFilter() fallback is retained.
iOS/MacCatalyst (TextFieldExtensions.cs):
When TextColor is null, now sets clearButton.TintColor = null instead of pinning a specific image. Setting TintColor = null restores UIKit's default behaviour of inheriting the tint from the parent view hierarchy, which is theme-adaptive and correctly follows the system light/dark mode.
Windows: No code change required. The native WinUI TextBox clear button already respects the system theme automatically.
Key Technical Details
- Android API gate: The fix is wrapped in
OperatingSystem.IsAndroidVersionAtLeast(23)becauseObtainStyledAttributeswith theTextColorPrimaryattribute requires API 23. Devices on API 21/22 (< Android 6.0) fall back to the oldClearColorFilter()path and may still show the issue, but such devices represent a tiny fraction of the install base and MAUI's effective minimum API is 21. - iOS TintColor semantics:
TintColor = nullcausesUIButtonto inherit tint from its superview chain, which UIKit updates automatically on trait collection changes (dark/light mode transitions). This is simpler and more correct than managing images manually.
Issues Fixed
Fixes #32886
Output Images
Android
| Before | After |
Screen.Recording.2025-11-27.at.8.02.46.PM.mov |
Screen.Recording.2025-11-27.at.8.00.16.PM.mov |
Platforms Tested
- Android
- iOS
- MacCatalyst
- Windows
Code Review: ✅ Passed
Code Review — PR #32889
🟡 Medium Issues
1. Inline #if Directives in UI Test Methods
Files:
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue32886.cs
Problem:
Both test methods (EntryClearButtonShouldBeVisibleOnLightTheme and EntryClearButtonShouldBeVisibleOnDarkTheme) use inline #if ANDROID, #if IOS, and #if WINDOWS preprocessor directives directly in the test body. The MAUI UI test guidelines explicitly prohibit this pattern:
Do NOT use
#if ANDROID,#if IOS, etc. directly in test methods. Platform-specific behavior must be hidden behind extension methods for readability.
// ❌ Current pattern (violates project conventions)
#if ANDROID
if (App.WaitForKeyboardToShow(timeout: TimeSpan.FromSeconds(1)))
{
App.DismissKeyboard();
}
#endif
#if IOS
VerifyScreenshot(cropBottom: 1550);
#else
VerifyScreenshot();
#endifRecommendation: Extract the platform-specific behavior into extension methods. For example:
// ✅ Recommended: extension method approach
App.TapAndDismissKeyboardIfNeeded("TestEntry");
App.VerifyScreenshotExcludingKeyboard();2. Android Fallback Leaves Clear Button Invisible on API 21–22
File: src/Core/src/Platform/Android/EditTextExtensions.cs
Problem:
The fix is gated behind OperatingSystem.IsAndroidVersionAtLeast(23). On API 21/22 (Android 5.x), the else branch still calls clearButtonDrawable?.ClearColorFilter(), which was exactly the code that caused the invisible clear button. The bug is therefore unresolved for the rare set of API 21/22 devices.
else
{
// API 21/22 fallback — still invisible on dark theme
clearButtonDrawable?.ClearColorFilter();
}Recommendation: Consider a simple white/black color selection based on the EditText's background luminance, or check if there's a pre-API-23 way to get a suitable text color. If the maintainer intentionally accepts this trade-off for a tiny audience, a code comment documenting the limitation would be valuable:
else
{
// API < 23: ObtainStyledAttributes requires API 23 for theme-based color lookup.
// On these rare devices, fall back to clearing the filter (may be invisible on dark theme).
clearButtonDrawable?.ClearColorFilter();
}🔵 Minor Nits
3. Missing Newline at End of Files
Both new test files (Issue32886.cs in HostApp and SharedTests) are missing a trailing newline (\ No newline at end of file in the diff). This is a minor style issue but consistent with the rest of the codebase.
4. [Test, Order(n)] Usage
The test methods use [Test, Order(1)] and [Test, Order(2)]. Ordered tests create an implicit dependency between EntryClearButtonShouldBeVisibleOnLightTheme (which must run first to establish the light-theme screenshot baseline) and EntryClearButtonShouldBeVisibleOnDarkTheme (which clicks ThemeButton). This is intentional since the dark test depends on the current theme state after the light test, but it's worth noting the ordering is load-bearing — if the runner skips test 1, test 2 will start in whatever state the app was left in from a previous test.
✅ Looks Good
- iOS fix is clean and correct: Setting
clearButton.TintColor = nullis the idiomatic iOS way to restore system-default tint color and theme-adaptivity. Far simpler than the previous approach of re-pinning images. - Android fix correctly uses
TypedArraywithusing:tais properly disposed viausing var ta = ..., preventing resource leaks. - Test host app uses
UITestEntry: Correctly uses the UITest-optimized entry control withIsCursorVisible = falseto prevent cursor blink from causing flaky screenshot tests. - Test uses
SetAppThemeColor: Page background correctly adapts to dark/light theme to make the clear button visibility easily verifiable via screenshot. - Both core fixes are minimal: Only the exact problematic code paths are modified — no unnecessary refactoring.
- Test snapshots included for all 4 platforms: Android, iOS, Mac, and Windows baseline screenshots are all present.
… Change (dotnet#32889) <!-- Please let the below note in for people that find this PR --> > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! <!-- !!!!!!! MAIN IS THE ONLY ACTIVE BRANCH. MAKE SURE THIS PR IS TARGETING MAIN. !!!!!!! --> ### Issue Details - When TextColor is not specified and the theme is switched between dark and light (and vice versa), the clearButton color in the Entry does not update correctly. ### Root cause - On iOS, The root cause of the issue is that on iOS, when the text color is null, the clearButton.SetImage method reuses the same image created in the light theme. This breaks iOS’s automatic theme handling, causing the clear button color to remain unchanged. - On Android, When entry.TextColor is null (default), the clear button drawable's color filter was being cleared without applying the appropriate theme color, making the button invisible on dark backgrounds. ### Description of Change - On Android, When TextColor is `null`, now properly retrieves the system's `TextColorPrimary` attribute. Applies the correct color filter to the clear button drawable. Respects the app theme by using the enabled state color from the theme's color state list - On iOS, When TextColor is `null`, now sets `clearButton.TintColor = null` to use system default. Allows the clear button to adapt to the current theme automatically. Ensures tinted clear button image uses correct system colors Validated the behaviour in the following platforms - [x] Android - [x] Windows , - [x] iOS, - [x] MacOS ### Issues Fixed Fixes dotnet#32886 ### Output images Android <table> <tr> <td> Before </td> <td> After </td> </tr> <tr> <td> https://github.com/user-attachments/assets/cde1145b-1440-442f-9cca-f7a2790500ab </td> <td> https://github.com/user-attachments/assets/b072641b-182f-4189-843f-589261da90cc </td> </tr> </table>
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Issue Details
Root cause
Description of Change
null, now properly retrieves the system'sTextColorPrimaryattribute. Applies the correct color filter to the clear button drawable. Respects the app theme by using the enabled state color from the theme's color state listnull, now setsclearButton.TintColor = nullto use system default. Allows the clear button to adapt to the current theme automatically. Ensures tinted clear button image uses correct system colorsValidated the behaviour in the following platforms
Issues Fixed
Fixes #32886
Output images
Android
Screen.Recording.2025-11-27.at.8.02.46.PM.mov
Screen.Recording.2025-11-27.at.8.00.16.PM.mov