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

Screen redraw problems / UI Collapse #11441

Closed
mschipperheyn opened this issue Dec 13, 2016 · 17 comments
Closed

Screen redraw problems / UI Collapse #11441

mschipperheyn opened this issue Dec 13, 2016 · 17 comments
Labels
Resolution: Locked This issue was locked by the bot.

Comments

@mschipperheyn
Copy link

I'm on 0.39.2 (upgraded from 0.35) and since the upgrade I'm having screen redraw issues. It primarily seems to happen in lists with complex variable height elements. I use SGListView but have also verified this with the regular ListView.

The problem seems to manifest itself most but not always when the number of items in the list changes. It leads to elements looking garbled with pieces floating around in the wrong places and some heights collapsing to zero. I also sometimes see parts of the screen starting to flash constantly, seemingly trying to redraw. When this happens, there doesn't seem to be any other activity (such as actions being executed).

screen shot 2016-12-13 at 14 24 02

The upgrade I did of course, covers the change in the way flex box behaves (flex : 1 => flexGrow : 1) and I have dealt with a fair number of issues in this area. So I'm inclined to think it might be related to this. On the other hand, on first load, the items usually look normal (but not always). If a redraw of the screen is forced, for instance by doing a hot reload by saving a template, the screen seems to return to normal.

Some yellow box items and warnings I see are:
View #119 of type RCTView has a shadow set but cannot calculate shadow efficiently. Consider setting a background color to fix this, or apply the shadow to a more specific component.

Is there a way to track down the actual template based on this View id?

Warning: In next release empty section headers will be rendered. In this release you can use 'enableEmptySections' flag to render empty section headers.

@mschipperheyn
Copy link
Author

mschipperheyn commented Dec 13, 2016

I just went back to 0.38.1 and the problem seemed no longer reproducible (BTW, testing on iOS simulator). Just to be on the safe side, upgrade to 0.39.1, cleaned the cache and the problem is back again. Then downgraded to 0.38.1 and the problem was visible again.

Almost seems like under certain conditions, flex boxes collapse.

screen shot 2016-12-13 at 15 38 21

@mschipperheyn mschipperheyn changed the title Screen redraw problems Screen redraw problems / UI Collapse Dec 13, 2016
@oureta
Copy link

oureta commented Dec 14, 2016

I encounter this problem on 0.39 but not on 0.38.1. In my case, seems that TouchableOpacity without fixed height causes the problem. But I haven't invested much time to investigate.

@goodhyun
Copy link

We are also having this issue with SwipeableListView on 0.39.*
It redraws almost 10 times right after the first load, and even lost some items in it.

@carlosdelsol
Copy link

Same happens to me. When scrolling down a ListView all elements collapse. Only with 0.39.*

@mschipperheyn
Copy link
Author

Glad I'm not alone! Any workarounds?

@mschipperheyn
Copy link
Author

Made a screen gif from 0.38.1 where the problem occurs but seems to recover at the end of tha animation (somewhat, it reoccurs on scrolling). On 0.39.2, it doesn't recover. The poll on top executes a layout animation which seems to trigger the collapse.

rn0 38 1

@rneiss
Copy link

rneiss commented Dec 14, 2016

Seeing what I think is a similar bug -- also on 0.39.0 -- not present in 0.38.0.

Content all initially loads properly -- in this case, clicking on the "more" button (which merely calls a function to change the state to show previously hidden content) causes the previously loaded content to disappear/collapse:

img_2322
screen shot 2016-12-05 at 5 04 57 pm

@mschipperheyn
Copy link
Author

mschipperheyn commented Dec 14, 2016

I have this problem from 0.38.1 onwards but not on 0.38. As far as I can tell, this may be the commit that is creating this impact: 9a13def

mschipperheyn referenced this issue Dec 14, 2016
Summary:
This PR fixes #11096.

I don't know enough the ReactAndroid's source code so I don't know if this is correct but I hope it is.

In a recent commit (d4b8ae7), the `dispatchUpdates` method now returns a boolean to dispatch or not the `onLayout` event. This works well but if the content is unchanged, the line `nativeViewHierarchyOptimizer.handleUpdateLayout(this);` is never called. I don't know if it was intended but it was this which introduces my issue. I called this again even if the content didn't change. This was the behaviour before 0.38 so I guess I didn't break anything.

**Test plan (required)**

I tested my pretty big app with this fix and every screen is ok.
Closes #11222

Differential Revision: D4252101

Pulled By: astreet

fbshipit-source-id: 551559234631ac37245a854d81ba568f0ddb02dd
@goodhyun
Copy link

Aren't we having the same symptom on iOS ? The change in that commit only refers to .java ?

@sachinrekhi
Copy link
Contributor

sachinrekhi commented Dec 16, 2016

I'm seeing this frequent initial redraw issue as well with SwipeableListView only after upgrading React Native to 0.39.2 in the iOS Simulator. The rendering issue appears to be related to the rendering of the quick actions within the SwipeableListView in my case.

Any workarounds?

@mschipperheyn
Copy link
Author

Yeah, I have only checked and experienced on ios but the change between 0.38.0 and 0.38.1, which seems the point of failure, I identified as this one. I might be wrong

@alexcurtis
Copy link

Can also confirm I am getting a flickering redraw with SwipeableListView on iOS simulator. Occurs on 0.39.2

@stinju
Copy link

stinju commented Dec 16, 2016

If it helps, I resolved my instance of this issue by removing an inappropriate flex: 1 in my row sub-container. Most likely related to https://github.com/facebook/react-native/wiki/Breaking-Changes#039.

@sachinrekhi
Copy link
Contributor

sachinrekhi commented Dec 17, 2016

Thanks @stinju! I think you're spot on. I initially removed a bunch of unnecessary flex: 1 styles in the view hierarchy for my renderRow() implementation assuming it would resolve the issue as mentioned in the breaking changes release notes, but I still had the flickering issue.

However, I noticed that the implementation of SwipeableRow in SwipeableListView creates an outer View component outside your own view hierarchy for renderRow and that had it's own flex: 1 style property set. Once I additionally removed that, the flickering issue appears to disappear.

Sounds like then the fix if you are experiencing the issue specifically with SwipeableListView requires modifying SwipeableRow to remove flex: 1 as well as any unnecessary flex: 1 in your own renderRow view hierarchy. @fred2028 is this something you might be able to help get fixed in SwipeableRow? Happy to contribute a PR to just remove the one line flex: 1, though would likely need support to get it through.

@sachinrekhi
Copy link
Contributor

sachinrekhi commented Dec 17, 2016

Just submitted a PR to resolve this issue specifically with folks using SwipeableListView. But main issue is definitely related to breaking changes introduced and documented in 0.39 here: https://github.com/facebook/react-native/wiki/Breaking-Changes#039.

For others seeing the issue but not using SwipeableListView, as @stinju suggested, you need to remove all the unnecessary flex: 1 as suggested in the breaking changes documentation.

@mschipperheyn
Copy link
Author

mschipperheyn commented Dec 19, 2016

Given that all all containers are flexbox by default, I wonder what is the use of flex : 1 at this point? Would it make sense to replace all flex : 1 with flexGrow : 1? The Yoga documentation doesn't even mention the flex property: https://facebook.github.io/yoga/docs/learn-more/

The RN documentation states that:
If flex = 1 it will be flexible and be sized proportional to its flex value.

The Yoga documentation states:
The FlexGrow property describes how any space within a container should be distributed among its children along the main axis.

Those seem near identical and since in RN all boxes are flex by default, I don't get it anymore. I have made a separate issue on documentation for this confusion (which might me just my own, but I imagine not): #11565

facebook-github-bot pushed a commit that referenced this issue Feb 9, 2017
… changes

Summary:
React Native 0.39 introduced a breaking change through a new C-based implementation of css-layout. Developers were encouraged to remove unnecessary flex: 1 styles where no longer required to address any resulting layout issues.

The SwipeableListView has not yet been updated to reflect this change. Specifically, SwipeableRow still sets flex: 1 even though it shouldn't.

This was resulting in a rendering issue for folks when they upgraded to React Native 0.39 and used SwipeableListView that caused significant initial flickering when rendering a SwipeableListView. The solution was simply to remove flex: 1 from the SwipeableRow implementation.

This small change removes flex: 1 to resolve this issue.

Suggested reviewer (initial SwipeableListView contributor): fred2028

See this issue for more details: #11441
Closes #11521

Differential Revision: D4532562

Pulled By: lacker

