-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Fix DatePicker CharacterSpacing property not working on Windows platform #30139
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
Co-authored-by: mattleibow <[email protected]>
Co-authored-by: mattleibow <[email protected]>
static void ApplyCharacterSpacingToTextBlocks(CalendarDatePicker platformDatePicker, double characterSpacing) | ||
{ | ||
// Find all TextBlock elements within the CalendarDatePicker and apply character spacing | ||
var textBlocks = platformDatePicker.GetChildren<Microsoft.UI.Xaml.Controls.TextBlock>(); |
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.
@copilot Can you directly access the placeholder TextBlock element in the CalendarDatePicker like below?
platformDatePicker.GetDescendantByName("DateText");
static void ApplyCharacterSpacingToTextBlocks(CalendarDatePicker platformDatePicker, double characterSpacing) | ||
{ | ||
// Find all TextBlock elements within the CalendarDatePicker and apply character spacing | ||
var textBlocks = platformDatePicker.GetChildren<Microsoft.UI.Xaml.Controls.TextBlock>(); |
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.
Like in other parts of the framework, can use the extension method
internal static T? GetDescendantByName<T>(this DependencyObject parent, string elementName) where T : DependencyObject |
|
||
foreach (var textBlock in textBlocks) | ||
{ | ||
if (textBlock != null) |
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.
if (textBlock is not null)
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.
Updated to use if (textBlock is not null)
pattern matching in commit ba3447c.
@@ -89,5 +90,36 @@ Color GetNativeTextColor(DatePickerHandler datePickerHandler) | |||
|
|||
return null; | |||
} | |||
|
|||
[Fact] | |||
public async Task CharacterSpacingInitializesCorrectly() |
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.
Could you create another test to validate updates? CharacterSpacingUpdatesCorrectly
passing different CharacterSpacing values.
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.
Added CharacterSpacingUpdatesCorrectly test that validates character spacing updates with different values (5.0 to 15.0) in commit ba3447c.
if (textBlock != null && textBlock.CharacterSpacing > 0) | ||
{ | ||
// Convert back from Em to original value (reverse of ToEm() calculation) | ||
return textBlock.CharacterSpacing / (0.0624 * 1000); |
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.
Could create an extension method to convert back from Em. In the extension method, include comments explaining the details.
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.
Created FromEm extension method in CharacterSpacingExtensions.cs with comprehensive XML documentation explaining the conversion details. Updated the test to use this method instead of manual calculation in commit ba3447c.
…method, and include CharacterSpacingUpdatesCorrectly test Co-authored-by: jsuarezruiz <[email protected]>
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
The added FromEm method is not part of the publicAPIs, could you add it?
|
Co-authored-by: jsuarezruiz <[email protected]>
Added FromEm method to Windows PublicAPI unshipped declarations in commit a921d93. |
Description
The
CharacterSpacing
property onDatePicker
was not working on the Windows platform. While this property worked correctly on other platforms like Android and iOS, it had no visible effect on Windows.Root Cause
The issue occurred because:
On Android:
MauiDatePicker
inherits fromAppCompatEditText
(which extendsTextView
), so it automatically gets theCharacterSpacing
property support through existingTextViewExtensions.UpdateCharacterSpacing()
.On Windows:
CalendarDatePicker
is a composite control that doesn't have a directCharacterSpacing
property like text controls do. The existing implementation tried to setplatformDatePicker.CharacterSpacing = datePicker.CharacterSpacing.ToEm()
butCalendarDatePicker
doesn't expose this property.Solution
Modified the Windows-specific
UpdateCharacterSpacing
method inDatePickerExtensions.cs
to:CalendarDatePicker
to find internalTextBlock
elementsTextBlock
using the existingCharacterSpacing.ToEm()
conversionOnLoaded
pattern to ensure the visual tree is availableTesting
DatePickerHandlerTests.Windows.cs
to verify character spacing is correctly appliedIssue30066
test case that reproduces the original problem and validates the fixBefore/After
Before: Setting
CharacterSpacing = 10
on a DatePicker had no visual effect on Windows.After: Character spacing is properly applied to the DatePicker text display on Windows, matching the behavior on other platforms.
Fixes #30066.
💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.