microsoft / microsoft-ui-xaml

Windows UI Library: the latest Windows 10 native controls and Fluent styles for your applications
MIT License
6.15k stars 666 forks source link

Several problems with Binding, x:Bind, and date/time controls, affecting both UWP and WinUI 3 (Project Reunion 0.5) #4737

Open nCastle1 opened 3 years ago

nCastle1 commented 3 years ago

Describe the bug

Apologies in advance for the omnibus bug report; I tried to be more focused, but there's a lot going on and its not clear where one bug ends and another begins. I suspect there's an interplay of the CsWinRT bindings, specific Microsoft.UI.xaml control implementations, and other aspects of the binding frameworks.

Nearly every workaround for a WinUI/UWP datetime support bug leads to another. Taken as a whole, it is safe to say WinUI, consumed via C#, does not, in practice, support data binding with date/time properties

All of the testing I did while logging this issue happened with a Win32 packaged app; some of the issues were initially discovered on UWP, so my assumption is that they are problems with WinUI or adjacent code.

I wish I could chalk this up to WinUI being in a pre-release state, but many of the problems are quite old (2+ years at least). Some new WinUI specific bugs deprive developers of use of the few workarounds needed to get things working. Typically when I find a bug with x:Bind bindings in UWP, I switch to Binding, and vice versa. With WinUI 3 it is quite likely that developers will end up with no viable way (short of using custom controls and circumventing the framework entirely) of offering date selection/entry in their UI.

For all of the XAML repro code, I'm using the following test view model:

test view model

```cs public class TestViewModel : INotifyPropertyChanged { private DateTimeOffset? _startingNull = null; private DateTimeOffset? _startingNotNull = new DateTimeOffset(DateTime.Now); private DateTimeOffset _notNullable; private DateTimeOffset _notNullableAndSet = new DateTimeOffset(DateTime.Now); private string _testString = "default value"; public DateTimeOffset? StartingNullTime { get => _startingNull; set { if (_startingNull != value) { _startingNull = value; PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(StartingNullTime))); } } } public DateTimeOffset? StartingNotNullTime { get => _startingNotNull; set { if (value != _startingNotNull) { _startingNotNull = value; PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(StartingNotNullTime))); } } } public DateTimeOffset NotNullTime { get => _notNullable; set { if (value != _notNullable) { _notNullable = value; PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(NotNullTime))); } } } public DateTimeOffset NotNullAndSetTime { get => _notNullableAndSet; set { if (value != _notNullableAndSet) { _notNullableAndSet = value; PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(NotNullAndSetTime))); } } } public string TestString { get => _testString; set { if (_testString != value) { _testString = value; PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(nameof(TestString))); } } } public event PropertyChangedEventHandler PropertyChanged; } ```

Problems

When binding (but not x:Bind binding) custom DateTimeOffset? (but not DateTimeOffset) dependency properties there is a binding conversion error for Windows.Foundation.DateTime

According to the docs, Windows.Foundation.DateTime is an implementation detail that should not be visible to the user. Nowhere in my code is Windows.Foundation.DateTime used, so I was surprised to see this. I originally found this issue (and it is the reason I came to GitHub to file) with UWP and can repro with the latest WinUI 3 release. The conversion error happens when Binding a dependency property of type `DateTimeOffset?` but not `DateTimeOffset`. Example binding error output: ```txt Error: Converter failed to convert value of type 'Windows.Foundation.DateTime' to type 'IReference`1'; BindingExpression: Path='StartingNullTime' DataItem='UWPBindingRepro.TestViewModel'; target element is 'UWPBindingRepro.DateTimeEditor' (Name='null'); target property is 'NullableDateTimeOffset' (type 'IReference`1'). ```

When a CalendarDatePicker's Date property is two-way Binding bound to a DateTimeOffset property and that property is set (pce raised) WinUI throws a COMException: E_FAIL returned from COM component

