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

Fixed views not being removed from the item container. #339

Merged
merged 4 commits into from
Jul 21, 2016

Conversation

NegreaVlad
Copy link

In order to remove all children you have to remove child at index 0 or
have indexes go in a decreasing order.

After removing child 0, all children go an index down.
If you have 3 views to remove, you remove position 0, then position 2,
when you get to position 3, this child will not be deleted, because
there no longer is a child at index 3. The only child is at position 0.

I was having trouble changing my BottomBarTabs. A view was not removed because of this.

Negrea Vlad added 2 commits July 7, 2016 16:19
Removing views works like a cascade delete.
If you have 3 views to remove, you remove position 0, then position 2,
when you get to position 3, you only have one child at 0. This child
will not be deleted.
You can remove by index in decreasing order or just remove at index 0.
@ygnessin
Copy link

Maybe I'm missing something, but why can't removeAllViews() be used instead of iterating over each child one by one?

@NegreaVlad
Copy link
Author

@ygnessin You are right removeAllViews() is probably the better approach. I did not want to change very much code, because I'm not very familiar with the project, that's why I did not use that.

@roughike
Copy link
Owner

roughike commented Jul 18, 2016

@NegreaVlad
Although this thing is a spaghetti code hell, removeAllViews() would be right for this and it doesn't affect negatively elsewhere. If you want credit for this, just update this PR with removeAllViews() instead of the loop. Also a nice catch.

@NegreaVlad
Copy link
Author

@roughike
I have updated the pull request. Nice work on the library. Keep up the good work!

@roughike roughike merged commit ce41d9a into roughike:master Jul 21, 2016
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.

3 participants