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

Make BindingList adaptors replace the changed item instead of re-adding #694

Merged

Conversation

Metadorius
Copy link
Contributor

Before it was deleting and adding the item again, resulting in two notifications for deleting and adding an item. This resulted in UI deselecting the updated item if it was selected, which is an undesired behavior. After this change it doesn't do that anymore.

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.

Technically if the item is not found (ie index -1) an exception should be thrown. However I have simply ignored those cases in binding. But food for thought for future consideration.

@codecov-commenter
Copy link

Codecov Report

Merging #694 (e860184) into main (82c9639) will decrease coverage by 0.05%.
The diff coverage is 76.47%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main     #694      +/-   ##
==========================================
- Coverage   71.90%   71.86%   -0.05%     
==========================================
  Files         216      216              
  Lines       10871    10876       +5     
==========================================
- Hits         7817     7816       -1     
- Misses       3054     3060       +6     
Impacted Files Coverage Δ
...rc/DynamicData/Binding/SortedBindingListAdaptor.cs 65.11% <70.00%> (-5.94%) ⬇️
src/DynamicData/Binding/BindingListAdaptor.cs 88.09% <85.71%> (-4.77%) ⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@RolandPheasant RolandPheasant merged commit d1ca572 into reactivemarbles:main Apr 7, 2023
@Metadorius Metadorius deleted the fix/BindingListUpdate branch April 7, 2023 23:32
@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 Apr 22, 2023
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.

3 participants