-
Notifications
You must be signed in to change notification settings - Fork 24.5k
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
Removing items with LayoutAnimation causes adjacent views/texts to disappear on Android #11828
Comments
CauseWhen LayoutAnimation is enabled, indices of to-be-removed items are incorrect (to reproduce it, we need to delete at least two items). So the adjacent items are removed. In uimanager/NativeViewHierarchyManager.java#L364-L373 because Essential CauseThe essential cause is in uimanager/NativeViewHierarchyOptimizer.java#L278-L285. It gets the index of a Says we want to delete 2 items When LayoutAnimation is not enabled, But when LayoutAnimation is enabled, the problem happens in deleting SolutionSkip deleting items by index when LayoutAnimation is enabled. These items will be deleted by correct It's hard to correct index at uimanager/NativeViewHierarchyOptimizer.java#L278-L285, because we don't know if we are using LayoutAnimation. NoteActually it tries to skip deleting in the existing code uimanager/NativeViewHierarchyManager.java#L364-L373. But because the index is wrong,
QuestionWhy do we need to pass in both index and tag in uimanager/NativeViewHierarchyOptimizer.java#L278-L285? I tried simply using |
Hi there! This issue is being closed because it has been inactive for a while. Maybe the issue has been fixed in a recent release, or perhaps it is not affecting a lot of people. Either way, we're automatically closing issues after a period of inactivity. Please do not take it personally! If you think this issue should definitely remain open, please let us know. The following information is helpful when it comes to determining if the issue should be re-opened:
If you would like to work on a patch to fix the issue, contributions are very welcome! Read through the contribution guide, and feel free to hop into #react-native if you need help planning your contribution. |
Fixes issue facebook#11828 that causes layout animations for removed views to remove some adjacent views as well. This happens because the animated views are still present in the ViewGroup, which throws off subsequent operations that rely on view indices having updated. This issue was addressed in facebook#11962, which was closed in favour of a more reliable solution that addresses the issue globally since it’s difficult to account for animated views everywhere. @janicduplessis recommended[0] handling the issue through ViewManager. Since API 11, Android provides `ViewGroup#startViewTransition(View)` that can be used to keep child views visible even if they have been removed from the group. ViewGroup keeps an array of these views, which is only used for drawing. Methods such as `ViewGroup#getChildCount()` and `ViewGroup#getChildAt(int)` will ignore them. I believe relying on these framework methods within ViewManager is the most reliable way to solve this issue because it also works if callers ignore ViewManager and reach into the native view indices and counts directly. [0]: facebook#11962 (review)
Summary: Fixes issue #11828 that causes layout animations for removed views to remove some adjacent views as well. This happens because the animated views are still present in the ViewGroup, which throws off subsequent operations that rely on view indices having updated. This issue was addressed in #11962, which was closed in favour of a more reliable solution that addresses the issue globally since it’s difficult to account for animated views everywhere. janicduplessis [recommended][0] handling the issue through ViewManager. Since API 11, Android provides `ViewGroup#startViewTransition(View)` that can be used to keep child views visible even if they have been removed from the group. ViewGroup keeps an array of these views, which is only used for drawing. Methods such as `ViewGroup#getChildCount()` and `ViewGroup#getChildAt(int)` will ignore them. I believe relying on these framework methods within ViewManager is the most reliable way to solve this issue because it also works if callers ignore ViewManager and reach into the native view indices and counts directly. [0]: #11962 (review) I wrote a minimal test app that you can find here: <https://gist.github.com/lnikkila/87f3825442a5773f17ead433a810d53f> The expected result is that the red and green squares disappear, a blue one appears, and the black one stays in place. iOS has this behaviour, but Android removes the black square as well. We can see the bug with some breakpoint logging. Without LayoutAnimation: ``` NativeViewHierarchyOptimizer: Removing node from parent with tag 2 at index 0 NativeViewHierarchyOptimizer: Removing node from parent with tag 4 at index 1 NativeViewHierarchyManager: Removing indices [0] with tags [2] RootViewManager: Removing child view at index 0 with tag 2 NativeViewHierarchyManager: Removing indices [1] with tags [4] RootViewManager: Removing child view at index 1 with tag 4 ``` With LayoutAnimation tag 3 gets removed when it shouldn’t be: ``` NativeViewHierarchyOptimizer: Removing node from parent with tag 2 at index 0 NativeViewHierarchyOptimizer: Removing node from parent with tag 4 at index 1 NativeViewHierarchyManager: Removing indices [0] with tags [2] NativeViewHierarchyManager: Removing indices [1] with tags [4] -> RootViewManager: Removing child view at index 1 with tag 3 RootViewManager: Removing child view at index 2 with tag 4 (Animation listener kicks in here) RootViewManager: Removing child view at index 1 with tag 2 ``` Here are some GIFs to compare, click to expand: <details> <summary><b>Current master (iOS vs Android)</b></summary> <p></p> <img src="https://user-images.githubusercontent.com/1291143/38695083-fbc29cd4-3e93-11e8-9150-9b8ea75b87aa.gif" height="400" /><img src="https://user-images.githubusercontent.com/1291143/38695108-06eb73a6-3e94-11e8-867a-b95d7f926ccd.gif" height="400" /> </details><p></p> <details> <summary><b>With this patch (iOS vs Android, fixed)</b></summary> <p></p> <img src="https://user-images.githubusercontent.com/1291143/38695083-fbc29cd4-3e93-11e8-9150-9b8ea75b87aa.gif" height="400" /><img src="https://user-images.githubusercontent.com/1291143/38695137-1090f782-3e94-11e8-94c8-ce33a5d7ebdb.gif" height="400" /> </details><p></p> Previously addressed in #11962, which wasn’t merged. Tangentially related to my other LayoutAnimation PR #18651. No documentation changes needed. [ANDROID] [BUGFIX] [LayoutAnimation] - Removal LayoutAnimations no longer remove adjacent views as well in certain cases. Closes #18830 Reviewed By: achen1 Differential Revision: D7612904 Pulled By: mdvacca fbshipit-source-id: a04cf47ab80e0e813fa043125b1f907e212b1ad4
Summary: Fixes issue facebook#11828 that causes layout animations for removed views to remove some adjacent views as well. This happens because the animated views are still present in the ViewGroup, which throws off subsequent operations that rely on view indices having updated. This issue was addressed in facebook#11962, which was closed in favour of a more reliable solution that addresses the issue globally since it’s difficult to account for animated views everywhere. janicduplessis [recommended][0] handling the issue through ViewManager. Since API 11, Android provides `ViewGroup#startViewTransition(View)` that can be used to keep child views visible even if they have been removed from the group. ViewGroup keeps an array of these views, which is only used for drawing. Methods such as `ViewGroup#getChildCount()` and `ViewGroup#getChildAt(int)` will ignore them. I believe relying on these framework methods within ViewManager is the most reliable way to solve this issue because it also works if callers ignore ViewManager and reach into the native view indices and counts directly. [0]: facebook#11962 (review) I wrote a minimal test app that you can find here: <https://gist.github.com/lnikkila/87f3825442a5773f17ead433a810d53f> The expected result is that the red and green squares disappear, a blue one appears, and the black one stays in place. iOS has this behaviour, but Android removes the black square as well. We can see the bug with some breakpoint logging. Without LayoutAnimation: ``` NativeViewHierarchyOptimizer: Removing node from parent with tag 2 at index 0 NativeViewHierarchyOptimizer: Removing node from parent with tag 4 at index 1 NativeViewHierarchyManager: Removing indices [0] with tags [2] RootViewManager: Removing child view at index 0 with tag 2 NativeViewHierarchyManager: Removing indices [1] with tags [4] RootViewManager: Removing child view at index 1 with tag 4 ``` With LayoutAnimation tag 3 gets removed when it shouldn’t be: ``` NativeViewHierarchyOptimizer: Removing node from parent with tag 2 at index 0 NativeViewHierarchyOptimizer: Removing node from parent with tag 4 at index 1 NativeViewHierarchyManager: Removing indices [0] with tags [2] NativeViewHierarchyManager: Removing indices [1] with tags [4] -> RootViewManager: Removing child view at index 1 with tag 3 RootViewManager: Removing child view at index 2 with tag 4 (Animation listener kicks in here) RootViewManager: Removing child view at index 1 with tag 2 ``` Here are some GIFs to compare, click to expand: <details> <summary><b>Current master (iOS vs Android)</b></summary> <p></p> <img src="https://user-images.githubusercontent.com/1291143/38695083-fbc29cd4-3e93-11e8-9150-9b8ea75b87aa.gif" height="400" /><img src="https://user-images.githubusercontent.com/1291143/38695108-06eb73a6-3e94-11e8-867a-b95d7f926ccd.gif" height="400" /> </details><p></p> <details> <summary><b>With this patch (iOS vs Android, fixed)</b></summary> <p></p> <img src="https://user-images.githubusercontent.com/1291143/38695083-fbc29cd4-3e93-11e8-9150-9b8ea75b87aa.gif" height="400" /><img src="https://user-images.githubusercontent.com/1291143/38695137-1090f782-3e94-11e8-94c8-ce33a5d7ebdb.gif" height="400" /> </details><p></p> Previously addressed in facebook#11962, which wasn’t merged. Tangentially related to my other LayoutAnimation PR facebook#18651. No documentation changes needed. [ANDROID] [BUGFIX] [LayoutAnimation] - Removal LayoutAnimations no longer remove adjacent views as well in certain cases. Closes facebook#18830 Reviewed By: achen1 Differential Revision: D7612904 Pulled By: mdvacca fbshipit-source-id: a04cf47ab80e0e813fa043125b1f907e212b1ad4
Summary: Fixes issue facebook#11828 that causes layout animations for removed views to remove some adjacent views as well. This happens because the animated views are still present in the ViewGroup, which throws off subsequent operations that rely on view indices having updated. This issue was addressed in facebook#11962, which was closed in favour of a more reliable solution that addresses the issue globally since it’s difficult to account for animated views everywhere. janicduplessis [recommended][0] handling the issue through ViewManager. Since API 11, Android provides `ViewGroup#startViewTransition(View)` that can be used to keep child views visible even if they have been removed from the group. ViewGroup keeps an array of these views, which is only used for drawing. Methods such as `ViewGroup#getChildCount()` and `ViewGroup#getChildAt(int)` will ignore them. I believe relying on these framework methods within ViewManager is the most reliable way to solve this issue because it also works if callers ignore ViewManager and reach into the native view indices and counts directly. [0]: facebook#11962 (review) I wrote a minimal test app that you can find here: <https://gist.github.com/lnikkila/87f3825442a5773f17ead433a810d53f> The expected result is that the red and green squares disappear, a blue one appears, and the black one stays in place. iOS has this behaviour, but Android removes the black square as well. We can see the bug with some breakpoint logging. Without LayoutAnimation: ``` NativeViewHierarchyOptimizer: Removing node from parent with tag 2 at index 0 NativeViewHierarchyOptimizer: Removing node from parent with tag 4 at index 1 NativeViewHierarchyManager: Removing indices [0] with tags [2] RootViewManager: Removing child view at index 0 with tag 2 NativeViewHierarchyManager: Removing indices [1] with tags [4] RootViewManager: Removing child view at index 1 with tag 4 ``` With LayoutAnimation tag 3 gets removed when it shouldn’t be: ``` NativeViewHierarchyOptimizer: Removing node from parent with tag 2 at index 0 NativeViewHierarchyOptimizer: Removing node from parent with tag 4 at index 1 NativeViewHierarchyManager: Removing indices [0] with tags [2] NativeViewHierarchyManager: Removing indices [1] with tags [4] -> RootViewManager: Removing child view at index 1 with tag 3 RootViewManager: Removing child view at index 2 with tag 4 (Animation listener kicks in here) RootViewManager: Removing child view at index 1 with tag 2 ``` Here are some GIFs to compare, click to expand: <details> <summary><b>Current master (iOS vs Android)</b></summary> <p></p> <img src="https://user-images.githubusercontent.com/1291143/38695083-fbc29cd4-3e93-11e8-9150-9b8ea75b87aa.gif" height="400" /><img src="https://user-images.githubusercontent.com/1291143/38695108-06eb73a6-3e94-11e8-867a-b95d7f926ccd.gif" height="400" /> </details><p></p> <details> <summary><b>With this patch (iOS vs Android, fixed)</b></summary> <p></p> <img src="https://user-images.githubusercontent.com/1291143/38695083-fbc29cd4-3e93-11e8-9150-9b8ea75b87aa.gif" height="400" /><img src="https://user-images.githubusercontent.com/1291143/38695137-1090f782-3e94-11e8-94c8-ce33a5d7ebdb.gif" height="400" /> </details><p></p> Previously addressed in facebook#11962, which wasn’t merged. Tangentially related to my other LayoutAnimation PR facebook#18651. No documentation changes needed. [ANDROID] [BUGFIX] [LayoutAnimation] - Removal LayoutAnimations no longer remove adjacent views as well in certain cases. Closes facebook#18830 Reviewed By: achen1 Differential Revision: D7612904 Pulled By: mdvacca fbshipit-source-id: a04cf47ab80e0e813fa043125b1f907e212b1ad4
Summary: Fixes issue facebook#11828 that causes layout animations for removed views to remove some adjacent views as well. This happens because the animated views are still present in the ViewGroup, which throws off subsequent operations that rely on view indices having updated. This issue was addressed in facebook#11962, which was closed in favour of a more reliable solution that addresses the issue globally since it’s difficult to account for animated views everywhere. janicduplessis [recommended][0] handling the issue through ViewManager. Since API 11, Android provides `ViewGroup#startViewTransition(View)` that can be used to keep child views visible even if they have been removed from the group. ViewGroup keeps an array of these views, which is only used for drawing. Methods such as `ViewGroup#getChildCount()` and `ViewGroup#getChildAt(int)` will ignore them. I believe relying on these framework methods within ViewManager is the most reliable way to solve this issue because it also works if callers ignore ViewManager and reach into the native view indices and counts directly. [0]: facebook#11962 (review) I wrote a minimal test app that you can find here: <https://gist.github.com/lnikkila/87f3825442a5773f17ead433a810d53f> The expected result is that the red and green squares disappear, a blue one appears, and the black one stays in place. iOS has this behaviour, but Android removes the black square as well. We can see the bug with some breakpoint logging. Without LayoutAnimation: ``` NativeViewHierarchyOptimizer: Removing node from parent with tag 2 at index 0 NativeViewHierarchyOptimizer: Removing node from parent with tag 4 at index 1 NativeViewHierarchyManager: Removing indices [0] with tags [2] RootViewManager: Removing child view at index 0 with tag 2 NativeViewHierarchyManager: Removing indices [1] with tags [4] RootViewManager: Removing child view at index 1 with tag 4 ``` With LayoutAnimation tag 3 gets removed when it shouldn’t be: ``` NativeViewHierarchyOptimizer: Removing node from parent with tag 2 at index 0 NativeViewHierarchyOptimizer: Removing node from parent with tag 4 at index 1 NativeViewHierarchyManager: Removing indices [0] with tags [2] NativeViewHierarchyManager: Removing indices [1] with tags [4] -> RootViewManager: Removing child view at index 1 with tag 3 RootViewManager: Removing child view at index 2 with tag 4 (Animation listener kicks in here) RootViewManager: Removing child view at index 1 with tag 2 ``` Here are some GIFs to compare, click to expand: <details> <summary><b>Current master (iOS vs Android)</b></summary> <p></p> <img src="https://user-images.githubusercontent.com/1291143/38695083-fbc29cd4-3e93-11e8-9150-9b8ea75b87aa.gif" height="400" /><img src="https://user-images.githubusercontent.com/1291143/38695108-06eb73a6-3e94-11e8-867a-b95d7f926ccd.gif" height="400" /> </details><p></p> <details> <summary><b>With this patch (iOS vs Android, fixed)</b></summary> <p></p> <img src="https://user-images.githubusercontent.com/1291143/38695083-fbc29cd4-3e93-11e8-9150-9b8ea75b87aa.gif" height="400" /><img src="https://user-images.githubusercontent.com/1291143/38695137-1090f782-3e94-11e8-94c8-ce33a5d7ebdb.gif" height="400" /> </details><p></p> Previously addressed in facebook#11962, which wasn’t merged. Tangentially related to my other LayoutAnimation PR facebook#18651. No documentation changes needed. [ANDROID] [BUGFIX] [LayoutAnimation] - Removal LayoutAnimations no longer remove adjacent views as well in certain cases. Closes facebook#18830 Reviewed By: achen1 Differential Revision: D7612904 Pulled By: mdvacca fbshipit-source-id: a04cf47ab80e0e813fa043125b1f907e212b1ad4
Description
When remove items with LayoutAnimation, the adjacent views/texts should not be impacted. But they disappear on Android.
In the following screenshot of the sample app, we can see some texts/buttons disappeared when we press
Remove with opacity animation
button andRemove with scaleXY animation
button. I found this issue when I worked on #11769.Reproduction
Reproduced on rnplay.org https://rnplay.org/apps/JbdOpQ
The sample project I created: https://github.com/vinceyuan/ReactNativeOpacityBugDemo
Solution
N/A
Update on Jan 18, 2017:
I figured out a solution. See below.
Additional Information
The text was updated successfully, but these errors were encountered: