[Windows/Mac] Fix RTL FlowDirection causes overlap with native window control buttons in TitleBar #30400
Conversation
This reverts commit 138797f.
|
Hey there @@devanathan-vaithiyanathan! 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
bot
added
the
partner/syncfusion
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for right-to-left (RTL) layouts in the TitleBar on Windows and MacCatalyst by applying platform-specific margin adjustments via visual states, and includes a new UI test and host app page to verify the fix.
- Introduces two new visual states (
TitleBarLeftToRight/TitleBarRightToLeft) and applies them in theTitleBartemplate based onFlowDirection. - Updates
TitleBarto update its visual state whenFlowDirectionchanges and on template application. - Adds a host-page and shared UI test to toggle and verify RTL behavior.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| src/Controls/src/Core/TitleBar/TitleBar.cs | Added LTR/RTL visual states, removed hardcoded margins, and hooked up FlowDirection changes to update the template. |
| src/Controls/tests/TestCases.HostApp/Issues/Issue30399.cs | New ContentPage to host a toggle button for switching FlowDirection and test TitleBar behavior. |
| src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue30399.cs | New UI test that taps the toggle button twice and captures a screenshot to verify correct RTL layout. |
Comments suppressed due to low confidence (2)
src/Controls/tests/TestCases.HostApp/Issues/Issue30399.cs:1
- Per UI testing guidelines, host app tests require a corresponding XAML page under TestCases.HostApp/Issues (Issue30399.xaml) with
AutomationIdattributes. A pure C# ContentPage may not be discovered properly by the test harness.
namespace Maui.Controls.Sample.Issues;
src/Controls/src/Core/TitleBar/TitleBar.cs:46
- The new FlowDirection visual states and their margin behaviors should be reflected in the public XML documentation under
/docs/so consumers know about the LTR/RTL support.
internal const string TitleBarLTRState = "TitleBarLeftToRight";
| @@ -0,0 +1,26 @@ | |||
| # if TEST_FAILS_ON_ANDROID && TEST_FAILS_ON_IOS // Titlebar applicable only on Windows and MacCatalyst | |||
There was a problem hiding this comment.
The preprocessor conditional only compiles the test when both TEST_FAILS_ON_ANDROID and TEST_FAILS_ON_IOS are true, which prevents the test from running on Windows/MacCatalyst. You likely want to exclude Android OR iOS (e.g. #if !ANDROID && !IOS) so it runs where TitleBar is supported.
| # if TEST_FAILS_ON_ANDROID && TEST_FAILS_ON_IOS // Titlebar applicable only on Windows and MacCatalyst | |
| #if !TEST_FAILS_ON_ANDROID && !TEST_FAILS_ON_IOS // Titlebar applicable only on Windows and MacCatalyst |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Are not failing on that platforms, I would use something like:
#if MACCATALYST && WINDOWS
There was a problem hiding this comment.
@jsuarezruiz , Based on your suggestion, I have modified the changes. Let me know if any changes needed
| Property = MarginProperty, | ||
| TargetName = TemplateRootName, | ||
| #if MACCATALYST | ||
| Value = new Thickness(80, 0, 0, 0) // System buttons on left in macOS |
There was a problem hiding this comment.
Could extract magic numbers to Constants? Also, include a detailed comment explaining the values.
There was a problem hiding this comment.
@jsuarezruiz, I’ve extracted the numeric values into const variables and added appropriate comments.
| @@ -0,0 +1,26 @@ | |||
| # if TEST_FAILS_ON_ANDROID && TEST_FAILS_ON_IOS // Titlebar applicable only on Windows and MacCatalyst | |||
There was a problem hiding this comment.
Are not failing on that platforms, I would use something like:
#if MACCATALYST && WINDOWS
| App.WaitForElement("ToggleFlowDirectionButton"); | ||
| App.Tap("ToggleFlowDirectionButton"); | ||
| App.Tap("ToggleFlowDirectionButton"); | ||
| VerifyScreenshot(includeTitleBar: true); |
|
/azp run MAUI-UITests-public |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
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
TitleBar doesn't properly handle right-to-left (RTL) layout direction, causing incorrect content alignment, overlapped with system buttons
Description of Change
Based on the FlowDirection, updated the TitleBar to apply appropriate Margin values to the content grid for both Windows and Mac platforms using visual states. This ensures correct alignment in both LTR and RTL layouts,
Issues Fixed
Fixes #30399
Tested the behavior in the following platforms.
Before.mov
After.mov