From 27df5a2bd9d8e5571a4f7228a599571c64352070 Mon Sep 17 00:00:00 2001 From: Steven Kirk Date: Thu, 19 Nov 2020 09:50:12 +0100 Subject: [PATCH] Fix LocalValue bindings during batch update. LocalValue bindings are special case due to their interaction with the local value set by a non-binding. --- .../PropertyStore/PriorityValue.cs | 16 ++++++ src/Avalonia.Base/ValueStore.cs | 5 ++ .../AvaloniaObjectTests_BatchUpdate.cs | 52 +++++++++++++++---- 3 files changed, 63 insertions(+), 10 deletions(-) diff --git a/src/Avalonia.Base/PropertyStore/PriorityValue.cs b/src/Avalonia.Base/PropertyStore/PriorityValue.cs index 723cd7eef5..f49d6129f9 100644 --- a/src/Avalonia.Base/PropertyStore/PriorityValue.cs +++ b/src/Avalonia.Base/PropertyStore/PriorityValue.cs @@ -27,6 +27,7 @@ namespace Avalonia.PropertyStore private Optional _localValue; private Optional _value; private bool _isCalculatingValue; + private bool _batchUpdate; public PriorityValue( IAvaloniaObject owner, @@ -93,6 +94,8 @@ namespace Avalonia.PropertyStore public void BeginBatchUpdate() { + _batchUpdate = true; + foreach (var entry in _entries) { (entry as IBatchUpdate)?.BeginBatchUpdate(); @@ -101,6 +104,8 @@ namespace Avalonia.PropertyStore public void EndBatchUpdate() { + _batchUpdate = false; + foreach (var entry in _entries) { (entry as IBatchUpdate)?.EndBatchUpdate(); @@ -165,6 +170,17 @@ namespace Avalonia.PropertyStore var binding = new BindingEntry(_owner, Property, source, priority, this); var insert = FindInsertPoint(binding.Priority); _entries.Insert(insert, binding); + + if (_batchUpdate) + { + binding.BeginBatchUpdate(); + + if (priority == BindingPriority.LocalValue) + { + binding.Start(ignoreBatchUpdate: true); + } + } + return binding; } diff --git a/src/Avalonia.Base/ValueStore.cs b/src/Avalonia.Base/ValueStore.cs index 6edefc36a9..feafe5193d 100644 --- a/src/Avalonia.Base/ValueStore.cs +++ b/src/Avalonia.Base/ValueStore.cs @@ -313,6 +313,11 @@ namespace Avalonia if (slot is IPriorityValueEntry e) { priorityValue = new PriorityValue(_owner, property, this, e); + + if (_batchUpdate is object) + { + priorityValue.BeginBatchUpdate(); + } } else if (slot is PriorityValue p) { diff --git a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_BatchUpdate.cs b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_BatchUpdate.cs index cfde361646..c556ac70c5 100644 --- a/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_BatchUpdate.cs +++ b/tests/Avalonia.Base.UnitTests/AvaloniaObjectTests_BatchUpdate.cs @@ -233,40 +233,72 @@ namespace Avalonia.Base.UnitTests } [Fact] - public void Bindings_Should_Not_Be_Subscribed_During_Batch_Update() + public void LocalValue_Bindings_Should_Be_Subscribed_During_Batch_Update() { var target = new TestClass(); var observable1 = new TestObservable("foo"); var observable2 = new TestObservable("bar"); - var observable3 = new TestObservable("baz"); + var raised = new List(); + + target.PropertyChanged += (s, e) => raised.Add(e); + // We need to subscribe to LocalValue bindings even if we've got a batch operation + // in progress because otherwise we don't know whether the binding or a subsequent + // SetValue with local priority will win. Notifications however shouldn't be sent. target.BeginBatchUpdate(); target.Bind(TestClass.FooProperty, observable1, BindingPriority.LocalValue); target.Bind(TestClass.FooProperty, observable2, BindingPriority.LocalValue); - target.Bind(TestClass.FooProperty, observable3, BindingPriority.Style); + + Assert.Equal(1, observable1.SubscribeCount); + Assert.Equal(1, observable2.SubscribeCount); + Assert.Empty(raised); + } + + [Fact] + public void Style_Bindings_Should_Not_Be_Subscribed_During_Batch_Update() + { + var target = new TestClass(); + var observable1 = new TestObservable("foo"); + var observable2 = new TestObservable("bar"); + + target.BeginBatchUpdate(); + target.Bind(TestClass.FooProperty, observable1, BindingPriority.Style); + target.Bind(TestClass.FooProperty, observable2, BindingPriority.StyleTrigger); Assert.Equal(0, observable1.SubscribeCount); Assert.Equal(0, observable2.SubscribeCount); - Assert.Equal(0, observable3.SubscribeCount); } [Fact] - public void Active_Binding_Should_Be_Subscribed_After_Batch_Uppdate() + public void Active_Style_Binding_Should_Be_Subscribed_After_Batch_Uppdate_1() { var target = new TestClass(); var observable1 = new TestObservable("foo"); var observable2 = new TestObservable("bar"); - var observable3 = new TestObservable("baz"); target.BeginBatchUpdate(); - target.Bind(TestClass.FooProperty, observable1, BindingPriority.LocalValue); - target.Bind(TestClass.FooProperty, observable2, BindingPriority.LocalValue); - target.Bind(TestClass.FooProperty, observable3, BindingPriority.Style); + target.Bind(TestClass.FooProperty, observable1, BindingPriority.Style); + target.Bind(TestClass.FooProperty, observable2, BindingPriority.Style); target.EndBatchUpdate(); Assert.Equal(0, observable1.SubscribeCount); Assert.Equal(1, observable2.SubscribeCount); - Assert.Equal(0, observable3.SubscribeCount); + } + + [Fact] + public void Active_Style_Binding_Should_Be_Subscribed_After_Batch_Uppdate_2() + { + var target = new TestClass(); + var observable1 = new TestObservable("foo"); + var observable2 = new TestObservable("bar"); + + target.BeginBatchUpdate(); + target.Bind(TestClass.FooProperty, observable1, BindingPriority.StyleTrigger); + target.Bind(TestClass.FooProperty, observable2, BindingPriority.Style); + target.EndBatchUpdate(); + + Assert.Equal(1, observable1.SubscribeCount); + Assert.Equal(0, observable2.SubscribeCount); } public class TestClass : AvaloniaObject