microsoft / microsoft-ui-xaml

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

Add public method to Page control that ensures all TwoWay bindings are updated. (Form Submit) #5473

Open omnilogix opened 3 years ago

omnilogix commented 3 years ago

Proposal: Add pubic method to Page control that ensures all TwoWay bindings are updated. (Form Submit)

Summary

Add a method or equivalent feature that ensures that all TwoWay bindings on a Page or UserControl etc. are updated. This would be called during Form submission to ensure the ViewModel is in sync with the user input at time of validation and data save.

Rationale

Scope

Capability Priority
This proposal will allow developers to accomplish Proper data binding Must
This proposal will allow end users to be sure their input data is validated and saved Should

Important Notes

Open Questions

mrlacey commented 3 years ago

Why only do this on a Page control? Wouldn't it also make sense to have this on a Window or any custom control too?

If this is specifically related to Forms, should this be connected to #82 and as it has connections to validation, a consideration for #179 may also be appropriate.

The idea of "accomplish Proper data binding" is vague and potentially judgmental. Is the goal of avoiding workarounds and seemingly unnecessary extra code that is also hard to test also appropriate.

Would it be sufficient to have a button press also move focus? (I suspect not, but this needs exploring and documenting.)

Even doing the proposed seems like only a potential partial solution. As a keyboard accelerator or button click can be used to invoke a command that might start a form submission process, there's no guarantee that the form is also being updated to reflect that a background operation is in progress. A clear way of saying "the form should go into submission mode" is needed. This would force all binding updates, so validation can be done and then the UI appropriately updated to indicate that something is happening while any background operation is done. (hence the links to the issues mentioned above)

omnilogix commented 3 years ago

@mrlacey In the broader scope, yes, this feature should be available for any hosting control, but probably most useful as a feature of the binding system itself. If we can call "UpdateBindings(..)", then we don't have to be concerned about the current state of the focus, managing focus, or even which kind of control currently holds the focus. My particular concern is for data validation in LOB type apps. I'm sure there are many other scenarios that would benefit.

StephenLPeters commented 3 years ago

@marb2000 @ryandemopoulos @MikeHillberg Seems like a good idea.

MikeHillberg commented 3 years ago

Love to have some form of this too (no pun intended).

Two existing precedences to think about

chrisglein commented 3 years ago

Agreed that from a scenario perspective this feels like part of the need for a Forms control.

If this is specifically related to Forms, should this be connected to #82 and as it has connections to validation, a consideration for #179 may also be appropriate.

So +1 to that.

It does sound like there might be some binding features from WPF that could be relevant, independent from the Forms scenario:

WPF has BindingGroup for this purpose, which is a single object that represents all of the {Bindings} in scope. The CommitEdit() method on that flushes all the TwoWay bindings back to source.

So we can keep this issue open to track that class of feature. @omnilogix it would be helpful if you could create a small illustrative repro of exactly what you'd hope to get here, that could be used as the benchmark of whether the binding feature meets the needs or not.

But to set expectations on this:

Now is the time to include such functionality. Before GA release

WinUI's first job is to replicate the experience of UWP XAML, lifted outside of the OS. That's what's generally expected for initial release, and there will still be gaps between those. Additional gaps between UWP and WPF are tracked (see these issues) to potentially be tackled over time.

omnilogix commented 3 years ago

@chrisglein I'll work on getting a repo together, but I am a little short on time right now. In summary though, The heart of the issue is that the currently focused control does not update it's binding in all form-submit scenarios. The WPF BindingGroup creates the necessity to add code for all potential TwoWay bound controls which in my opinion adds too much complexity. All that is really necessary is a way to Trigger a binding update for the control that currently holds the focus. Where exactly is the best place to plumb that in, I am not sure. I do know that a "FlushTwoWayBindings()" or "UpdateFocusedBinding()" method would solve all of the issues that I deal with as far as forms submission go. I really don't feel that this is a "Forms" issue so much as a binding issue. Just my 2 cents.

MikeHillberg commented 3 years ago

It's a binding issue that's common to the forms scenario, but you're right it's not limited to forms.

Can you do this today using FrameworkElement.GetBindingExpression to get the binding from the TextBox.Text property, and then call BindingExpression.UpdateSource on it to flush the value?

omnilogix commented 3 years ago

It's a binding issue that's common to the forms scenario, but you're right it's not limited to forms.

Can you do this today using FrameworkElement.GetBindingExpression to get the binding from the TextBox.Text property, and then call BindingExpression.UpdateSource on it to flush the value?

@MikeHillberg Not with the x:Bind, which I am using primarily

MikeHillberg commented 3 years ago

Got it. That's where I'd like to go too. For x:Bind we have this.Bindings.Update() to update from source to target, what we need is a this.Bindings.UpdateSource() to go in the other direction.

omnilogix commented 3 years ago

@MikeHillberg Is the this.Bindings.UpdateSource() available for WinUI? I didn't find it with the Page Control. The docs looked like it was for UWP. Thanks

MikeHillberg commented 3 years ago

You're right, it's not. I was suggesting it as a solution.

oliverw commented 1 year ago

You're right, it's not. I was suggesting it as a solution.

So what are you supposed to do call in WinUI to force refresh the bindings?