[iOS] Fix Button Content Overlap in RTL Layout - #38339
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 38339Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 38339" |
|
Azure Pipelines: Successfully started running 1 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
|
/azp run maui-pr-uitests , maui-pr-devicetests |
|
Azure Pipelines: Successfully started running 1 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: Successfully started running 2 pipeline(s). |
There was a problem hiding this comment.
🟡 Changes recommended
The newly added visual test lacks committed baseline snapshots (and should be scoped appropriately), which will cause CI failures until addressed.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Fixes iOS (and MacCatalyst) Button image/title inset calculations so ContentLayout spacing and Left/Right positioning behave correctly under RTL flow direction, addressing #38251.
Changes:
- Adjusted
UIButtonImageEdgeInsets/TitleEdgeInsetsmath to account for effective RTL direction. - Added a HostApp repro page for Issue 38251 with LTR/RTL and Left/Right image-position variants.
- Added a new visual UITest which verifies the rendered layout via screenshot comparison.
File summaries
| File | Description |
|---|---|
| src/Controls/src/Core/Button/Button.iOS.cs | Applies RTL-aware inset direction when computing image/title edge insets for iOS/MacCatalyst buttons. |
| src/Controls/tests/TestCases.HostApp/Issues/Issue38251.cs | Adds an Issue page with LTR/RTL reference buttons and image-position variants for visual validation. |
| src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue38251.cs | Adds a screenshot-based UI regression test for the RTL Button.ContentLayout scenario. |
Review details
Suppressed comments (2)
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue38251.cs:20
- The screenshot includes all four buttons from the host page, but the test only waits for two of them. This can make the visual snapshot flaky if the other two buttons (or their images) haven’t finished rendering yet.
_ = App.WaitForElement("LtrReferenceButton");
_ = App.WaitForElement("RtlReferenceButton");
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue38251.cs:22
- This new visual test will fail until baseline snapshots are added. Please commit the expected images under the platform snapshot folders (e.g., TestCases.iOS.Tests/snapshots//ButtonContentLayoutShouldRespectRightToLeftFlowDirection.png and TestCases.Mac.Tests/snapshots/mac/ButtonContentLayoutShouldRespectRightToLeftFlowDirection.png).
VerifyScreenshot(retryTimeout: TimeSpan.FromSeconds(2));
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
| using NUnit.Framework; | ||
| using UITest.Appium; | ||
| using UITest.Core; | ||
|
|
||
| namespace Microsoft.Maui.TestCases.Tests.Issues; | ||
|
|
||
| public class Issue38251 : _IssuesUITest | ||
| { | ||
| public Issue38251(TestDevice device) : base(device) | ||
| { | ||
| } | ||
|
|
||
| public override string Issue => "Button RTL image and text overlap on iOS"; | ||
|
|
||
| [Test] | ||
| [Category(UITestCategories.Button)] | ||
| public void ButtonContentLayoutShouldRespectRightToLeftFlowDirection() | ||
| { | ||
| _ = App.WaitForElement("LtrReferenceButton"); | ||
| _ = App.WaitForElement("RtlReferenceButton"); | ||
|
|
||
| VerifyScreenshot(retryTimeout: TimeSpan.FromSeconds(2)); | ||
| } | ||
| } No newline at end of file |
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
On iOS, RTL buttons using ContentLayout="Left, 10" displayed overlapping or widely separated image and text.
Description of Change
Updated the iOS button inset calculations to respect RTL direction while preserving the configured spacing and Left/Right positioning.
Issues Fixed
Fixes #38251
Tested the behavior in the following platforms.
Before.mov
After.mov