fbshipit-source-id: 5c2907186d00481a731bd81794947a019465a031
GaborWnuk pushed a commit to GaborWnuk/react-native that referenced this issue Feb 28, 2017
… changes

Summary:
React Native 0.39 introduced a breaking change through a new C-based implementation of css-layout. Developers were encouraged to remove unnecessary flex: 1 styles where no longer required to address any resulting layout issues.

The SwipeableListView has not yet been updated to reflect this change. Specifically, SwipeableRow still sets flex: 1 even though it shouldn't.

This was resulting in a rendering issue for folks when they upgraded to React Native 0.39 and used SwipeableListView that caused significant initial flickering when rendering a SwipeableListView. The solution was simply to remove flex: 1 from the SwipeableRow implementation.

This small change removes flex: 1 to resolve this issue.

Suggested reviewer (initial SwipeableListView contributor): fred2028

See this issue for more details: facebook#11441
Closes facebook#11521

Differential Revision: D4532562

Pulled By: lacker

fbshipit-source-id: 5c2907186d00481a731bd81794947a019465a031
dudeinthemirror pushed a commit to dudeinthemirror/react-native that referenced this issue Mar 1, 2017
… changes

Summary:
React Native 0.39 introduced a breaking change through a new C-based implementation of css-layout. Developers were encouraged to remove unnecessary flex: 1 styles where no longer required to address any resulting layout issues.

The SwipeableListView has not yet been updated to reflect this change. Specifically, SwipeableRow still sets flex: 1 even though it shouldn't.

This was resulting in a rendering issue for folks when they upgraded to React Native 0.39 and used SwipeableListView that caused significant initial flickering when rendering a SwipeableListView. The solution was simply to remove flex: 1 from the SwipeableRow implementation.

This small change removes flex: 1 to resolve this issue.

Suggested reviewer (initial SwipeableListView contributor): fred2028

See this issue for more details: facebook#11441
Closes facebook#11521

Differential Revision: D4532562

Pulled By: lacker

fbshipit-source-id: 5c2907186d00481a731bd81794947a019465a031
dudeinthemirror pushed a commit to dudeinthemirror/react-native that referenced this issue Mar 1, 2017
… changes

Summary:
React Native 0.39 introduced a breaking change through a new C-based implementation of css-layout. Developers were encouraged to remove unnecessary flex: 1 styles where no longer required to address any resulting layout issues.

The SwipeableListView has not yet been updated to reflect this change. Specifically, SwipeableRow still sets flex: 1 even though it shouldn't.

This was resulting in a rendering issue for folks when they upgraded to React Native 0.39 and used SwipeableListView that caused significant initial flickering when rendering a SwipeableListView. The solution was simply to remove flex: 1 from the SwipeableRow implementation.

This small change removes flex: 1 to resolve this issue.

Suggested reviewer (initial SwipeableListView contributor): fred2028

See this issue for more details: facebook#11441
Closes facebook#11521

Differential Revision: D4532562

Pulled By: lacker

fbshipit-source-id: 5c2907186d00481a731bd81794947a019465a031
maarf pushed a commit to fullcontact/react-native that referenced this issue Apr 26, 2017
… changes

Summary:
React Native 0.39 introduced a breaking change through a new C-based implementation of css-layout. Developers were encouraged to remove unnecessary flex: 1 styles where no longer required to address any resulting layout issues.

The SwipeableListView has not yet been updated to reflect this change. Specifically, SwipeableRow still sets flex: 1 even though it shouldn't.

This was resulting in a rendering issue for folks when they upgraded to React Native 0.39 and used SwipeableListView that caused significant initial flickering when rendering a SwipeableListView. The solution was simply to remove flex: 1 from the SwipeableRow implementation.

This small change removes flex: 1 to resolve this issue.

Suggested reviewer (initial SwipeableListView contributor): fred2028

See this issue for more details: facebook#11441
Closes facebook#11521

Differential Revision: D4532562

Pulled By: lacker

fbshipit-source-id: 5c2907186d00481a731bd81794947a019465a031
@hramos
Copy link
Contributor

hramos commented May 25, 2017

Closing this issue because it has been inactive for a while. If you think it should still be opened let us know why.

@hramos hramos closed this as completed May 25, 2017
@hramos hramos added the Icebox label May 26, 2017
@facebook facebook locked as resolved and limited conversation to collaborators Jul 19, 2018
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jul 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests

10 participants