UpdateSourceTrigger

[pr8x]

What does the pull request do?

Implement WPF's UpdateSourceTrigger (https://docs.microsoft.com/en-us/dotnet/api/system.windows.data.binding.updatesourcetrigger?view=windowsdesktop-6.0)

TODO:

• Documentation
• I need a proper interface for getting focus loss event inside Avalonia.Base. Any ideas? There could be some IUpdateSourceTriggerLostFocusProvider or something. Right now I am just checking for "IsFocused" property (by name) which is not a good solution for obvious reasons.

What is the current behavior?

It's not supported.

Checklist

• Added unit tests (if possible)?
• Added XML documentation to any related classes?
• Consider submitting a PR to https://github.com/AvaloniaUI/Documentation with user documentation

Breaking changes

Most important ones:
InstancedBinding.ctor()
InstancedBinding.TwoWay()
InstancedBinding.OneWayToSource()

These could be defaulted to UpdateSourceTrigger.Default though.

Fixed issues

Fixes #3754

[avaloniaui-team]

You can test this PR using the following package version. 0.10.999-cibuild0019276-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json) [PRBUILDID]

[avaloniaui-team]

You can test this PR using the following package version. 0.10.999-cibuild0019289-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json) [PRBUILDID]

[MarchingCube]

Maybe you can add an example to BindingDemo? (side note: styles are broken in there, can probably switch to fluent theme).

[MarchingCube]

Did a brief review. It seems UpdateSource might be a Pandora's box but in general having it would help writing controllable binding code.

[grokys]

I need a proper interface for getting focus loss event inside Avalonia.Base. Any ideas?

To be honest, I think we need to wait until #5831 is merged in order to do this properly, because this feature breaks the layering we currently have.

[grokys]

A few comments, but unable to get much further with the review because there seems to be a problem with the PR as evidenced by the failing tests.

My main concern here will be that of performance, and adding the additional explicitSourceUpdate observable is probably going to add a bit of weight - especially as it uses rx operators which we shouldn't really be using at this low level due to their lousy performance.

[pr8x]

Decided to remove UpdateSource and UpdateTarget for now. I feel like exposing them on the InstancedBinding is not really helpful in many cases and they have some overhead even when they're not used. I wonder if it's really worth having them.

---

Anyways, ran some benchmarks and it seems that this PR improves bindings a bit:

Master

Method	Mean	Error	StdDev	Gen 0	Gen 1	Allocated
TwoWayBinding_Via_Binding	3.984 μs	0.0778 μs	0.0895 μs	0.3395	0.0076	3 KB
UpdateTwoWayBinding_Via_Binding	118.075 μs	0.3807 μs	0.3375 μs	4.1504	-	34 KB

This PR

Method	Mean	Error	StdDev	Gen 0	Gen 1	Allocated
TwoWayBinding_Via_Binding	3.177 μs	0.0447 μs	0.0396 μs	0.3014	0.0076	2 KB
UpdateTwoWayBinding_Via_Binding	109.579 μs	0.9755 μs	0.9125 μs	4.0283	-	34 KB

[grokys]

@pr8x wanted to take a look at this, but seems a bunch of binding unit tests are failing?

[avaloniaui-team]

You can test this PR using the following package version. 0.10.999-cibuild0022898-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json) [PRBUILDID]

[pr8x]

@grokys UTs fixed.

[TomEdwardsEnscape]

@pr8x Our product is being ported from WPF and needs UpdateSourceTrigger.Explicit, UpdateSource, and UpdateTarget.

We can work around the lack of these options these by creating proxy properties and assigning values or bindings to them at times of our choosing, but this is an ugly hack that can cause confusion among control consumers.

Have you seen the method AvaloniaObject.CoerceValue? This sort of does the same thing as UpdateTarget, but a) only for StyledProperty properties and b) by recalculating the effective property value, not a specific binding.

Between the AvaloniaObject._directBindings and ValueStore._values fields, it seem to me that direct access to bindings applied to each property should be possible, allowing something equivalent to this WPF call:

myControl.GetBindingExpression(Control.MyProperty).UpdateTarget();

Use case for UpdateSource

We have a map pin that the user can drag around the screen. We need other UI elements to immediately respond to its position changing, but we only want to update the position value in our viewmodel (via a two-way binding on the pin) when the user lets go of the mouse.

Use case for UpdateTarget

Our TimePicker template includes text boxes which the user can type into. If the control is in 12-hour mode and set to 11pm, then when the user types "23" into the hour field and commits the value, we need the box to change to display "11". But because the underlying TimeSpan value has not changed, no event is raised, the binding is not re-evaluated, and the box continues to display "23". We need to manually re-run the text box binding.

[Tenjim]

Hi guys,
do you have news about this merge ?
I'm trying to find workaround like using BindingOperations.Apply but I'm not sure is the right way to force an update on a binding

Thanks in advance.

[robloo]

Also would like to see an update on the status here. @pr8x Are you waiting for core maintainer feedback?

It would be great to get this in for 11.0.

[grokys]

I'm kinda 👎 on merging this PR right now for a couple of reasons:

1. We to do a major refactor of the binding system very soon anyway, and would like to include this feature anyway. The way it's implemented here feels rather bolted onto an architecture that wasn't really designed for it
2. There's only 1 unit test. I'm 99% sure that one unit test isn't going to cover all of the edge-cases

In the absence of more unit tests, I'd be ok with merging if it'd had extensive testing. I think I asked you a while ago @pr8x if you'd tested this in your product and you hadn't yet. Have you had chance to test it now?

[pr8x]

@grokys I did test it in one scenario and it was working fine.

At this point this PR was stagnant waiting for review that we might as well close it and wait for the refactoring of the bindings. Current binding system seems messy anyway relying way too much on observables/LINQ selectors.

[grokys]

Ok, lets close this and I'll get started on refactoring the binding system soon.

[mightypanda]

Hello,
Any update on the possibility to have this feature (and refactoring of the binding system) in new 11 previews?
The workaround using a Behavior for lost focus seems not to work anymore.