Skip to content
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

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

Open
omnilogix opened this issue Jul 15, 2021 · 13 comments
Assignees
Labels
area-Binding feature proposal New feature proposal product-winui3 WinUI 3 issues team-Markup Issue for the Markup team wpf-vs-winui-mismatch

Comments

@omnilogix
Copy link

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

  • Current UI design allows for a button click or keyboard accelerator that does not change the current focus. Without focus change, TextBox default UpdateSourceTrigger of FocusChanged will not fire, and data binding remains incomplete for the currently focused input, allowing for un-validated as well as un-saved user data.
  • In previous frameworks (WPF, etc), the binding could (with significant effort) be programmatically discovered and updates could be forced when necessary. With x:Bind, this is no longer possible.
  • This not-so-edge case has been overlooked in all previous frameworks
  • One might argue that UpdateSourceTrigger could be set to PropertyChanged. While true, this is far from optimal, especially when performing validation which would usually prefer to be on FocusChanged event, not after every keystroke.
  • Now is the time to include such functionality. Before GA release
  • Getting the currently focused control and trying to move the focus is unreliable at best. There should be a way to explicity update the bindings, or implicitly through a feature that would trigger a focus change event on the current control

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

@omnilogix omnilogix added the feature proposal New feature proposal label Jul 15, 2021
@ghost ghost added the needs-triage Issue needs to be triaged by the area owners label Jul 15, 2021
@mrlacey
Copy link
Contributor

mrlacey commented Jul 15, 2021

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
Copy link
Author

@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
Copy link
Contributor

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

@MikeHillberg
Copy link
Contributor

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

Two existing precedences to think about

  • 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.
  • x:Bind in WinUI provides an Update() method which unfortunately goes in the opposite direction from the case here. That's useful too but not the scenario proposed here. There's not a lot of documentation but there is some here. So on your page/usercontrol/etc you can call this.Bindings.Update(); to get all of the binds to update from source to the view.

@chrisglein chrisglein changed the title Add pubic method to Page control that ensures all TwoWay bindings are updated. (Form Submit) Add public method to Page control that ensures all TwoWay bindings are updated. (Form Submit) Jul 20, 2021
@chrisglein
Copy link
Member

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.

@chrisglein chrisglein removed the needs-triage Issue needs to be triaged by the area owners label Jul 20, 2021
@omnilogix
Copy link
Author

omnilogix commented Jul 21, 2021

@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
Copy link
Contributor

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
Copy link
Author

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
Copy link
Contributor

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
Copy link
Author

@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
Copy link
Contributor

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

@oliverw
Copy link

oliverw commented Feb 17, 2023

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?

@bpulliam bpulliam added team-Markup Issue for the Markup team and removed team-Framework labels Aug 22, 2023
@microsoft-github-policy-service microsoft-github-policy-service bot added the needs-triage Issue needs to be triaged by the area owners label Aug 22, 2023
@bpulliam bpulliam removed area-Navigation Frame, Page needs-triage Issue needs to be triaged by the area owners labels Aug 23, 2023
@omnilogix
Copy link
Author

Has there been any movement on this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-Binding feature proposal New feature proposal product-winui3 WinUI 3 issues team-Markup Issue for the Markup team wpf-vs-winui-mismatch
Projects
None yet
Development

No branches or pull requests

8 participants