[iOS][Android] Label: Fix RTL padding not mirroring#32333
[iOS][Android] Label: Fix RTL padding not mirroring#32333kubaflo merged 1 commit intodotnet:inflight/currentfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request fixes an issue where label padding does not properly mirror in RTL (right-to-left) mode on iOS and Android platforms. The fix ensures that when a layout direction is set to RTL, the left and right padding values are swapped appropriately.
Key Changes:
- iOS: Modified
MauiLabel.DrawTextto swap left/right insets when in RTL layout direction - Android: Changed
SetPaddingtoSetPaddingRelativeto use Android's built-in RTL support - Added UI tests to validate RTL padding behavior
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Core/src/Platform/iOS/MauiLabel.cs | Adds RTL-aware inset flipping logic to swap left/right text insets when layout direction is RTL |
| src/Core/src/Platform/Android/TextViewExtensions.cs | Changes from absolute padding to relative padding method for automatic RTL support |
| src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue32316.cs | Adds NUnit test implementation to validate RTL padding behavior |
| src/Controls/tests/TestCases.HostApp/Issues/Issue32316.cs | Creates test UI page demonstrating label padding with RTL flow direction toggling |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
| { | ||
| App.WaitForElement("ToggleFlowDirectionButton"); | ||
| App.Tap("ToggleFlowDirectionButton"); | ||
| VerifyScreenshot(); |
There was a problem hiding this comment.
Pending snapshots, running a build.
There was a problem hiding this comment.
Snapshots are available in the latest build.
Could you commit the images?
|
/azp run MAUI-public |
|
Azure Pipelines successfully started running 1 pipeline(s). |
kubaflo
force-pushed
the
fix-Issue32316
branch
from
af8a268 to
43d86be
Compare
🤖 PR Agent Review📊 Expand Full ReviewStatus: IN PROGRESS
🔍 Phase 1: Pre-Flight — Context & Validation📝 Review Session — Post pr comment skill ·
|
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #32333 | iOS: Swap left/right insets in RTL mode in DrawText override. Android: Use SetPaddingRelative instead of SetPadding for automatic RTL support | ⏳ PENDING (Gate) | MauiLabel.cs, TextViewExtensions.cs |
Original PR - pending Gate validation |
Note: try-fix candidates (1, 2, 3...) will be added during Phase 4. PR's fix is reference only.
Exhausted: No
Selected Fix: ⏳ PENDING
📝 Review Session — Excluded android testing · 3bcbdd6
Status: ⏳ PENDING
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #32333 | iOS: Swap left/right insets in RTL mode in DrawText override. Android: Use SetPaddingRelative instead of SetPadding for automatic RTL support | ⏳ PENDING (Gate) | MauiLabel.cs (+15), TextViewExtensions.cs (+1/-1) |
Original PR - validated by Gate |
Note: try-fix candidates (1, 2, 3...) are added during Phase 4. PR's fix is reference only.
Exhausted: No
Selected Fix: ⏳ PENDING
📋 Phase 5: Report — Final Recommendation
No review sessions yet
Review Complete — All phases passed. PR is ready for merge pending CI validation.
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 32333Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 32333" |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
kubaflo
added
s/agent-changes-requested
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/azp run maui-pr-uitests |
|
Command 'maui-pr-uitests' is not supported by Azure Pipelines. Supported commands
See additional documentation. |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Fix Co-Authored-By: Copilot <223556219+Copilot@users.noreply.github.com>
kubaflo
added
s/agent-approved
🤖 AI Summary📊 Expand Full Review —
|
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #32333 | Android: SetPaddingRelative; iOS: Flip insets in DrawText | ⏳ PENDING (Gate) | TextViewExtensions.cs, MauiLabel.cs |
Original PR |
Test Files
- HostApp:
src/Controls/tests/TestCases.HostApp/Issues/Issue32316.cs - NUnit:
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue32316.cs - Test method:
RTLModePaddingShouldWork(screenshot comparison) - Category:
UITestCategories.Label
🚦 Gate — Test Verification
Gate Result: ✅ PASSED
Platform: Android
Mode: Full Verification
- Tests FAIL without fix: ✅
- Tests PASS with fix: ✅
Details: Test RTLModePaddingShouldWork correctly catches the RTL padding bug. Without fix (SetPadding), test fails with snapshot mismatch. With fix (SetPaddingRelative), test passes confirming padding mirrors correctly in RTL mode.
🔧 Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix (claude-sonnet-4.6) | Walk MAUI parent hierarchy to resolve effective FlowDirection, swap padding in SetPadding + MapFlowDirection override | ✅ PASS | TextViewExtensions.cs, LabelHandler.Android.cs, LabelHandler.cs |
More verbose; handles MatchParent via parent walk |
| 2 | try-fix (claude-opus-4.6) | Check textView.LayoutDirection at handler/mapper level in MapPadding override, swap left/right; MapFlowDirection re-triggers it |
✅ PASS | LabelHandler.Android.cs, LabelHandler.cs |
Clean handler-level fix; uses Android platform LayoutDirection |
| 3 | try-fix (claude-sonnet-4.6) | Override OnLayout in MauiTextView subclass to call SetPaddingRelative after layout direction is resolved natively |
✅ PASS | MauiTextView.cs, TextViewExtensions.cs, PublicAPI.Unshipped.txt |
Most complex; native-level hook |
| PR | PR #32333 | Android: SetPaddingRelative (1-line change); iOS: Flip left/right insets in DrawText when RTL |
✅ PASSED (Gate) | TextViewExtensions.cs, MauiLabel.cs |
Simplest — 1-line for Android |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| claude-sonnet-4.6 | 2 | Yes | Suggested OnRtlPropertiesChanged override |
| claude-opus-4.6 | 2 | No | NO NEW IDEAS |
| claude-sonnet-4.6 | 3 | Yes (not viable) | OnRtlPropertiesChanged — already proven unbound in C# (CS0115, confirmed in Attempt 3) |
| claude-opus-4.6 | 3 | No | NO NEW IDEAS |
Exhausted: Yes — all new ideas are variants of covered approaches or proven unviable
Selected Fix: PR #32333 — The PR's SetPaddingRelative (1-line Android change) is the simplest, most idiomatic solution. It relies on Android's built-in RTL layout system rather than manual direction detection. All 3 alternative approaches passed but are more complex. PR's fix is optimal.
📋 Report — Final Recommendation
✅ Final Recommendation: APPROVE
Phase Status
| Phase | Status | Notes |
|---|---|---|
| Pre-Flight | ✅ COMPLETE | Issue #32316 context gathered; prior agent review imported |
| Gate | ✅ PASSED | Android — tests FAIL without fix, PASS with fix |
| Try-Fix | ✅ COMPLETE | 3 attempts, all 3 passing; PR fix selected as best |
| Report | ✅ COMPLETE |
Summary
PR #32333 correctly fixes RTL label padding mirroring on Android and iOS. The Android fix is a clean 1-line change (SetPadding → SetPaddingRelative), and the iOS fix properly flips left/right insets when the effective layout direction is RTL. Three independent alternative approaches were explored — all passed — but the PR's fix is the simplest and most idiomatic.
Root Cause
Android: TextViewExtensions.UpdatePadding used SetPadding(left, top, right, bottom) which always applies padding in absolute left/right coordinates regardless of layout direction. Android's SetPaddingRelative uses start/end semantics and automatically maps to the correct physical side based on the view's resolved layout direction.
iOS: MauiLabel.DrawText applied TextInsets directly without checking the effective UI layout direction. When RTL is active, the left inset should become the right inset and vice versa — the PR adds this check using EffectiveUserInterfaceLayoutDirection.
Fix Quality
Android fix (SetPadding → SetPaddingRelative):
- ✅ Minimal change — 1 line, no added complexity
- ✅ Idiomatic — relies on Android's own RTL layout system instead of manual direction detection
- ✅ Robust — works automatically even when FlowDirection is inherited (MatchParent) from ancestors
- ✅ No edge cases — Android handles all RTL remapping natively at draw time
iOS fix (flip insets in DrawText):
- ✅ Correct use of
EffectiveUserInterfaceLayoutDirection(resolves inherited direction, unlikeUserInterfaceLayoutDirection) - ✅ Minimal scope — only affects drawing, no layout side effects
⚠️ Minor style nit: extra blank line afterrect = insets.InsetRect(rect);(cosmetic only)
Alternative approaches explored (all ✅ PASS):
| Attempt | Approach | Verdict vs PR |
|---|---|---|
| 1 | Walk MAUI parent hierarchy + MapFlowDirection override | More complex, 3 files, good but verbose |
| 2 | Check platform LayoutDirection in handler MapPadding |
Clean but adds handler complexity unnecessarily |
| 3 | Override OnLayout in MauiTextView |
Most complex, native-level hook, overkill |
Selected Fix: PR #32333 — The PR's approach is the optimal solution. Simpler, fewer files, relies on platform-native RTL mechanisms.
Code Review Notes
- Minor: Extra blank line in
MauiLabel.csafterrect = insets.InsetRect(rect);(line 50-51) — cosmetic only, no functional impact - The
PlatformAffected.iOS | PlatformAffected.Androidattribute in the HostApp test is correct - Snapshot images have been updated (confirmed by reviewer
jsuarezruiz)
Result: ✅ APPROVE — Fix is correct, minimal, and optimal. Tests verified on Android.
📋 Expand PR Finalization Review
PR #32333 Finalization Review
PR: Fix RTL label padding on iOS and Android
Branch: fix-Issue32316 → main
Fixes: #32316
Phase 1: Title & Description Review
🟡 Title: Needs Minor Update
Current: Fix RTL label padding on iOS and Android
Recommended: [iOS][Android] Label: Fix RTL padding not mirroring
The current title is understandable but doesn't follow the repo's standard [Platform] Component: What changed formula. The recommended version is more searchable in git history.
🟡 Description: Accurate but Minimal — Enhancement Recommended
Quality assessment:
| Indicator | Status | Notes |
|---|---|---|
| NOTE block | ✅ | Present at top |
| Description of Change | ✅ | Present, accurate |
| Issues Fixed | ✅ | Links to #32316 |
| Root cause | ❌ | Missing |
| Technical depth | ❌ | No per-file breakdown |
| Platform coverage | Says iOS+Android but tests run on all platforms |
The description is accurate but thin. It describes the "what" but not the "why" (root cause) or the technical approach per platform. Recommended enhancement:
<!-- 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!
### Root Cause
On Android, `TextView.SetPadding()` treats its parameters as absolute left/right, ignoring layout direction. In RTL mode this causes the padding to appear on the wrong side.
On iOS, `MauiLabel.DrawText()` applied `TextInsets` directly without accounting for RTL layout direction, causing insets to be mirrored incorrectly.
### Description of Change
**Android (`TextViewExtensions.cs`):**
Replaced `SetPadding` with `SetPaddingRelative`. The relative variant maps `start`/`end` parameters to the correct visual sides depending on the view's layout direction, so RTL labels receive padding on the correct side.
**iOS (`MauiLabel.cs`):**
In `DrawText`, after reading `TextInsets`, detect `EffectiveUserInterfaceLayoutDirection == RightToLeft` and swap the `Left`/`Right` inset values before applying to `rect`. This mirrors the padding to the correct visual side in RTL layouts.
**Tests:**
Added UI test page (`Issue32316.cs`) with a toggle button to switch `FlowDirection` to `RightToLeft` and a screenshot test (`RTLModePaddingShouldWork`) to verify correct padding mirroring. Snapshots added for Android, iOS, Mac, and Windows.
### Issues Fixed
Fixes https://github.com/dotnet/maui/issues/32316Phase 2: Code Review
🔴 Critical Issues
None.
🟡 Suggestions
1. Extra blank line in MauiLabel.cs (line 51-52)
- File:
src/Core/src/Platform/iOS/MauiLabel.cs - Problem: There are two consecutive blank lines between
rect = insets.InsetRect(rect);and theif (_verticalAlignment ...)check. The diff introduced an unintended extra blank line. - Recommendation: Remove one blank line for consistent style.
// Before (two blank lines):
rect = insets.InsetRect(rect);
if (_verticalAlignment != ...)
// After (one blank line):
rect = insets.InsetRect(rect);
if (_verticalAlignment != ...)2. Missing newline at end of new test files
- Files:
src/Controls/tests/TestCases.HostApp/Issues/Issue32316.cs,src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue32316.cs - Problem: Both files are missing a trailing newline (
\ No newline at end of filein the diff). - Recommendation: Add a newline at end of each file for POSIX compliance and consistent git diffs.
3. [Issue] attribute PlatformAffected inconsistent with test scope
- File:
src/Controls/tests/TestCases.HostApp/Issues/Issue32316.cs - Problem: The attribute declares
PlatformAffected.iOS | PlatformAffected.Android, but snapshot files were generated for Windows and Mac as well, indicating the test actually runs cross-platform. The attribute should reflect the actual test scope. - Recommendation: Update to
PlatformAffected.All(or at minimum add Windows/Mac to the flags) if the test is expected to pass on all platforms, or ensure that the existingRightToLeftFlowDirectionShouldWorksnapshot update on Android/iOS is intentional and not a regression.
4. Updated existing snapshot RightToLeftFlowDirectionShouldWork.png on Android and iOS
- Files:
src/Controls/tests/TestCases.Android.Tests/snapshots/android/RightToLeftFlowDirectionShouldWork.pngsrc/Controls/tests/TestCases.iOS.Tests/snapshots/ios/RightToLeftFlowDirectionShouldWork.png
- Problem: An existing test's baseline snapshot was modified. This could indicate the fix changes visual output for an existing RTL test — which may be intentional (the old snapshot was wrong) or could be a regression.
- Recommendation: Confirm in the PR/issue that the updated snapshot reflects the correct, intended RTL behavior and is not a side-effect regression.
✅ Looks Good
- Android fix is correct:
SetPaddingRelative(start, top, end, bottom)correctly mapslabel.Padding.Left→ logicalstartandlabel.Padding.Right→ logicalend, which Android resolves to the proper visual sides in RTL. This is the idiomatic Android approach for RTL-aware padding. - iOS fix is correct: Flipping
Left/RightinDrawTextwhenEffectiveUserInterfaceLayoutDirection == RightToLeftis the right approach sinceTextInsetsare set in LTR terms from MAUI'sPadding. - Fix is minimal and focused: Two surgical one-line and one-block changes. Low risk of regressions beyond the known snapshot update.
- Tests added: Both the HostApp page and the NUnit test are present and use proper patterns (AutomationId,
VerifyScreenshot). - NOTE block present in description.
- Issue correctly linked via
Fixes #32316.
Summary
| Area | Status |
|---|---|
| Title | 🟡 Minor improvement recommended |
| Description | 🟡 Accurate but thin — root cause + per-file breakdown recommended |
| Core fix correctness | ✅ Both Android and iOS fixes are correct |
| Tests | ✅ Present and well-structured |
| Code quality | 🟡 Extra blank line, missing EOF newlines, PlatformAffected mismatch |
| Snapshot regression risk |
Overall verdict: PR is ready to merge with minor cleanup (extra blank line, EOF newlines). The core fix is correct. Recommend confirming the updated RightToLeftFlowDirectionShouldWork snapshots reflect correct behavior before merging.
kubaflo
added
the
s/agent-fix-pr-picked
kubaflo
changed the title
<!-- 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. !!!!!!! --> ### Description of Change Corrects label padding handling in RTL mode by using SetPaddingRelative on Android and flipping left/right insets on iOS. Adds test cases and UI tests to verify correct padding mirroring for labels in RTL layouts. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #32316 <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. --> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
<!-- 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. !!!!!!! --> ### Description of Change Corrects label padding handling in RTL mode by using SetPaddingRelative on Android and flipping left/right insets on iOS. Adds test cases and UI tests to verify correct padding mirroring for labels in RTL layouts. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #32316 <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. --> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
<!-- 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. !!!!!!! --> ### Description of Change Corrects label padding handling in RTL mode by using SetPaddingRelative on Android and flipping left/right insets on iOS. Adds test cases and UI tests to verify correct padding mirroring for labels in RTL layouts. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes dotnet#32316 <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. --> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
<!-- 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. !!!!!!! --> ### Description of Change Corrects label padding handling in RTL mode by using SetPaddingRelative on Android and flipping left/right insets on iOS. Adds test cases and UI tests to verify correct padding mirroring for labels in RTL layouts. ### Issues Fixed <!-- Please make sure that there is a bug logged for the issue being fixed. The bug should describe the problem and how to reproduce it. --> Fixes #32316 <!-- Are you targeting main? All PRs should target the main branch unless otherwise noted. --> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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!
Description of Change
Corrects label padding handling in RTL mode by using SetPaddingRelative on Android and flipping left/right insets on iOS. Adds test cases and UI tests to verify correct padding mirroring for labels in RTL layouts.
Issues Fixed
Fixes #32316