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 TagInput bug when TagsProperty changed. #247

Merged
merged 2 commits into from
May 20, 2024

Conversation

dameng324
Copy link
Contributor

When TagInput's Tags changed, you want remove all the tags except the textBox from Items collection, right? But the code does not do that.

old code:

 for (int i = 0; i < Items.Count - 1; i++)
 {
     Items.RemoveAt(Items.Count - 1);
 }

Items.Count will change after every RemoveAt so If the Items.Count is 6, It only remove 3 times, then i will be 3,Items.Count will be 3, the loop will end.

So, I fix this problem.
new code:

// remove all tags except the textbox
int count = Items.Count;
for (int i = 0; i < count - 1; i++)
{
    Items.RemoveAt(0);
}

I create a repro branch(https://github.com/dameng324/Ursa.Avalonia/tree/taginput-fix-tags-changed-repo) to repro this problem. Feel free to check it.

@rabbitism
Copy link
Member

I would suggest to count down i from Count-1( not verified) because RemoveAt(0) has very bad performance.

@dameng324
Copy link
Contributor Author

There is always a textBox at the end of Items. RemoveAt(Items.Count-2) always move that element.

If you concern abount the performance, clear and add the textBox will be nicer ?

@rabbitism
Copy link
Member

Once upon a time I tried but if I remembered correctly it triggered some visual parent re-attach issue, But you can try, if it works well I accept.

@dameng324
Copy link
Contributor Author

The clear and add method works well, but I've found an even better approach. By default, Items is an AvaloniaList, which includes the RemoveRange method. This method is advantageous because it triggers the CollectionChanged event only once.

However, since the Items property is public and users may modify it, we'll need to implement a fallback mechanism. If Items is not an AvaloniaList, it will revert to the clear and add logic.

Both has been tested on my local machine.

@rabbitism rabbitism self-requested a review May 20, 2024 08:39
@rabbitism rabbitism merged commit 68c55dd into irihitech:main May 20, 2024
@dameng324 dameng324 deleted the taginput-fix-tags-changed branch May 20, 2024 23:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants