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

Fix BindToObservableList: Previous values not being removed & sorting resets being ignored. #401

Merged

Conversation

ryanholden8
Copy link
Contributor

What kind of change does this PR introduce?
This is two bug fixes for the BindToObservableList extension method.
Improves unit tests to cover these situations.

What is the current behavior?

  1. Currently if you make a change to a model in an observable list that causes the updated model to move to a new index, the previous model is not removed from its old index.

  2. When using .Sort and the reset threshold is hit, the observable list does not update it's data set because it did not receive a list of changes.

What is the new behavior?

  1. The old model at the old index is now removed as expected.
  2. SortReason.Reset is now detected to clear and re-apply using the SortedItems property.

What might this PR break?
This should only fix.

Please check if the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

Other information:

Copy link
Collaborator

@RolandPheasant RolandPheasant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes perfect sense to me. and good to see loads of unit tests

@RolandPheasant RolandPheasant merged commit fb578dd into reactivemarbles:main Sep 3, 2020
@github-actions
Copy link

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.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants