-
-
Notifications
You must be signed in to change notification settings - Fork 526
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
fix: webview in stack #607
Conversation
Co-authored-by: Krisztiaan <[email protected]>
Hah, idk how it goes in this repo, but lgtm! :) |
looks good |
@WoLewicki can we merge this? |
We will see after some discussion if there are no better options. For now, you can use patch-package and apply those changes in order to test if everything works right. |
@@ -151,7 +152,22 @@ public void setTransitioning(boolean transitioning) { | |||
return; | |||
} | |||
mTransitioning = transitioning; | |||
super.setLayerType(transitioning ? View.LAYER_TYPE_HARDWARE : View.LAYER_TYPE_NONE, null); | |||
boolean isWebViewInScreen = hasWebView(this); | |||
super.setLayerType(transitioning && !isWebViewInScreen ? View.LAYER_TYPE_HARDWARE : View.LAYER_TYPE_NONE, null); |
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 line isn't consistent with the PR description. You say that we don't change the layer type in case there is a webview, but it appears that we change it to NONE. Should we avoid updating layer type altogether as stated in the description?
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.
We could not change the layer type, but if it is already HARDWARE
for some reason, it will probably lead to crash. I can add a check for if the layer is not HARDWARE
and, if so, just do nothing when there is a WebView
.
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.
Rubber stamping this PR, please just address the inline comment I left prior to merging.
can we merge this? |
Added check for if there is WebView on the screen we are transitioning to. If there is, we don't change the layer type in stack navigator to avoid crash seen in #105.