Fixed Editor HorizontalTextAlignment does not update at run time#25129
Fixed Editor HorizontalTextAlignment does not update at run time#25129kubaflo merged 28 commits intodotnet:inflight/currentfrom
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/rebase |
c5494b6 to
57421e0
Compare
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
1 similar comment
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
@PureWeen Yes, we have included the UI test cases for the RTL scenario |
428fb27 to
ac297ed
Compare
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 25129Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 25129" |
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 25129Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 25129" |
|
@kubaflo , Addressed test case failure. |
🤖 AI Summary📊 Expand Full Review —
|
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #25129 | Android: update runtime editor alignment by applying both TextAlignment and Gravity; iOS/macOS: relayout placeholder and sync placeholder text alignment via MauiTextView changes |
⏳ PENDING (Gate) | src/Core/src/Platform/Android/TextAlignmentExtensions.cs, src/Core/src/Handlers/Editor/EditorHandler.Android.cs, src/Core/src/Platform/iOS/MauiTextView.cs, src/Core/src/Platform/iOS/TextViewExtensions.cs, test files and snapshots |
Original PR; combines Android fix with broader iOS/macOS behavior changes |
🚦 Gate — Test Verification
Gate Result: ⚠️ SKIPPED
Platform: android
Mode: Full Verification
- Tests FAIL without fix: ❌ Unable to determine (deployment blocked)
- Tests PASS with fix: ❌ Not reached
Evidence
verify-tests-fail-without-fixwas invoked through an isolated task agent as required.- The skill successfully identified PR Fixed Editor HorizontalTextAlignment does not update at run time #25129, reverted the detected fix files to establish the pre-fix state, built the HostApp, and detected Android emulator
emulator-5554. - Verification stopped before test execution because app deployment to the emulator failed with
ADB0010/Mono.AndroidTools.InstallFailedException:Unexpected install output: cmd: Failure calling service package: Broken pipe (32). - Because the without-fix run never reached actual test execution, the with-fix run was not attempted.
Final Status
Gate is skipped due to Android environment/deployment failure rather than a demonstrated pass/fail result for the PR.
🔧 Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix (claude-opus-4.6) | Override MapHorizontalTextAlignment in EditorHandler.Android.cs to explicitly set both TextAlignment and Gravity, preserving vertical gravity |
✅ PASS | 1 file | Narrower Android-only change; simplest passing candidate |
| 2 | try-fix (claude-sonnet-4.6) | Use TextAlignment.Gravity and drive alignment purely through gravity flags in EditTextExtensions.cs |
❌ FAIL | 1 file | Breaks RTL behavior; ViewStart/ViewEnd still required |
| 3 | try-fix (gpt-5.3-codex) | Reapply alignment in EditorHandler.Android on next UI turn via Post() plus layout invalidation |
❌ FAIL | 1 file | Timing workaround still missed visual baselines |
| 4 | try-fix (gemini-3-pro-preview) | Override MapHorizontalTextAlignment in EditorHandler.Android.cs to explicitly set gravity and force RequestLayout() |
✅ PASS | 1 file | Android-only; slightly heavier than candidate #1 |
| 5 | try-fix (round 2) | Store alignment in MauiAppCompatEditText and reapply in OnAttachedToWindow |
✅ PASS | 3 files | Works, but adds platform-view lifecycle state and new Android API surface |
| 6 | try-fix (round 3) | Re-sync gravity from TextAlignment during MauiAppCompatEditText.OnLayout |
✅ PASS | 2 files | Reactive native workaround; broader lifecycle coupling than candidate #1 |
| 7 | try-fix (round 3) | Apply AlignmentSpanStandard to text/hint instead of using view gravity |
❌ FAIL | 4 files | Paragraph-direction alignment conflicts with RTL view-direction expectations |
| 8 | try-fix (round 3) | Preserve MAUI horizontal bits by intercepting Android gravity mutation in MauiAppCompatEditText |
❌ FAIL | 2 files | Not viable with current Android binding/API shape |
| PR | PR #25129 | Update shared Android text alignment helper to always set both TextAlignment and Gravity; plus iOS/macOS placeholder changes |
4 implementation files + tests/snapshots | Original PR bundles Android and iOS/macOS changes; unresolved iOS/macOS review concerns remain |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| claude-opus-4.6 | 1 | Yes | Handler-scoped Android fix candidate completed |
| claude-sonnet-4.6 | 1 | Yes | Gravity-delegation variant failed because RTL mapping broke |
| gpt-5.3-codex | 1 | Yes | Deferred reapply/layout invalidation variant failed visual verification |
| gemini-3-pro-preview | 1 | Yes | Handler-scoped gravity + RequestLayout() candidate completed |
| claude-opus-4.6 | 2 | No | NO NEW IDEAS beyond handler-scoped solution family |
| claude-sonnet-4.6 | 2 | Yes | New platform-view lifecycle idea tested as candidate #5 |
| gpt-5.3-codex | 2 | No | NO NEW IDEAS after failed timing workaround |
| gemini-3-pro-preview | 2 | No | NO NEW IDEAS beyond explicit handler gravity application |
| claude-opus-4.6 | 3 | Yes | New native layout-pipeline idea tested as candidate #6 after OnRtlPropertiesChanged proved unavailable |
| claude-sonnet-4.6 | 3 | Yes | New AlignmentSpan idea tested as candidate #7 and failed |
| gpt-5.3-codex | 3 | Yes | New gravity-preservation interception idea tested as candidate #8 and failed |
| gemini-3-pro-preview | 3 | No | NO NEW IDEAS |
Best Fix Comparison
| Candidate | Passes Tests | Files | Scope | Risks |
|---|---|---|---|---|
| PR | Unknown on Android gate; CI overall failing | 4 impl + tests | Android + iOS/macOS | Mixes concerns; unresolved iOS/macOS review/API questions |
| #1 | Yes | 1 | Android Editor only | Does not generalize to other EditText-based controls |
| #4 | Yes | 1 | Android Editor only | Adds extra layout forcing; more churn than #1 |
| #5 | Yes | 3 | Android Editor platform view | Adds state/lifecycle code and Android API surface |
| #6 | Yes | 2 | Android Editor platform view | Couples behavior to every layout pass |
Exhausted: Yes
Selected Fix: Candidate #1 — it passed Android validation, changes only EditorHandler.Android.cs, avoids shared-helper regressions, and avoids the PR's unrelated iOS/macOS surface area.
📋 Report — Final Recommendation
⚠️ Final Recommendation: REQUEST CHANGES
Phase Status
| Phase | Status | Notes |
|---|---|---|
| Pre-Flight | ✅ COMPLETE | PR mixes Android fix for #10987 with iOS/macOS placeholder changes for #30052; unresolved iOS/macOS review threads remain |
| Gate | Android full verification was blocked by ADB install failure (Broken pipe) before tests ran |
|
| Try-Fix | ✅ COMPLETE | 8 attempts, 4 passing; independent Android-only alternatives found |
| Report | ✅ COMPLETE |
Summary
The Android bug has multiple working fixes, but the best one I found is a smaller Android-only handler change than the PR. Because the PR bundles that Android behavior change together with broader iOS/macOS work that still has open concerns, I would request changes rather than approve this version.
Root Cause
On Android, changing Editor.HorizontalTextAlignment at runtime needs both parts of Android's rendering state updated: the view's TextAlignment for correct RTL-aware semantics and the horizontal Gravity bits for actual placement. Updating only one side leaves the editor visually stale or breaks RTL behavior.
Fix Quality
The PR's Android direction is plausible, but it changes the shared Android text-alignment helper and ships unrelated iOS/macOS placeholder/runtime changes in the same PR. My try-fix exploration found four Android-only passing candidates, with candidate #1 the strongest: it fixes the bug by updating EditorHandler.Android.cs only, keeping the blast radius to Editor instead of every EditText-based control. That makes the current PR harder to justify as-is, especially with unresolved iOS/macOS review/API concerns still present.
|
@kubaflo , The Gate phase failed due to "ADB0010 / Mono.AndroidTools.InstallFailedException: Unexpected install output: cmd: Failure calling service package: Broken pipe (32).". |
🤖 AI Summary📊 Expand Full Review —
|
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #25129 | Android: update runtime alignment by setting both TextAlignment and Gravity; iOS/macOS: relayout placeholder and sync placeholder alignment via MauiTextView changes; add UI/device tests and snapshots |
⏳ PENDING (Gate) | src/Core/src/Platform/Android/TextAlignmentExtensions.cs, src/Core/src/Handlers/Editor/EditorHandler.Android.cs, src/Core/src/Platform/iOS/MauiTextView.cs, src/Core/src/Platform/iOS/TextViewExtensions.cs, tests/snapshots |
Original PR; Android gate requested, but PR also includes broader iOS/macOS behavior changes |
🚦 Gate — Test Verification
Gate Result: ✅ PASSED
Platform: android
Mode: Full Verification
- Tests FAIL without fix: ✅
- Tests PASS with fix: ✅
Evidence
- Verification ran via the required isolated workflow in
verify-tests-fail-without-fix. - Merge base used for the broken baseline:
a1731cfb5ce6f9a53fe0e57a821ec98b76d12767. - Six fix files were reverted to merge-base,
Issue10987failed on Android without the fix, then the same files were restored fromHEADandIssue10987passed. - Verification timestamps from
verification-log.txt: without-fix run completed at16:44:14; with-fix run completed at16:52:57with expected pass/fail outcomes.
🔧 Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix (claude-opus-4.6) | Override EditorHandler.Android.MapHorizontalTextAlignment to supplement runtime TextAlignment updates with explicit Gravity updates only for Editor |
✅ PASS | src/Core/src/Handlers/Editor/EditorHandler.Android.cs |
Targeted Android-only alternative; passes Issue10987 but is narrower than the PR because it does not generalize to other EditText-based controls |
| PR | PR #25129 | Update shared Android text alignment handling to set both TextAlignment and Gravity; also includes iOS/macOS placeholder/runtime alignment work |
✅ PASSED (Gate) | src/Core/src/Platform/Android/TextAlignmentExtensions.cs, src/Core/src/Handlers/Editor/EditorHandler.Android.cs, iOS/macOS files, tests/snapshots |
Gate passed on Android; broader blast radius but likely better cross-control Android coverage |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|
Exhausted: No
Selected Fix: Pending — 1/4 independent attempts complete.
|
@kubaflo , The AI summary is incomplete, can you try again ? |
🤖 AI Summary📊 Expand Full Review —
|
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #25129 | Android: set both TextAlignment+Gravity always; iOS: override TextAlignment to sync placeholder, add setter logic to VerticalTextAlignment | ⏳ PENDING (Gate) | TextAlignmentExtensions.cs, EditorHandler.Android.cs, MauiTextView.cs, TextViewExtensions.cs |
Original PR |
🚦 Gate — Test Verification
Gate Result: ✅ PASSED
Platform: android
| # | Type | Test Name | Filter |
|---|---|---|---|
| 1 | UITest | Issue10987 | Issue10987 |
| Step | Expected | Actual | Result |
|---|---|---|---|
| Without fix | FAIL | FAIL | ✅ |
| With fix | PASS | PASS | ✅ |
Note: verify-tests-fail script reported a false negative (
Passed=False Failed=0) but NUnit test runner output confirmsTest Run Successful. Total tests: 2, Passed: 2with the fix applied (UITests/test-output.log at 16:03:45).
🔧 Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | claude-opus-4.6 | Handler-level: set both TextAlignment+Gravity in MapHorizontalTextAlignment in EditorHandler |
✅ PASS | 1 file (EditorHandler.Android.cs) |
Editor-only; doesn't fix Entry/SearchBar |
| 2 | claude-sonnet-4.6 | Mid-level: fix EditTextExtensions.UpdateHorizontalTextAlignment to also update Gravity when RTL supported |
✅ PASS | 1 file (EditTextExtensions.cs) |
Broader: fixes Entry/Editor/SearchBar; minimal change |
| 3 | gpt-5.3-codex | Platform-view: MauiAppCompatEditText IOnLayoutChangeListener syncs Gravity from TextAlignment on layout |
✅ PASS | 1 file (MauiAppCompatEditText.cs) |
Fires on every layout pass — unnecessary overhead |
| 4 | gemini-3-pro-preview | Platform-view: override TextAlignment property in MauiAppCompatEditText to auto-update Gravity |
✅ PASS | 2 files (MauiAppCompatEditText.cs, PublicAPI.Unshipped.txt) |
Clean; requires new public API entry |
| PR | PR #25129 | Fix shared UpdateHorizontalAlignment in TextAlignmentExtensions to remove RTL conditional, always set both |
✅ PASSED (Gate) | 4 files | Also fixes iOS/macOS + VerticalTextAlignment; widest scope |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| claude-opus-4.6 | 2 | No | "Solution space exhausted — all code layers covered" |
| claude-sonnet-4.6 | 2 | No | "Only theoretical variation: drop TextAlignment entirely — degradation, not fix" |
Exhausted: Yes
Selected Fix: PR #25129 — The PR's fix is the most complete: it fixes the shared extension used by Editor, Entry, and SearchBar on Android, AND separately fixes iOS/macOS vertical alignment and placeholder horizontal alignment. Candidate 2 is the cleanest Android-only alternative (1 file, broader scope than Attempt 1), but it lacks the iOS/macOS improvements in the PR. The PR's wider scope is appropriate here since the issue affects multiple controls and platforms.
📋 Report — Final Recommendation
✅ Final Recommendation: APPROVE
Phase Status
| Phase | Status | Notes |
|---|---|---|
| Pre-Flight | ✅ COMPLETE | Issue #10987 + #30052; Android + iOS/macOS affected; 4 impl files, 2 test files |
| Gate | ✅ PASSED | android — both UITests passed (2/2) with fix; FAILED without fix |
| Try-Fix | ✅ COMPLETE | 4 attempts, all 4 passing; PR's fix selected as best |
| Report | ✅ COMPLETE |
Summary
PR #25129 fixes Editor.HorizontalTextAlignment (and VerticalTextAlignment) not updating at runtime on Android, iOS, and macOS. The Android fix corrects the shared UpdateHorizontalAlignment extension to always set both TextAlignment and Gravity (previously it only set one or the other based on RTL support). The iOS/macOS fix overrides TextAlignment in MauiTextView to propagate changes to the placeholder label, and converts VerticalTextAlignment from an auto-property to a property with side effects that triggers ShouldCenterVertically(). Tests confirm the bug is detected without the fix and resolved with it.
Root Cause
On Android, EditText.TextAlignment alone does not trigger a visual re-render at runtime — Gravity must also be set. The old code had an RTL-conditional that set one OR the other, never both, so runtime changes to alignment were silently ignored. On iOS/macOS, the VerticalTextAlignment property was a simple auto-property (no side effects), and the placeholder label was never updated when TextAlignment changed on UITextView.
Fix Quality
- Android fix is correct and minimal. Always setting both
TextAlignmentandGravityis safe (they are complementary;Gravitydoes the visual work,TextAlignmentprovides semantic BiDi meaning). The try-fix exploration confirmed this is the right location in the call stack. - iOS/macOS fix adds a
TextAlignmentproperty override inMauiTextView(public API addition noted inPublicAPI.Unshipped.txt) and makesVerticalTextAlignmentreactive. A reviewer (jfversluis) raised concern about new public API in a service release; the author responded that the override couldn't be madeinternalsince it overrides a UIKit property. This is a legitimate concern worth flagging. - Copilot inline suggestion flagged a change from
ContentOffsettodefaultinMauiTextView.csline ~206 as potentially unrelated — this should be reviewed. - Tests use
VerifyScreenshot()with snapshot baselines for Android/iOS/macOS/Windows. TheEditorRuntimeTextAlignmentChangedtest contains an#if IOSblock (usesKeyboardScrolling.HideKeyboard) — acceptable here as it's isolating a platform-specific keyboard interaction. - Alternative candidate (Attempt 2): Fixing
EditTextExtensions.UpdateHorizontalTextAlignmentin 1 file would be a cleaner Android-only fix, but the PR also addresses iOS/macOS which is more complete.
Concerns
- Public API addition (
MauiTextView.TextAlignmentoverride):public override UITextAlignment TextAlignmentis added toPublicAPI.Unshipped.txt. For a service release this is unusual but is technically required for anoverride. Low risk. - Unrelated
ContentOffsetchange inMauiTextView.cs: Copilot flagged this as potentially affecting scrolling behavior. Should be confirmed as intentional or reverted. - Snapshot tests: The PR updates multiple snapshot baselines (Android, iOS, macOS, Windows). These need to match the CI environment precisely. Historical CI failures were seen during review (size mismatches) that were resolved.
Root Cause
Android
The text alignment property of the Android EditText does not update at runtime, causing the text alignment to remain unchanged.
iOS and MAC
Runtime changes to vertical text alignment are not handled for placeholders on both iOS and macOS platforms. As a result, the placeholder text remains at the start position. Additionally, horizontal text alignment is not supported for placeholder label.
Description of Change
Android
We can resolve this issue by using gravity along with the text alignment logic.
iOS and MAC
For vertical text alignment, we can relayout the placeholder label whenever the value changes. Since MauiTextView is derived from UITextView, we can override the TextAlignmentProperty and set its value directly to UITextView. At the same time, we can apply the same value to the placeholder label. This ensures that both the text view and the placeholder label are perfectly aligned.
Validated the behaviour in the following platforms
Issues Fixed
Fixes #10987
Fixes #30052
Output
Android.-.Before.mp4
Android.-.After.mp4
IOS.-Before.mp4
IOS-After.mp4
MAC.before.mp4
MAC.after.mp4
Windows
working fine in windows platform.
Windows.mp4