From 092cdc23266b57a294905d47bb86c633f9185ff2 Mon Sep 17 00:00:00 2001 From: danjoconnell Date: Thu, 19 Nov 2020 12:32:03 +1300 Subject: [PATCH 1/2] Added an internal ObservableStack collection implementing INotifyCollectionChanged and replaced Stack collection instances with ObservableStack instances in UndoRoot. Added unit test methods to confirm that UndoRoot.UndoStack and UndoRoot.RedoStack implement INotifyCollectionChanged and raise CollectionChanged events correctly. UI now shouldn't need manual refresh calls for these Stacks if consumed by a control/view supporting INotifyCollectionChanged collections. --- src/MonitoredUndo/ObservableStack.cs | 83 ++++++++++++++++ src/MonitoredUndo/UndoRoot.cs | 8 +- tests/MonitoredUndo.Tests/UndoTests.cs | 127 +++++++++++++++++++++++++ 3 files changed, 214 insertions(+), 4 deletions(-) create mode 100644 src/MonitoredUndo/ObservableStack.cs diff --git a/src/MonitoredUndo/ObservableStack.cs b/src/MonitoredUndo/ObservableStack.cs new file mode 100644 index 0000000..6ed8116 --- /dev/null +++ b/src/MonitoredUndo/ObservableStack.cs @@ -0,0 +1,83 @@ +/* + Based on sample code posted on Stack Overflow by 'Ernie S' (https://stackoverflow.com/users/1324284/ernie-s) + at https://stackoverflow.com/questions/3127136/observable-stack-and-queue/56177896#56177896 +*/ +using System.Collections.Generic; +using System.Collections.Specialized; +using System.ComponentModel; + +namespace MonitoredUndo +{ + /// + /// Observable version of the generic Stack collection + /// + internal class ObservableStack : Stack, INotifyCollectionChanged, INotifyPropertyChanged + { + #region Constructors + + public ObservableStack() : base() + { + } + + public ObservableStack(IEnumerable collection) : base(collection) + { + } + + public ObservableStack(int capacity) : base(capacity) + { + } + + #endregion + + #region Overrides + + public new virtual T Pop() + { + T item = base.Pop(); + OnCollectionChanged(NotifyCollectionChangedAction.Remove, item); + + return item; + } + + public new virtual void Push(T item) + { + base.Push(item); + OnCollectionChanged(NotifyCollectionChangedAction.Add, item); + } + + public new virtual void Clear() + { + base.Clear(); + OnCollectionChanged(NotifyCollectionChangedAction.Reset, default); + } + + #endregion + + #region CollectionChanged + + public virtual event NotifyCollectionChangedEventHandler CollectionChanged; + + protected virtual void OnCollectionChanged(NotifyCollectionChangedAction action, T item) + { + CollectionChanged?.Invoke(this, new NotifyCollectionChangedEventArgs(action, + item, + item == null ? -1 : 0) + ); + + OnPropertyChanged(nameof(Count)); + } + + #endregion + + #region PropertyChanged + + public virtual event PropertyChangedEventHandler PropertyChanged; + + protected virtual void OnPropertyChanged(string proertyName) + { + PropertyChanged?.Invoke(this, new PropertyChangedEventArgs(proertyName)); + } + + #endregion + } +} diff --git a/src/MonitoredUndo/UndoRoot.cs b/src/MonitoredUndo/UndoRoot.cs index 794183e..4a8a879 100644 --- a/src/MonitoredUndo/UndoRoot.cs +++ b/src/MonitoredUndo/UndoRoot.cs @@ -18,8 +18,8 @@ public class UndoRoot private WeakReference _Root; // The list of undo / redo actions. - private Stack _UndoStack; - private Stack _RedoStack; + private ObservableStack _UndoStack; + private ObservableStack _RedoStack; // Tracks whether a batch (or batches) has been started. private int _IsInBatchCounter = 0; @@ -53,8 +53,8 @@ public class UndoRoot public UndoRoot(object root) { _Root = new WeakReference(root); - _UndoStack = new Stack(); - _RedoStack = new Stack(); + _UndoStack = new ObservableStack(); + _RedoStack = new ObservableStack(); } diff --git a/tests/MonitoredUndo.Tests/UndoTests.cs b/tests/MonitoredUndo.Tests/UndoTests.cs index df7234c..86b819f 100644 --- a/tests/MonitoredUndo.Tests/UndoTests.cs +++ b/tests/MonitoredUndo.Tests/UndoTests.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Collections.Specialized; using System.Linq; using System.Text; using Microsoft.VisualStudio.TestTools.UnitTesting; @@ -306,6 +307,132 @@ public void UndoRoot_Raises_RedoStackChanged_Event_When_ChangeSet_Added() } } + [TestMethod] + public void UndoRoot_UndoStack_Implements_INotifyCollectionChanged() + { + string orig = Document1.A.Name; + var undoRoot = UndoService.Current[Document1]; + Assert.IsInstanceOfType(undoRoot.UndoStack, typeof(INotifyCollectionChanged)); + + bool wasCalledForAdd = false; + bool wasCalledForRemove = false; + bool wasCalledForReset = false; + + var callback = new NotifyCollectionChangedEventHandler((s, e) => + { + switch (e.Action) + { + case NotifyCollectionChangedAction.Add: + wasCalledForAdd = true; + break; + case NotifyCollectionChangedAction.Remove: + wasCalledForRemove = true; + break; + case NotifyCollectionChangedAction.Reset: + wasCalledForReset = true; + break; + } + }); + + ((INotifyCollectionChanged)undoRoot.UndoStack).CollectionChanged += callback; + + try + { + Document1.A.Name = "Updated1"; + Assert.IsTrue(wasCalledForAdd); + Assert.IsFalse(wasCalledForRemove); + Assert.IsFalse(wasCalledForReset); + + wasCalledForAdd = false; + + undoRoot.Undo(); + Assert.IsTrue(wasCalledForRemove); + Assert.IsFalse(wasCalledForAdd); + Assert.IsFalse(wasCalledForReset); + + wasCalledForRemove = false; + + undoRoot.Redo(); + Assert.IsTrue(wasCalledForAdd); + Assert.IsFalse(wasCalledForRemove); + Assert.IsFalse(wasCalledForReset); + + wasCalledForAdd = false; + + undoRoot.Clear(); + Assert.IsTrue(wasCalledForReset); + Assert.IsFalse(wasCalledForAdd); + Assert.IsFalse(wasCalledForRemove); + } + finally + { + ((INotifyCollectionChanged)undoRoot.UndoStack).CollectionChanged -= callback; + } + } + + [TestMethod] + public void UndoRoot_RedoStack_Implements_INotifyCollectionChanged() + { + string orig = Document1.A.Name; + var undoRoot = UndoService.Current[Document1]; + Assert.IsInstanceOfType(undoRoot.RedoStack, typeof(INotifyCollectionChanged)); + + bool wasCalledForAdd = false; + bool wasCalledForRemove = false; + bool wasCalledForReset = false; + + var callback = new NotifyCollectionChangedEventHandler((s, e) => + { + switch (e.Action) + { + case NotifyCollectionChangedAction.Add: + wasCalledForAdd = true; + break; + case NotifyCollectionChangedAction.Remove: + wasCalledForRemove = true; + break; + case NotifyCollectionChangedAction.Reset: + wasCalledForReset = true; + break; + } + }); + + ((INotifyCollectionChanged)undoRoot.RedoStack).CollectionChanged += callback; + + try + { + Document1.A.Name = "Updated1"; + Assert.IsTrue(wasCalledForReset, "Redo stack CollectionChanged event should have been called with NotifyCollectionChangedAction.Reset args because of the new change pruning the redo stack."); + Assert.IsFalse(wasCalledForAdd); + Assert.IsFalse(wasCalledForRemove); + + wasCalledForReset = false; + + undoRoot.Undo(); + Assert.IsTrue(wasCalledForAdd); + Assert.IsFalse(wasCalledForRemove); + Assert.IsFalse(wasCalledForReset); + + wasCalledForAdd = false; + + undoRoot.Redo(); + Assert.IsTrue(wasCalledForRemove); + Assert.IsFalse(wasCalledForAdd); + Assert.IsFalse(wasCalledForReset); + + wasCalledForRemove = false; + + undoRoot.Clear(); + Assert.IsTrue(wasCalledForReset); + Assert.IsFalse(wasCalledForAdd); + Assert.IsFalse(wasCalledForRemove); + } + finally + { + ((INotifyCollectionChanged)undoRoot.RedoStack).CollectionChanged -= callback; + } + } + [TestMethod] public void UndoRoot_Can_Undo_the_Last_ChangeSet() { From 86716beab1700700f84efa0b49eaf4c9a7142eb8 Mon Sep 17 00:00:00 2001 From: danjoconnell Date: Thu, 19 Nov 2020 12:53:53 +1300 Subject: [PATCH 2/2] Changed sample projects, removing the unnecessary manual refresh of the undo / redo stack collections, since they now implement INotifyCollectionChanged. --- samples/WpfUndoSample/MainWindow.xaml.cs | 29 ------------------- .../MainWindowViewModel.cs | 28 ------------------ 2 files changed, 57 deletions(-) diff --git a/samples/WpfUndoSample/MainWindow.xaml.cs b/samples/WpfUndoSample/MainWindow.xaml.cs index 33a43f6..3b0e6cf 100644 --- a/samples/WpfUndoSample/MainWindow.xaml.cs +++ b/samples/WpfUndoSample/MainWindow.xaml.cs @@ -42,26 +42,9 @@ public MainWindow() private void LayoutRoot_Loaded(object sender, RoutedEventArgs e) { - // The undo / redo stack collections are not "Observable", so we - // need to manually refresh the UI when they change. - var root = UndoService.Current[this]; - root.UndoStackChanged += new EventHandler(OnUndoStackChanged); - root.RedoStackChanged += new EventHandler(OnRedoStackChanged); FirstNameTextbox.Focus(); } - // Refresh the UI when the undo stack changes. - void OnUndoStackChanged(object sender, EventArgs e) - { - RefreshUndoStackList(); - } - - // Refresh the UI when the redo stack changes. - void OnRedoStackChanged(object sender, EventArgs e) - { - RefreshUndoStackList(); - } - // The following 4 event handlers support the "CommandBindings" in the window. // These hook to the Undo and Redo commands. @@ -246,18 +229,6 @@ public IEnumerable RedoStack - // Refresh the UI when the undo / redo stacks change. - private void RefreshUndoStackList() - { - // Calling refresh on the CollectionView will tell the UI to rebind the list. - // If the list were an ObservableCollection, or implemented INotifyCollectionChanged, this would not be needed. - var cv = CollectionViewSource.GetDefaultView(UndoStack); - cv.Refresh(); - - cv = CollectionViewSource.GetDefaultView(RedoStack); - cv.Refresh(); - } - diff --git a/samples/WpfUndoSampleMVVM.Core/MainWindowViewModel.cs b/samples/WpfUndoSampleMVVM.Core/MainWindowViewModel.cs index 0cf5c94..692ded6 100644 --- a/samples/WpfUndoSampleMVVM.Core/MainWindowViewModel.cs +++ b/samples/WpfUndoSampleMVVM.Core/MainWindowViewModel.cs @@ -82,25 +82,9 @@ private void OnSliderMouseDown(MouseButtonEventArgs e) private void OnWindowLoaded() { - // The undo / redo stack collections are not "Observable", so we - // need to manually refresh the UI when they change. - var root = UndoService.Current[this]; - root.UndoStackChanged += new EventHandler(OnUndoStackChanged); - root.RedoStackChanged += new EventHandler(OnRedoStackChanged); //FirstNameTextbox.Focus(); } - // Refresh the UI when the undo stack changes. - void OnUndoStackChanged(object sender, EventArgs e) - { - RefreshUndoStackList(); - } - - // Refresh the UI when the redo stack changes. - void OnRedoStackChanged(object sender, EventArgs e) - { - RefreshUndoStackList(); - } @@ -218,18 +202,6 @@ public IEnumerable RedoStack - // Refresh the UI when the undo / redo stacks change. - private void RefreshUndoStackList() - { - // Calling refresh on the CollectionView will tell the UI to rebind the list. - // If the list were an ObservableCollection, or implemented INotifyCollectionChanged, this would not be needed. - var cv = CollectionViewSource.GetDefaultView(UndoStack); - cv.Refresh(); - - cv = CollectionViewSource.GetDefaultView(RedoStack); - cv.Refresh(); - } - private void InitialiseCommandBindings()