Fix Bug #2382 DelayChangeNotifications does not delay PropertyChanged events #2403
Conversation
Improve the test of deferring change notifications on reactive objects to test that "Changing" notifications are delayed in addition to "Changed" notifications. Also improved to verify which properties change notifications were sent.
…opertyChanged events DelayChangeNotifications() delayed notifications through the Changing and Changed observables, but had no effect on the PropertyChanging and PropertyChanged events. This fixes that.
|
|
| IObservable<Exception> ThrownExceptions { get; } | ||
|
|
||
| /// <summary> | ||
| /// sdfsdg. |
There was a problem hiding this comment.
We probably want a real comment here
There was a problem hiding this comment.
I absolutely agree. That was an oversight on my part. Sorry about that. I'll fix this one and the other one below.
| "StyleCop.CSharp.NamingRules", | ||
| "SA1300:Element should begin with upper-case letter", | ||
| Justification = "Event is private backing field for PropertyChanging")] | ||
| private event PropertyChangingEventHandler _propertyChanging; |
There was a problem hiding this comment.
Probably best to stick to the naming scheme regardless if it's private.
|
I'll have a full review over the next hour or so. Thanks for submitting the pr |
|
Ok, tested your changes they look good. One you fix up those comments/style issues we are good to go :) |
|
All the code review comments have been addressed. |
|
I'll push out a new version later today. Thanks again. |
|
Great, thank you for being very responsive and helpful. |
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
3 participants
What kind of change does this PR introduce?
Fixes #2382 DelayChangeNotifications does not delay PropertyChanged events
What is the current behavior?
Calling
DelayChangeNotifications()on aReactiveObjectdelays theChangingandChangedobservable notifications, but not the actualPropertyChangingandPropertyChangedevents.What is the new behavior?
DelayChangeNotifications()now delaysPropertyChangingandPropertyChangedevents the same as the observables. Duplicate events are also removed the same as with the observables.What might this PR break?
Code relying on the current behavior of
PropertyChangingandPropertyChangedevents not being delayed.While I don't think this will break stuff, the most likely places for other bugs to be introduced:
PropertyChangingandPropertyChangedeventsChangingandChangedobservablesPropertyChangingandPropertyChangedeventsPlease check if the PR fulfills these requirements
DelayChangeNotifications()was already not very specific, so didn't need to change.Other Information:
The approach used here was discussed in issue #2382. See specifically my comment and those follow.