-
Notifications
You must be signed in to change notification settings - Fork 24.9k
Fix LayoutAnimation Android bug when adding and removing views at the same time #7959
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -352,6 +352,7 @@ public void manageChildren( | |
arrayContains(tagsToDelete, viewToRemove.getId())) { | ||
// The view will be removed and dropped by the 'delete' layout animation | ||
// instead, so do nothing | ||
mLayoutAnimator.addViewAnimatingDeletion(viewToRemove); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should happen automatically within the LayoutAnimation code There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I wanted to do that initially but I need to do this before the code that add views. I could move up the block of code that drop views and start the layout animation before adding views. Now: remove -> add -> drop |
||
} else { | ||
viewManager.removeViewAt(viewToManage, indexToRemove); | ||
} | ||
|
@@ -375,7 +376,20 @@ public void manageChildren( | |
viewsToAdd, | ||
tagsToDelete)); | ||
} | ||
viewManager.addView(viewToManage, viewToAdd, viewAtIndex.mIndex); | ||
|
||
// When performing layout delete animations views are not removed immediately | ||
// from their container so we need to offset the insert index if a view | ||
// that is going to be removed is before the view we want to insert. | ||
int addViewIndex = viewAtIndex.mIndex; | ||
if (mLayoutAnimator.isAnimatingViewDeletion()) { | ||
for(int index = 0; index < viewAtIndex.mIndex; index++) { | ||
if (mLayoutAnimator.isViewAnimatingDeletion(viewManager.getChildAt(viewToManage, index))) { | ||
addViewIndex++; | ||
} | ||
} | ||
} | ||
|
||
viewManager.addView(viewToManage, viewToAdd, addViewIndex); | ||
} | ||
} | ||
|
||
|
@@ -399,6 +413,7 @@ public void manageChildren( | |
mLayoutAnimator.deleteView(viewToDestroy, new LayoutAnimationListener() { | ||
@Override | ||
public void onAnimationEnd() { | ||
mLayoutAnimator.removeViewAnimatingDeletion(viewToDestroy); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same with this |
||
viewManager.removeView(viewToManage, viewToDestroy); | ||
dropView(viewToDestroy); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,6 +2,9 @@ | |
|
||
package com.facebook.react.uimanager.layoutanimation; | ||
|
||
import java.util.ArrayList; | ||
import java.util.List; | ||
|
||
import javax.annotation.Nullable; | ||
import javax.annotation.concurrent.NotThreadSafe; | ||
|
||
|
@@ -29,6 +32,8 @@ public class LayoutAnimationController { | |
private final AbstractLayoutAnimation mLayoutDeleteAnimation = new LayoutDeleteAnimation(); | ||
private boolean mShouldAnimateLayout; | ||
|
||
private List<View> mDeleteAnimatedViews = new ArrayList<>(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. May as well use a bitset and index by tag, right? |
||
|
||
public void initializeFromConfig(final @Nullable ReadableMap config) { | ||
if (!ENABLED) { | ||
return; | ||
|
@@ -138,6 +143,41 @@ public void onAnimationEnd(Animation anim) { | |
} | ||
} | ||
|
||
/** | ||
* Add a view to list of views that are being deleted with an animation. | ||
* | ||
* @param view View to add | ||
*/ | ||
public void addViewAnimatingDeletion(View view) { | ||
mDeleteAnimatedViews.add(view); | ||
} | ||
|
||
/** | ||
* Remove a view to list of views that are being deleted with an animation. | ||
* | ||
* @param view View to remove | ||
*/ | ||
public void removeViewAnimatingDeletion(View view) { | ||
mDeleteAnimatedViews.remove(view); | ||
} | ||
|
||
/** | ||
* Returns if the view is being deleted with an animation. | ||
* | ||
* @param view View to check | ||
* @return If it is being animated | ||
*/ | ||
public boolean isViewAnimatingDeletion(View view) { | ||
return mDeleteAnimatedViews.contains(view); | ||
} | ||
|
||
/** | ||
* Returns if there are any view being deleted with an animation. | ||
*/ | ||
public boolean isAnimatingViewDeletion() { | ||
return mDeleteAnimatedViews.size() > 0; | ||
} | ||
|
||
/** | ||
* Disables user interactions for a view and all it's subviews. | ||
*/ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment probably needs to be updated. Since do nothing now looks like doing something.