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

Possible RN 0.61 Android view regressions #26574

Closed
lafiosca opened this issue Sep 25, 2019 · 29 comments
Closed

Possible RN 0.61 Android view regressions #26574

lafiosca opened this issue Sep 25, 2019 · 29 comments
Labels
Bug Impact: Regression Describes a behavior that used to work on a prior release, but stopped working recently. Platform: Android Android applications. Resolution: Locked This issue was locked by the bot.

Comments

@lafiosca
Copy link

lafiosca commented Sep 25, 2019

After upgrading from 0.60.5 to 0.61.1, I'm seeing some funky visual behaviors with Android which might be directly related to #26289, but I don't know enough about them to say for sure. For example, look at the difference in the buttons from react-native-paper@2.16.0 in the first screenshot below. Another example is my swipe deck component based on Animated.View. In Android now the swipe cards which have solid white backgrounds are somehow merging visually (second screenshot below). That is to say, all text and icon elements of both the top card and the underlying card are visible simultaneously.

Note: all screenshots are from a Samsung Galaxy S7 Edge. These behaviors did NOT occur for me in the Android emulator.

React Native version:
System:
OS: macOS Mojave 10.14.6
CPU: (4) x64 Intel(R) Core(TM) i5-4278U CPU @ 2.60GHz
Memory: 19.70 MB / 8.00 GB
Shell: 3.2.57 - /bin/bash
Binaries:
Node: 8.11.4 - ~/.nvm/versions/node/v8.11.4/bin/node
Yarn: 1.17.3 - /usr/local/bin/yarn
npm: 6.10.3 - ~/.nvm/versions/node/v8.11.4/bin/npm
Watchman: 4.9.0 - /usr/local/bin/watchman
SDKs:
iOS SDK:
Platforms: iOS 13.0, DriverKit 19.0, macOS 10.15, tvOS 13.0, watchOS 6.0
Android SDK:
API Levels: 28, 29
Build Tools: 28.0.3, 29.0.2
System Images: android-28 | Intel x86 Atom_64, android-28 | Google APIs Intel x86 Atom, android-28 | Google Play Intel x86 Atom
IDEs:
Android Studio: 3.4 AI-183.6156.11.34.5692245
Xcode: 11.0/11A420a - /usr/bin/xcodebuild
npmPackages:
react: 16.9.0 => 16.9.0
react-native: 0.61.1 => 0.61.1
npmGlobalPackages:
react-native-cli: 2.0.1
react-native: 0.61.0

