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

Added an internal ObservableStack collection implementing INotifyCollectionChanged for UndoRoot's Undo and Redo stacks #12

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

danjoconnell
Copy link

Hi Nathan,

From the comments in the sample projects and my own experience binding UndoRoot’s UndoStack and RedoStack to ItemsSource properties in WPF, it would be a lot easier if these Stacks implemented INotifyCollectionChanged.

I found sample code for an ObservableStack collection posted on Stack Overflow which seems to fit the bill and have added this class to the MonitoredUndo project with an attributation comment at the beginning of the file linking back to the post on Stack Overflow.

I replaced Stack collection instances with ObservableStack instances in UndoRoot and added unit test methods to confirm that UndoRoot.UndoStack and UndoRoot.RedoStack implement INotifyCollectionChanged and raise CollectionChanged events correctly.

Finally, I removed the unnecessary manual refresh of the undo / redo stack collections in the sample projects, since the
Stacks now implement INotifyCollectionChanged.

Thank you for your time and consideration.

Regards,

Daniel O’Connell

…ectionChanged 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.
…he undo / redo stack collections, since they now implement INotifyCollectionChanged.
@nathanaw
Copy link
Owner

Thanks Daniel!
I'll take a look this week.

@nathanaw
Copy link
Owner

This seems like a useful enhancement.

I need to spend a bit more time testing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants