-
Notifications
You must be signed in to change notification settings - Fork 1.9k
[iOS] Fix for DatePicker FlowDirection Not Working on iOS #30193
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Hey there @@SyedAbdulAzeemSF4852! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes an issue where the DatePicker control's FlowDirection was not being applied properly on iOS by correcting the API signature and adding logic for text alignment adjustments.
- Updated the MapFlowDirection method signature to use IDatePickerHandler instead of DatePickerHandler.
- Added text alignment logic in DatePickerExtensions.cs based on the effective flow direction.
- Included new test cases in TestCases.HostApp and TestCases.Shared.Tests to validate the fix.
Reviewed Changes
Copilot reviewed 10 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
File | Description |
---|---|
src/Core/src/PublicAPI/net-maccatalyst/PublicAPI.Unshipped.txt | Updated the API signature to match the expected interface type for MapFlowDirection. |
src/Core/src/PublicAPI/net-maccatalyst/PublicAPI.Shipped.txt | Removed the old signature to reflect the breaking change. |
src/Core/src/PublicAPI/net-ios/PublicAPI.Unshipped.txt | Updated the API signature to match the expected interface type for MapFlowDirection. |
src/Core/src/PublicAPI/net-ios/PublicAPI.Shipped.txt | Removed the old signature to reflect the breaking change. |
src/Core/src/Platform/iOS/DatePickerExtensions.cs | Added logic to adjust text alignment based on EffectiveFlowDirection. |
src/Core/src/Handlers/DatePicker/DatePickerHandler.iOS.cs | Updated the MapFlowDirection method signature to use IDatePickerHandler. |
src/Core/src/Handlers/DatePicker/DatePickerHandler.cs | Updated the MapFlowDirection method signature in the handler class. |
src/Core/src/Handlers/DatePicker/DatePickerHandler.MacCatalyst.cs | Updated the MapFlowDirection method signature to use IDatePickerHandler. |
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue30065.cs | Added a test case to validate DatePicker FlowDirection behavior. |
src/Controls/tests/TestCases.HostApp/Issues/Issue30065.cs | Added a test page to validate switching FlowDirection on DatePicker. |
Comments suppressed due to low confidence (1)
@@ -1 +1,3 @@ | |||
#nullable enable | |||
*REMOVED*static Microsoft.Maui.Handlers.DatePickerHandler.MapFlowDirection(Microsoft.Maui.Handlers.DatePickerHandler! handler, Microsoft.Maui.IDatePicker! datePicker) -> void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The removal and update of the MapFlowDirection signature here introduces a breaking change in the public API. Please ensure that all consumers are aware of this change and update their implementations accordingly.
Copilot uses AI. Check for mistakes.
@@ -80,7 +80,7 @@ public static partial void MapTextColor(IDatePickerHandler handler, IDatePicker | |||
handler.PlatformView?.UpdateTextColor(datePicker); | |||
} | |||
|
|||
public static partial void MapFlowDirection(DatePickerHandler handler, IDatePicker datePicker) | |||
public static partial void MapFlowDirection(IDatePickerHandler handler, IDatePicker datePicker) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The method signature change for MapFlowDirection in the iOS handler is a breaking API change. Ensure that all related documentation and client codes reflect the new interface usage.
Copilot uses AI. Check for mistakes.
@@ -73,7 +73,7 @@ public DatePickerHandler(IPropertyMapper? mapper, CommandMapper? commandMapper) | |||
/// </summary> | |||
/// <param name="handler">The associated handler.</param> | |||
/// <param name="datePicker">The associated <see cref="IDatePicker"/> instance.</param> | |||
public static partial void MapFlowDirection(DatePickerHandler handler, IDatePicker datePicker); | |||
public static partial void MapFlowDirection(IDatePickerHandler handler, IDatePicker datePicker); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The updated MapFlowDirection signature in the handler class changes the public API contract. This is a breaking change; please ensure API consumers are notified.
Copilot uses AI. Check for mistakes.
@@ -70,7 +70,7 @@ public static partial void MapTextColor(IDatePickerHandler handler, IDatePicker | |||
|
|||
} | |||
|
|||
public static partial void MapFlowDirection(DatePickerHandler handler, IDatePicker datePicker) | |||
public static partial void MapFlowDirection(IDatePickerHandler handler, IDatePicker datePicker) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The MapFlowDirection signature update on MacCatalyst is a breaking API change. Ensure that documentation and clients targeting MacCatalyst are updated accordingly.
Copilot uses AI. Check for mistakes.
{ | ||
App.WaitForElement("ToggleFlowDirectionBtn"); | ||
App.Tap("ToggleFlowDirectionBtn"); | ||
VerifyScreenshot(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pending snapshot on Windows.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Running a build.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jsuarezruiz , I've updated the test case and added snapshots for iOS and Android. I will add the missing snap in the next CI run.
/azp run MAUI-UITests-public |
Azure Pipelines successfully started running 1 pipeline(s). |
@SyedAbdulAzeemSF4852 Could you rebase and fix the conflicts? Thanks in advance. |
1b8264f
to
20a54a3
Compare
@jsuarezruiz , Rebased the branch as suggested. |
/azp run MAUI-UITests-public |
Azure Pipelines successfully started running 1 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
Root Cause
Description of Change
Issues Fixed
Fixes #30065
Validated the behaviour in the following platforms
Output
Before.mov
After.mov