-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Add converters to DatePicker and TimePicker for DateOnly and TimeOnly #30790
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
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
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 adds TypeConverter support for DateOnly and TimeOnly types to DatePicker and TimePicker controls in .NET MAUI. This enhancement allows these .NET 6+ date/time types to be bound directly to the controls without manual conversion.
- Implements DateTimeTypeConverter and TimeSpanTypeConverter classes to handle conversion between DateOnly/TimeOnly and DateTime/TimeSpan
- Adds TypeConverter attributes to DatePicker and TimePicker properties
- Extends type conversion extensions to support DateOnly and TimeOnly parsing
Reviewed Changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| DateTimeTypeConverter.cs | New converter for DateOnly/DateTime type conversions |
| TimeSpanTypeConverter.cs | New converter for TimeOnly/TimeSpan type conversions |
| TypeConversionExtensions.cs | Adds DateOnly/TimeOnly parsing support |
| DatePicker.cs | Adds TypeConverter attributes to Date, MinimumDate, MaximumDate properties |
| TimePicker.cs | Adds TypeConverter attribute to Time property |
| BindableProperty.cs | Registers new converters in type converter dictionary |
| Issue20438.cs | UI test implementation for the new converter functionality |
| Issue20438.cs (HostApp) | Host app test page demonstrating DateOnly/TimeOnly binding |
| DateTimeTypeConverterTests.xaml | XAML test page for converter validation |
| DateTimeTypeConverterTests.xaml.cs | Test code-behind for XAML converter tests |
| DateOnlyTypeConverterTests.cs | Unit tests for DateOnly converter functionality |
| TimeOnlyTypeConverterTests.cs | Unit tests for TimeOnly converter functionality |
| PublicAPI.Unshipped.txt | API surface additions for the new converter classes |
Comments suppressed due to low confidence (2)
src/Controls/src/Core/DateTimeTypeConverter.cs:30
- The variable name 'dateTimeOnly' is confusing since it represents a DateOnly value, not a DateTime. Consider renaming to 'parsedDateOnly' or similar.
if (DateOnly.TryParse(stringValue, culture, DateTimeStyles.None, out DateOnly dateTimeOnly))
src/Controls/src/Core/TimeSpanTypeConverter.cs:51
- The variable name 'timeSpan1' is unclear and uses a numeric suffix. Consider renaming to 'parsedTimeSpan' for better clarity.
else if (TimeSpan.TryParse(stringValue, culture, out var timeSpan1))
|
|
||
| var stringValue = "Not a TimeOnly string"; | ||
|
|
||
| await Assert.ThrowsAsync<NotSupportedException>(async () => converter.ConvertToInvariantString(stringValue)); |
Copilot
AI
Jul 25, 2025
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 test is calling ConvertToInvariantString on a TimeSpanTypeConverter, but this method is not implemented in the TimeSpanTypeConverter class. This will likely throw a NotImplementedException rather than NotSupportedException.
| await Assert.ThrowsAsync<NotSupportedException>(async () => converter.ConvertToInvariantString(stringValue)); | |
| await Assert.ThrowsAsync<NotImplementedException>(async () => converter.ConvertToInvariantString(stringValue)); |
Copilot uses AI. Check for mistakes.
|
|
||
| var stringValue = "Not a DateOnly string"; | ||
|
|
||
| await Assert.ThrowsAsync<NotSupportedException>(async () => converter.ConvertToInvariantString(stringValue)); |
Copilot
AI
Jul 25, 2025
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 test is calling ConvertToInvariantString on a DateTimeTypeConverter, but this method is not implemented in the DateTimeTypeConverter class. This will likely throw a NotImplementedException rather than NotSupportedException.
Copilot uses AI. Check for mistakes.
|
/rebase |
1a71772 to
5f64b89
Compare
|
/azo run |
|
I need to investigate what works now, Binding, Xaml, without DateOnly, and what impact it has. It makes sense to parse invariant strings to DateTime and TimeSpan in xaml, it doesn't make a lot of sense to support binding to a string, this is why I think we do not support it right now. this is all supposition, I've yet to do some investigations on this |
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.
I would like a slimmer version of this PR that supports only what we want: Binding from a DateOnly view model to a DateTime property (and TimeSpan)
everything else is probably already supported, or not supported on purpose (convert to/from string on a binding)
| { typeof(ImageSource), new ImageSourceConverter() } | ||
| { typeof(ImageSource), new ImageSourceConverter() }, | ||
| #if NET6_0_OR_GREATER | ||
| { typeof(DateTime?), new DateTimeTypeConverter() }, |
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.
this is where we declare some built-in conversions for bindings. automatically convert to from DateTime-DateOnly make sense.
is the nullability marker required ?
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.
Yes, the nullability marker is required because the TypeConverter registration must exactly match the property types. Since DatePicker.Date is DateTime? and TimePicker.Time is TimeSpan?, the converters must be registered using the nullable types.
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.
so there's no Binding conversion for non-nullable date time ?
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.
so it's the case for other types as well, I should look at that
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 current implementation supports both nullable and non-nullable values, so no separate binding conversion is needed.
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 you bind to a DateTime (non-nullable) property, no conversion happens
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.
I've added the conversion for non-nullable types as well.
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.
I would have preferred to avoid the duplication...
| } | ||
|
|
||
| /// <include file="../../docs/Microsoft.Maui.Controls/DatePicker.xml" path="//Member[@MemberName='Date']/Docs/*" /> | ||
| #if NET6_0_OR_GREATER |
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.
this is used by Xaml, and Xaml has a built-in conversion from string to DateTime, so this isn't required. please remove
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.
same for the other ones
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.
Removed the unnecessary changes, as suggested.
|
|
||
| namespace Microsoft.Maui.Controls; | ||
|
|
||
| [ProvideCompiled("Microsoft.Maui.Controls.XamlC.DateTimeTypeConverter")] |
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.
except there's no compiled converter provided. remove the attribute
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.
Attribute has been removed.
| namespace Microsoft.Maui.Controls; | ||
|
|
||
| [ProvideCompiled("Microsoft.Maui.Controls.XamlC.DateTimeTypeConverter")] | ||
| public class DateTimeTypeConverter : TypeConverter |
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.
make this internal so we do not change the public api
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.
Changed the converter class to internal and removed the related public API changes.
| public class DateTimeTypeConverter : TypeConverter | ||
| { | ||
| public override bool CanConvertFrom(ITypeDescriptorContext? context, Type? sourceType) | ||
| => sourceType == typeof(string) || sourceType == typeof(DateTime) || sourceType == typeof(DateOnly); |
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 converter should not do string conversion on Bindings
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.
Removed the string-based conversion logic from the converter.
| public TimeOnly TimeWithTimeOnly { get; set; } = new(10, 30, 0); | ||
| public TimeSpan TimeWithTimeSpan { get; set; } = new(10, 30, 0); | ||
| public DateTime DateWithDateTime { get; set; } = new(2025, 7, 28, 10, 30, 0); | ||
| public string DateWithString { get; set; } = "2025-07-28"; |
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.
no need for string conversion
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.
Removed the test as suggested.
| [Category(UITestCategories.DatePicker)] | ||
| public void DatePickerConverters() | ||
| { | ||
| Assert.That(App.WaitForElement("LabelDateOnly").GetText(), Is.EqualTo("DateOnly Value: 2025-07-28")); |
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.
not sure this test is resistant to localisation changes, and probably won't pass on all machines
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.
Replaced with a simpler unit test in Core.UnitTests
| x:Class="Microsoft.Maui.Controls.Xaml.UnitTests.DateTimeTypeConverterTests"> | ||
| <StackLayout> | ||
| <DatePicker x:Name="datePicker" | ||
| Date="2025-07-23"/> |
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.
this was already supported, and isn't testing anything new
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.
you can drop the test
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.
Removed the test as suggested.
| [Issue(IssueTracker.Github, 20438, "Add DateOnly and TimeOnly converters to DatePicker and TimePicker", PlatformAffected.All)] | ||
| public class Issue20438 : ContentPage | ||
| { | ||
| public Issue20438() |
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.
we're not testing anything visual. a simpler unit test in Controls/test/Core.UnitTests would execute faster, and for a fraction of the cost
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.
Replaced with a simpler unit test in Core.UnitTests
|
/azp run MAUI-UITests-public |
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.
your tests for binding does not work. Here's a fix
diff --git a/src/Controls/src/Core/BindableProperty.cs b/src/Controls/src/Core/BindableProperty.cs
index fd30224e13..3d0d010725 100644
--- a/src/Controls/src/Core/BindableProperty.cs
+++ b/src/Controls/src/Core/BindableProperty.cs
@@ -229,6 +229,11 @@ namespace Microsoft.Maui.Controls
return true;
}
if (KnownTypeConverters.TryGetValue(returnType, out TypeConverter typeConverterTo) && typeConverterTo.CanConvertFrom(valueType))
+ {
+ value = typeConverterTo.ConvertFrom(value);
+ return true;
+ }
+ if (KnownTypeConverters.TryGetValue(returnType, out typeConverterTo) && typeConverterTo.CanConvertFrom(typeof(string)))
{
value = typeConverterTo.ConvertFromInvariantString(value.ToString());
return true;
converting to non-nullable date time doesn't work, here's a test for it
diff --git a/src/Controls/tests/Core.UnitTests/DateOnlyTypeConverterTests.cs b/src/Controls/tests/Core.UnitTests/DateOnlyTypeConverterTests.cs
index 6185abaf30..255208402a 100644
--- a/src/Controls/tests/Core.UnitTests/DateOnlyTypeConverterTests.cs
+++ b/src/Controls/tests/Core.UnitTests/DateOnlyTypeConverterTests.cs
@@ -50,6 +50,22 @@ public class DateOnlyTypeConverterTests : BaseTestFixture
Assert.Equal(expectedDateTime, datePicker.Date);
}
+ [Fact]
+ public void DateOnlyToNonNullableBinding()
+ {
+ var dateProperty = BindableProperty.Create("Date", typeof(DateTime), typeof(DatePicker), null, BindingMode.TwoWay);
+ var source = new Issue20438DatePickerViewModel
+ {
+ SelectedDate = new DateOnly(2025, 3, 15)
+ };
+ var bo = new MockBindable { BindingContext = source };
+
+ bo.SetBinding(dateProperty, "SelectedDate");
+ var expectedDateTime = new DateTime(2025, 3, 15);
+ Assert.Equal(expectedDateTime, bo.GetValue(dateProperty));
+ }
+
+
public class Issue20438DatePickerViewModel
{
public DateOnly SelectedDate { get; set; }
To make that work, you'll have to either register the KnownTypeConverter twice, or make sure BindableProperty.TryConvert is agnostic to nullable
Also, there is no unit test for CompiledBindings, showing that the behaviour is th same. The easiest way to test that is to add a test in Xaml.UnitTests/Issues/. there's a pattern, you'll figure it out
| { typeof(ImageSource), new ImageSourceConverter() } | ||
| { typeof(ImageSource), new ImageSourceConverter() }, | ||
| #if NET6_0_OR_GREATER | ||
| { typeof(DateTime?), new DateTimeTypeConverter() }, |
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 you bind to a DateTime (non-nullable) property, no conversion happens
@StephaneDelcroix I've added a unit test under Xaml.UnitTests/Issues. Please let me know if any further changes are needed. |
| { typeof(ImageSource), new ImageSourceConverter() } | ||
| { typeof(ImageSource), new ImageSourceConverter() }, | ||
| #if NET6_0_OR_GREATER | ||
| { typeof(DateTime?), new DateTimeTypeConverter() }, |
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.
I would have preferred to avoid the duplication...
e24ebf7 to
9cf6825
Compare
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
6490390 to
a51d0af
Compare
|
I rebased, squashed, fixed (tests weren't passing) and force-pushed your branch. let's see if it builds |
|
Azure Pipelines successfully started running 3 pipeline(s). |
allow bindings between DateTime properties and DateTimeOnly values. Same to Time - fixes dotnet#20438 - closes dotnet#21989
a51d0af to
b606c5f
Compare
| if (KnownTypeConverters.TryGetValue(targetType, out TypeConverter typeConverterTo) && typeConverterTo.CanConvertFrom(valueType)) | ||
| { | ||
| value = typeConverterTo.ConvertFromInvariantString(value.ToString()); | ||
| value = typeConverterTo.ConvertFromInvariantString(Convert.ToString(value, CultureInfo.InvariantCulture)); |
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.
there was a bug here
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
/backport to release/10.0.1xx-rc1 |
|
Started backporting to release/10.0.1xx-rc1: https://github.com/dotnet/maui/actions/runs/17127006079 |
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!
Fix Details
MAUI DatePicker and TimePicker only supported DateTime/TimeSpan binding. In .NET 6 we have the option to use DateOnly and TimeOnly - provided support for these types to work seamlessly with MAUI controls.
Added TypeConverter support to enable automatic conversion for DateOnly and TimeOnly types, allowing them to be bound directly to DatePicker and TimePicker controls without manual conversion.
Issues Fixed
Fixes #20438
Existing PR: #21989
Screenshots