Steps To Reproduce

  1. Connect an Android device (Does it have to be a Samsung Galaxy S7 Edge? I don't know)
  2. react-native init oldbutton --version 0.60.6; cd oldbutton
  3. yarn add react-native-vector-icons react-native-paper
  4. Edit App.js to add a button somewhere with this code:
import { Button } from 'react-native-paper';
<Button dark color="#aaaaff" mode="contained" onPress={() => {}}>Test Button</Button>
  1. react-native run-android and observe the button's visual behavior before/while touching
  2. cd ..; react-native init newbutton; cd newbutton
  3. Repeat steps 3-5

Describe what you expected to happen:

I expected the views not to change so significantly between versions. See the third screenshot below. I used a lighter color on the button so you can see that even unpressed, the button has changed visually (odd dark outline), but the difference is more significant while pressing.

Snack, code example, screenshot, or link to a repository:

image

image

image

Note: all screenshots are from a Samsung Galaxy S7 Edge. These behaviors did NOT occur for me in the Android emulator.

@mikebouwmans
Copy link

Seems to be related to #26544

@react-native-bot react-native-bot added Impact: Regression Describes a behavior that used to work on a prior release, but stopped working recently. Platform: Android Android applications. labels Sep 25, 2019
@mjmasn
Copy link
Contributor

mjmasn commented Sep 25, 2019

I think #26544 is a duplicate of #26289 (or at least strongly related), and this would appear to be too. The only one I'm not sure about is the card as it doesn't look like that has any elevation shadow to begin with.

@lafiosca is it fixed by adding TouchableNativeFeedback.SelectableBackgroundBorderless = () => null; to index.js?

@lafiosca
Copy link
Author

@mikebouwmans Yes, at least the button examples above look likely related to that, to me. Apologies if my issue is redundant.

Although I wonder my other visual problem with the swipe cards becoming transparent is directly related. If it's about elevation, perhaps so? Those cards were using zIndex and did not use the elevation style property directly. I don't know that I have the time right now to try to boil down a minimal example unfortunately.

@lafiosca
Copy link
Author

@mjmasn It seems that workaround does not fix the buttons or cards in my app, nor the button in the reproduction steps app. 😔

@darknblack
Copy link

@mjmasn It seems that workaround does not fix the buttons or cards in my app, nor the button in the reproduction steps app. 😔

Have you tried removing the border-radius?

@elicwhite
Copy link
Member

It would be helpful if you are able to get a repro that doesn't require react-native-paper

@mjmasn
Copy link
Contributor

mjmasn commented Sep 25, 2019

@lafiosca how about TouchableNativeFeedback.Ripple = () => null;

@lafiosca
Copy link
Author

@TheSavior I am currently trying to work up a minimal example of the overlapping views problem. I'll update here when I have something.

@lafiosca
Copy link
Author

Something making this a lot more difficult and time-consuming: I'm seeing inconsistent rendering results on the Android device after doing a manual shake&reload (even with Fast Refresh disabled). That is to say: I'll have a piece of code that seems to be rendering wrong, but then if I kill the app and run react-native run-android again, it becomes fine.

To phrase it another way: working from a non-minimal bad sample, pruning out bits of code and doing a manual reload may show the same bad behavior even though killing and restarting does not. This makes it confusing and tough to track down the minimal sample quickly.

@lafiosca
Copy link
Author

After some extensive testing to try to reproduce the overlapping swipe card views problem, I have made some strange findings. As I mentioned in my previous comment, "breaking" the view breaks it for all subsequent reloads, unless I kill the app and restart from scratch. I determined that the thing that "breaks" the view is by having the react-native-paper button render on the screen, even though the button is not involved in the card view!!

The code is here: https://gist.github.com/lafiosca/dda9096adbc9ab3078c83d1d3bff112d

Creating a new 0.61.1 app and copying those files into it, when I run on the Android device (Galaxy S7 Edge) I see the overlapping text on the cards:

Screenshot_20190925-154031

If I comment out line 23 of App.js (the Button) and kill/restart (NOT just reload) the app, the cards look fine:

Screenshot_20190925-154130

Alternatively, following @mjmasn's suggestion and uncommenting line 6 of index.js to replace TouchableNativeFeedback.Ripple seems to clear up the issue (although it means the button no longer has ripple feedback):

Screenshot_20190925-154250

I've tried the Ripple workaround in my original app, and it seems to clear up the initial problems I noticed there as well. This may be a solid enough workaround for me to proceed using 0.61.1.

I am curious to know what in react-native-paper could be causing this problem that impacts other views, and I would love to understand why that also permanently messes up the views even through reloads of "good" code. But I do not have the time this afternoon to delve any deeper into this.

@lafiosca
Copy link
Author

lafiosca commented Sep 25, 2019

callstack/react-native-paper#1341 also seems possibly related, but it raises a question in my mind: when there's a change like this, who is responsible for addressing it?

@elicwhite
Copy link
Member

It depends what the problem is. If react-native core had a breaking change to behavior that we didn't expect to have this result then it should probably be fixed in core because it is more widespread than just one library. If it is caused by something fishy being done in react-native-paper, then I'd expect it to be fixed in react-native-paper.

If you can get a repro that doesn't require paper, or able to track down a commit in the core that caused this, that would help it get addressed in core.

@greis
Copy link
Contributor

greis commented Sep 26, 2019

It could be happening only with specific versions of android. In my case it renders fine on API 28 but on API 23 it renders with this issue.

@lafiosca it looks like your emulator is running API 28. Try to boot another emulator running API 23

@lafiosca
Copy link
Author

Excellent call, @greis. Running it on an API 23 emulator reproduced the issue. Thanks.

@kdawgwilk
Copy link

I am also seeing these shadow/zIndex issues on emulator running API 27

@grabbou
Copy link
Contributor

grabbou commented Sep 26, 2019

I wonder if this issue isn't a duplicate of #26544, according to the screenshots.

Would be awesome if OP could confirm.

Also, @ferranp and @satya164, do you think this could be related to any potential business logic that lives inside Paper?

@lafiosca
Copy link
Author

I think it’s very likely that all of the issues referenced above are duplicates. I find it bizarre that even just rendering the component from react-native-paper also seems to “contaminate” my other views, but I am guessing that part is not the fault of react-native. I don’t have the time and knowledge to confirm for certain, but if you’d like to close this as a duplicate I’d understand.

@ahce
Copy link

ahce commented Sep 27, 2019

Animated component also broken.
I think this should be fixed asap.

@gdoudeng
Copy link

I am facing this problem and hope to solve it as soon as possible.

@mjmasn
Copy link
Contributor

mjmasn commented Sep 27, 2019

@grabbou it's definitely a react-native issue linked to TouchableNativeFeedback.SelectableBackgroundBorderless() and TouchableNativeFeedback.Ripple() with borderless param set to true.

We don't use react-native-paper.

I'll upload my repro repos in a sec.

@mjmasn
Copy link
Contributor

mjmasn commented Sep 27, 2019

Reproduction:
https://github.com/mjmasn/ElevationIssue60 (working)
https://github.com/mjmasn/ElevationIssue61 (broken)

@iamacup
Copy link

iamacup commented Sep 27, 2019

I am also seeing very strange visual artifacts on certain components on Android after 0.61.x

I am also experiencing massive levels of inconsistency between reloads and / or fast refreshes as @lafiosca identifies above.

All of my probems only exist when there is a border radius applied to something - two examples:

Previously working border radius on a view over an icon

{
  backgroundColor: 'white',
  borderColor: primary,
  borderWidth: 1, 
}

image

{
  backgroundColor: 'white',
  borderColor: primary,
  borderWidth: 1, 
  borderRadius: 0,
}

image

Previously working border radius on a set of navigation tabs

{
  backgroundColor: white,
  borderTopLeftRadius: 15,
  borderTopRightRadius: 15,
}

image

{
  backgroundColor: white,
}

image

I will try and pull this behavior out into an example repo over the weekend.

@tianjyan
Copy link

tianjyan commented Sep 29, 2019

@lafiosca I think I have the same issue with your second screenshot. The backgroundColor can not be rendered correctly. See below:

On React Native 0.61
Screenshot_2019-09-29-09-19-33-312_app cybrook teamlink

On React Native 0.60
Screenshot_2019-09-29-09-19-58-649_app cybrook teamlink

I tried to remove the Animated.View but backgroundColor is still not working. I am trying to give a example to reproduce it.

Update:
I remove the borderRadius on the Component that I have set backgroundColor then I get the below screenshot:
Screenshot_2019-09-29-09-55-07-465_app cybrook teamlink

@jeremy-deutsch
Copy link

jeremy-deutsch commented Sep 30, 2019

Just want to note that this issue isn't entirely new to RN 0.61 - I don't have a repro immediately on hand, but I've noticed in previous versions of RN that if an element has both borderRadius and backgroundColor, and is very, very large, the backgroundColor disappears.

EDIT: here's a repro (try it on Android, and it takes at least 11 or 12 clicks): https://snack.expo.io/HyHS1Tkur

The repro case above should have problems at least as far back as 0.56, if I remember right

@mjmasn
Copy link
Contributor

mjmasn commented Sep 30, 2019

@jeremy-deutsch that sounds like #15826

No idea if it's actually related to these new issues or just a similar symptom.

@jeremy-deutsch
Copy link

@jeremy-deutsch that sounds like #15826

No idea if it's actually related to these new issues or just a similar symptom.

Ah yeah, didn't see that issue somehow. The connection might still be worth looking into, though.

@jstheoriginal
Copy link
Contributor

FWIW my app on latest version of 0.60 didn’t have these issues at all.

@tianjyan
Copy link

In my case, the height is not so large, so I think maybe we have different issues here. @mjmasn

@radko93
Copy link
Contributor

radko93 commented Oct 1, 2019

Duplicate of #26544. Please continue discussion there. If you believe it is not please mention me there.
@grabbou @empyrical

@radko93 radko93 closed this as completed Oct 1, 2019
@facebook facebook locked and limited conversation to collaborators Oct 1, 2019
@facebook facebook unlocked this conversation Oct 1, 2019
@facebook facebook locked and limited conversation to collaborators Oct 1, 2019
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Oct 3, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug Impact: Regression Describes a behavior that used to work on a prior release, but stopped working recently. Platform: Android Android applications. Resolution: Locked This issue was locked by the bot.
Projects
None yet
Development

No branches or pull requests