```xml ``` Stack trace: ```txt at WinRT.ExceptionHelpers.ThrowExceptionForHR(Int32 hr) at ABI.System.ComponentModel.PropertyChangedEventHandler.NativeDelegateWrapper.Invoke(Object sender, PropertyChangedEventArgs e) at System.ComponentModel.PropertyChangedEventHandler.Invoke(Object sender, PropertyChangedEventArgs e) at UWPBindingRepro.TestViewModel.set_NotNullAndSetTime(DateTimeOffset value) in E:\ReMapped\Repros\UWPBindingTime\UWPBindingRepro\UWPBindingRepro\UWPBindingRepro\BasicViewModel.cs:line 73 at UWPBindingRepro.TestViewModel.ChangeStartingNullTime() in E:\ReMapped\Repros\UWPBindingTime\UWPBindingRepro\UWPBindingRepro\UWPBindingRepro\BasicViewModel.cs:line 24 at UWPBindingRepro.MainWindow.Button_Click(Object sender, RoutedEventArgs e) in E:\ReMapped\Repros\UWPBindingTime\UWPBindingRepro\UWPBindingRepro\UWPBindingRepro\MainWindow.xaml.cs:line 35 at ABI.Microsoft.UI.Xaml.RoutedEventHandler.<>c__DisplayClass10_0.b__0(RoutedEventHandler invoke) at WinRT.ComWrappersSupport.MarshalDelegateInvoke[T](IntPtr thisPtr, Action`1 invoke) at ABI.Microsoft.UI.Xaml.RoutedEventHandler.Do_Abi_Invoke(IntPtr thisPtr, IntPtr sender, IntPtr e) ```

When a CalendarDatePicker's Date property is two-way x:Bind bound to a DateTimeOffset or DateTimeOffset? property, and that property is set (pce raised) WinUI throws InvalidCastException

``` ``` Stack trace: ```txt at System.Runtime.InteropServices.Marshal.ThrowExceptionForHR(Int32 errorCode) at WinRT.IObjectReference.As[T](Guid iid) at WinRT.IObjectReference.As[T]() at WinRT.MarshalInterface`1.CreateMarshaler(T value) at ABI.Microsoft.UI.Xaml.Controls.ICalendarDatePicker.global::Microsoft.UI.Xaml.Controls.ICalendarDatePicker.set_Date(Nullable`1 value) at Microsoft.UI.Xaml.Controls.CalendarDatePicker.set_Date(Nullable`1 value) at UWPBindingRepro.MainWindow.XamlBindingSetters.Set_Microsoft_UI_Xaml_Controls_CalendarDatePicker_Date(CalendarDatePicker obj, Nullable`1 value, String targetNullValue) in E:\ReMapped\Repros\UWPBindingTime\UWPBindingRepro\UWPBindingRepro\UWPBindingRepro\obj\x86\Debug\net5.0-windows10.0.19041.0\MainWindow.g.cs:line 29 at UWPBindingRepro.MainWindow.MainWindow_obj1_Bindings.Update_MainViewModel_NotNullAndSetTime(DateTimeOffset obj, Int32 phase) in E:\ReMapped\Repros\UWPBindingTime\UWPBindingRepro\UWPBindingRepro\UWPBindingRepro\obj\x86\Debug\net5.0-windows10.0.19041.0\MainWindow.g.cs:line 243 at UWPBindingRepro.MainWindow.MainWindow_obj1_Bindings.MainWindow_obj1_BindingsTracking.PropertyChanged_MainViewModel(Object sender, PropertyChangedEventArgs e) in E:\ReMapped\Repros\UWPBindingTime\UWPBindingRepro\UWPBindingRepro\UWPBindingRepro\obj\x86\Debug\net5.0-windows10.0.19041.0\MainWindow.g.cs:line 380 at UWPBindingRepro.TestViewModel.set_NotNullAndSetTime(DateTimeOffset value) in E:\ReMapped\Repros\UWPBindingTime\UWPBindingRepro\UWPBindingRepro\UWPBindingRepro\BasicViewModel.cs:line 73 at UWPBindingRepro.TestViewModel.ChangeStartingNullTime() in E:\ReMapped\Repros\UWPBindingTime\UWPBindingRepro\UWPBindingRepro\UWPBindingRepro\BasicViewModel.cs:line 24 at UWPBindingRepro.MainWindow.Button_Click(Object sender, RoutedEventArgs e) in E:\ReMapped\Repros\UWPBindingTime\UWPBindingRepro\UWPBindingRepro\UWPBindingRepro\MainWindow.xaml.cs:line 35 at ABI.Microsoft.UI.Xaml.RoutedEventHandler.<>c__DisplayClass10_0.b__0(RoutedEventHandler invoke) at WinRT.ComWrappersSupport.MarshalDelegateInvoke[T](IntPtr thisPtr, Action`1 invoke) at ABI.Microsoft.UI.Xaml.RoutedEventHandler.Do_Abi_Invoke(IntPtr thisPtr, IntPtr sender, IntPtr e) ```

CalendarDatePicker uses placeholder when value is set to null via x:Bind, but selects current date minus 100 years when value is two-way bound via Binding

![image](https://user-images.githubusercontent.com/29742178/113497402-a51d8280-94b8-11eb-89cd-e8883d7b75b4.png) ```xml ```

Binding CalendarDataPicker.Date to any DateTimeOffset? or DateTimeOffset property, regardless of value, using Binding fails

As you can see in the screenshot, binding to the properties using a textblock works without issue. The CalendarDatePicker both ignores its placeholder value and the property it is bound to. When CalendarDatePicker is bound (using binding) to a null value or when the binding fails, it defaults to 1/1/{current year minus 100}, which is strange. This is the same behavior as old style UWP. ![image](https://user-images.githubusercontent.com/29742178/113497168-20ca0000-94b6-11eb-8bef-ffda25756900.png) ```xml ``` DataContext is set on RootElement in code behind: ```cs RootElement.DataContext = MainViewModel; ```

Some uses of x:Bind (binding CalendarDatePicker.Date to either DateTimeOffset or DateTimeOffset?) will break all other x:Bind bindings in the window

Note: I haven't done robust debugging to find out if this is an issue at the app level or window level... I came to log one issue and stumbled on 10, I don't have time to research them all in depth. As noted in the below snippet, two of the calendar date picker bindings will break all other x:Bind bindings; no exception is thrown, they just silently fail. Lines can be uncommented individually, they both independently break bindings ```xml ``` Before uncommenting: ![image](https://user-images.githubusercontent.com/29742178/113497321-cb8eee00-94b7-11eb-9a2a-e4e80f7542a9.png) After uncommenting: ![image](https://user-images.githubusercontent.com/29742178/113497334-ebbead00-94b7-11eb-8786-c340c1db7c96.png)

These issues aren't bugs, but they have an almost bug-level impact on the developer experience; they would be low-cost high-impact fixes and make WinUI even more attractive than WPF, if accomplished

* The .NET Framework date/time types combine the date and time fields into one property (e.g. DateTime, DateTimeOffset). This makes binding difficult when there are only controls for separate parts (DatePicker, TimePicker). While it is possible for the developer to create a custom control that combines both and enables binding a single field to both a calendar and a time picker, that is cumbersome and time-consuming (especially when working around the bugs and limitations in the controls, cswinrt projection, and binding frameworks). A built-in DateTimePicker would be very convenient. * TimePicker and DatePicker have nullable Selected{Date/Time} properties in addition to non-nullable Date/Time properties. CalendarDatePicker only has the nullable Date property (but it doesn't work particularly well, see above). The API is inconsistent. Before release, it would be great to see these APIs rationalized a bit, and perhaps made more flexible when set via binding. Its not a showstopper to throw an exception when I bind a nullable value to the Date property instead of the SelectedDate property, but it is very annoying.

Version Info

NuGet package version:

WinUI 3 - Project Reunion 0.5: 0.5.0

Windows app type: UWP Win32
Yes
Windows 10 version Saw the problem?
Insider Build (xxxxx)
October 2020 Update (19042)
May 2020 Update (19041) Yes
November 2019 Update (18363)
May 2019 Update (18362)
October 2018 Update (17763)
April 2018 Update (17134)
Fall Creators Update (16299)
Creators Update (15063)
Device form factor Saw the problem?
Desktop Yes
Xbox
Surface Hub
IoT

Additional context

There's a lot going on here, it feels like a rats nest of bugs rather than any one bug or category of bugs.

I will note that many of these individual bugs have been logged (and ignored) elsewhere, often for years. What's new about this, is that with the new parts, date and time support is now almost entirely broken, so that felt worthy of a new bug; the impact of this bug is greater than the sum of its parts.

StephenLPeters commented 3 years ago

@fabiant3 @chrisglein and @RealTommyKlein FYI about the reported binding and x:bind issues

wbokkers commented 3 years ago

Also when setting the ExperationTime of a toast notification, I get an exception because of a DateTimeOffset conversion.

Exception message: Specified cast is not valid.

StackTrace at System.Runtime.InteropServices.Marshal.ThrowExceptionForHR(Int32 errorCode) at WinRT.IObjectReference.As[T](Guid iid) at WinRT.IObjectReference.As[T]() at lambda_method869(Closure , IObjectReference ) at WinRT.MarshalInterface1.CreateMarshaler(T value) at ABI.Windows.UI.Notifications.IToastNotification.global::Windows.UI.Notifications.IToastNotification.set_ExpirationTime(Nullable1 value)

manodasanW commented 3 years ago

Some of the cast issues mentioned here should be resolved with the CsWinRT fix in .NET SDK 5.0.202.

billhenn commented 3 years ago

@manodasanW - I am on .NET SDK 5.0.202 and have a error when binding one POCO property of type int? (nullable Int32) to another dependency property on our DoubleEditBox control that is also type int?.

This is for code from UWP that is being ported to WinUI and never threw an error before in UWP. In WinUI, I get this in VS output:

Error: Converter failed to convert value of type 'Windows.Foundation.Int32' to type 'IReference`1<Int32>'; BindingExpression: Path='ValuePropertyEditor.RoundingDecimalPlace' DataItem='ActiproSoftware.UI.Xaml.Controls.Grids.PropertyData.PropertyModel'; target element is 'ActiproSoftware.UI.Xaml.Controls.Editors.DoubleEditBox' (Name='null'); target property is 'RoundingDecimalPlace' (type 'IReference`1<Int32>'). 

Is this a known issue?

To summarize it again:

And here's the XAML:

    <DataTemplate x:Key="DoubleValueTemplate">
        <editors:DoubleEditBox 
            RoundingDecimalPlace="{Binding ValuePropertyEditor.RoundingDecimalPlace}"
            ...
            />
    </DataTemplate>
mrbronkman commented 2 years ago

Still unable to bind to CalendarDatePicker. Any workarounds?

ArchieCoder commented 2 years ago

Setting values will generate a crash depending of the order.

This will crash: CalendarPicker.MinDate = ViewModel.MinimumDate; CalendarPicker.MaxDate = ViewModel.MaximumDate; CalendarPicker.Date = ViewModel.SelectedDate;

This will not crash: CalendarPicker.Date = ViewModel.SelectedDate; CalendarPicker.MinDate = ViewModel.MinimumDate; CalendarPicker.MaxDate = ViewModel.MaximumDate;

github-actions[bot] commented 11 months ago

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 5 days.

ArchieCoder commented 11 months ago

Please close the bug, it is an important control