Avalonia
Avalonia copied to clipboard
UpdateSourceTrigger
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
- [X] 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 https://github.com/AvaloniaUI/Avalonia/issues/3754
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]
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]
Maybe you can add an example to BindingDemo? (side note: styles are broken in there, can probably switch to fluent theme).
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 https://github.com/AvaloniaUI/Avalonia/pull/5831 is merged in order to do this properly, because this feature breaks the layering we currently have.
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 |
@pr8x wanted to take a look at this, but seems a bunch of binding unit tests are failing?
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]
@grokys UTs fixed.
@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.
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.
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.
I'm kinda 👎 on merging this PR right now for a couple of reasons:
- 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
- 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?
@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.
Ok, lets close this and I'll get started on refactoring the binding system soon.
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.