I get this error after update RN. The showLightBox is not shown
I have somewhat the same issue, one the 2 errors come when dismissing a LightBox on Android.
@diennguyentien @Bhullnatik
node_modules/react-native-navigation/android/app/src/main/java/com/reactnativenavigation/layouts/:
class SingleScreenLayout & BottomTabsLayout
rewrite function :
@Override
public void dismissLightBox() {
if (lightBox != null) {
lightBox.dismiss();
// lightBox.hide();
lightBox = null;
}
}
@lipingruan thanks you
Does @lipingruan workaround work for you guys?
@lipingruan's workaround works.
That will only work if you want to dismiss the lightbox from your screen, meaning by calling this.props.navigator.dismissLightBox()
from your component.
However, if you wanna also close the lightbox by tapping in the background (common use case) using tapBackgroundToDismiss
it will still break, i.e:
this.props.navigator.showLightBox({
screen: screens.lightBox.sort,
style: {
backgroundBlur: 'dark',
backgroundColor: 'rgba(0, 0, 0, 0.7)',
tapBackgroundToDismiss: true,
},
});
@lipingruan's workaround also removes entirely the animation on dismissal. My workaround works for all cases and keeps the animation, see https://github.com/rauliyohmc/react-native-navigation/commit/c15bde4899c9779d7be47670a7ed3f988c4b38af.
I am concerned if that would imply memory leaks though. @guyca do you have any thoughts on that? It'd likely break previous RN versions as well.
@rauliyohmc
hello, I found bug is 'tag', and solved in new workaround, but it need you custom build RN.
file1: node_modules/react-native/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java :
rewrite function:
private void detachViewFromInstance(
ReactRootView rootView,
CatalystInstance catalystInstance) {
UiThreadUtil.assertOnUiThread();
// int tag = rootView.getId();
int tag = rootView.getRootViewTag();
catalystInstance.getJSModule(AppRegistry.class)
.unmountApplicationComponentAtRootTag(tag);
}
file2: node_modules/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java :
rewrite function:
protected synchronized void dropView(View view) {
UiThreadUtil.assertOnUiThread();
int tag;
if ( view instanceof ReactRootView ) {
tag = ((ReactRootView) view).getRootViewTag();
} else {
tag = view.getId();
}
if (!mRootTags.get( tag )) {
// For non-root views we notify viewmanager with {@link ViewManager#onDropInstance}
resolveViewManager( tag ).onDropViewInstance(view);
}
ViewManager viewManager = mTagsToViewManagers.get( tag );
if (view instanceof ViewGroup && viewManager instanceof ViewGroupManager) {
ViewGroup viewGroup = (ViewGroup) view;
ViewGroupManager viewGroupManager = (ViewGroupManager) viewManager;
for (int i = viewGroupManager.getChildCount(viewGroup) - 1; i >= 0; i--) {
View child = viewGroupManager.getChildAt(viewGroup, i);
if (mTagsToViews.get(child.getId()) != null) {
dropView(child);
}
}
viewGroupManager.removeAllViews(viewGroup);
}
mTagsToViews.remove( tag );
mTagsToViewManagers.remove( tag );
}
it's ok :)
detail message in :
file: node_modules/react-native/Libraries/Renderer/ReactNativeStack-dev.js: 2040
function: unmountComponentAtNode & renderComponent
Hey guys
Can someone upload an example project where this issue reproduces? I understand it doesn't happen in RN 0.45
@guyca Just change the version of react-native to 0.46.4, then run the example, this error will show when hiding light box.
Any update @guyca? I'm using many lightboxes on my app mostly for loading popup indicator
[using V2] not too sure how much help it is but i did some debugging and commenting out unmountReactApplication()
in ReactContainerView.java
stopped the error for me but it doesnt seem like that is a/the solution. @guyca
I get this error in any way i want to close lighbox
I get error when using : tapBackgroundToDismiss: true,
or this.props.navigator.dismissLightBox()
even when rewrite method :
@Override
public void dismissLightBox() {
if (lightBox != null) {
lightBox.dismiss();
// lightBox.hide();
lightBox = null;
}
}
my react native version : "react-native": "0.47.2"
please someone help to solve it
This hacky by @rawrmaan fixed for me
change the destroy
method on node_modules/react-native-navigation/android/app/src/main/java/com/reactnativenavigation/views/LightBox.java
public void destroy() {
// content.unmountReactView();
dismiss();
}
tank you man
It's trow error in some devices like lenovo tab3
... please fixed this issues i needed for my app
I encounter this with light box when using Android's back button. Also using tapBackgroundToDismiss: true
I'm having the same issue
Also getting this on android, is this being looked at ? Basically can't use lightboxes at all at the moment.
As epic as this library is, it is making my app pretty much iOS only instead of cross platform. Please !!!!! fix this issue.
@hemedani Thanks, your workaround works quite nice. But within my lightbox I'm dependent on the componentWillUnmount
method beeing called. With your workaround this method is never called for my lightbox. So currently I have a really ugly hack for this:
if (Platform.OS === 'android') {
setTimeout(() => {
this.componentWillUnmount()
}, 800)
}
beyond two month for fixing a little and irritating bug ...
please fixed.
+1 Please fix this issue. I can't use lightbox in my project.
I'm having exactly same issue. tapBackgroundToDismiss: true
isn't working.
any update @guyca on this issue?
I am literally checking this every day just in case I did not get the notification for some reason.
Any updated?
This is a really annoying bug I also have, but guys, we're asking Wix to do something nice 😄
@drpiou Yeah, it's tough. One one hand, this is a regression clearly introduced under the oversight of the project maintainers. On the other hand, it's clear as day that the Wix team and especially @guyca are oversubscribed at the moment. Look at how many open issues there are! This library clearly grew much faster than the company's ability to put a fully functioning support system behind it.
It's also clear that the 2.x rewrite is making everything 1.x a lesser priority, which makes sense if it will be done soon, but its development keeps getting dragged out more and more. It's completely understandable, too, because this is a huge library with a ton of functionality and it does a lot of things really well. It's clear that the Wix team wants 2.x to be that golden egg that can be maintained by the community due to its full e2e test suite.
I guess what I would ask of the maintainers is to just talk to us, be honest about how much they're struggling to keep up with the library, and work with the community to try to delegate some of the development and support so they're not so burdened. Personally I'd love to go through and clean up the issues in this repo. Let us know how we can help, Wix team!
@rawrmaan Hi ! Many thanks for your answer. Yep, I totally understand what you're saying and I'm also very okay with all of this. The only thing that bother me is that this library, at this day, is not fully functional on Android, thus avoiding many developers to use this plugin. I discovered today that a v2 is on its way browsing the releases, which I hope will correct everything :) My point is, this is NOT a simple small bug or wish that can be included on the v2, but a critical bug that makes every Android developers stuck in this. I now have to make my own lightbox component and just simply skip yours, which is not really fashioned. I cannot use your modal component too because I cannot change the background to transparent, so I have to use an another plugin. I thank you very much for your work guys, and I'm happy to triggered you to finally discuss this. Please, provide a simple fix that will cover all of the sides effect of the workarounds (not all working anymore today). We understand your work, but we're really stuck... Kindest regards
@drpiou Can you share with us your lightbox components?
iOS lightbox is working fine for me with 1.228 and RN 0.48.3.
On Android see if you can substitute with a modal using fade
animation and a 0.9 background opacity.
what is the update on this issue? actually, this is a very annoying issue, I am using code push to generate build so hacky fix will not work for me and unfortunately I am using lightbox so many places in my application. Please release a new build for this fix asap.
Because lightbox is the only roadblock to migrate to v2. Does anyone know native modal works with wix-native-navigation
@guyca This is an important issue as it prevents developers from using react-native-navigation
and harms the image of the library, in my opinion. I understand that V2 is an important milestone but keeping the original version usable is very important for your current community of developers. I don't want to nag as I like this library and the work that's going on in V2 but it now prevents us to use this library in our app. Is there a way someone with more experience in this library could take a look at it?
In the meantime, does anyone know what the problem exactly is? I don't mean quick dirty fixes but the origin of the whole problem? I am happy to help or get started on this.
tldr; I also had the exact problem with RN 0.49.5 and the work around to ignore unmounting react view/application scares me a bit as the more LightBox we show, the more memory leak we create. So I tried to read the code and understand it, and I see that RN blames the contentView of LightBox.java is not the root view, which is make sense to me, as LightBox is a Dialog and have "loosely connection" to the NavigationActivity which is the root view. So I think why don't I register the contentView as the root view?
Here is my solution: (in LightBox.java)
private void createContent(final Context context, LightBoxParams params) {
lightBox = new RelativeLayout(context);
lightBox.setAlpha(0);
content = new ContentView(context, params.screenId, params.navigationParams);
content.setAlpha(0);
// --- Start of the hack ---
// content.setId(ViewUtils.generateViewId());
int rootViewTag = ViewUtils.generateViewId();
content.setRootViewTag(View.NO_ID);
content.setId(View.NO_ID);
ReactContext reactContext = NavigationApplication.instance.getReactGateway().getReactInstanceManager().getCurrentReactContext();
ThemedReactContext themedReactContext = new ThemedReactContext((ReactApplicationContext) reactContext, content.getContext());
reactContext.getNativeModule(UIManagerModule.class).getUIImplementation().registerRootView(
content, rootViewTag, 0, 0, themedReactContext
);
content.setId(rootViewTag);
// --- End of the hack ---
RelativeLayout.LayoutParams lp = new RelativeLayout.LayoutParams(WRAP_CONTENT, WRAP_CONTENT);
...
I've tested on the emulator and it works. But honestly I have no idea about the consequence of doing this, so please consider carefully before using it. I hope that some experts can have a look on this and tell us if this is ok.
@hnvcam It seems like for me this hack causes problems with pushing screens after dismissing a lightbox.
@fliPmitKlammern yeah, there is a racing problem between the hiding animation and the native Dialog destroying.
To make it last longer, we'll need to make another fix to skip the hiding animation (SingleScreenLayout.java)
@Override
public void dismissLightBox() {
if (lightBox != null) {
// lightBox.hide();
lightBox.dismiss();
lightBox = null;
}
}
However, as my self verification, all this workaround is not stable. Sometime, RN still claims for missing view tag. So I think you should not go with this solution. Sorry for proposing this and wasting your time.
I've migrated all my lightbox needs to use my own version which employed the RN Modal component and deliver the same result without animation effects.
I end up with custom lightbox which base on React native modal
Any update ? @pqkluan ? @guyca ?
There is no way I am going to use this lib for any future project. All the cool features that lured me into this are all buggy. For some reason this keeps getting closed even though there is no fix
@ODelibalta There is no need to get emotional. This issue is still open and nobody closed it. You are probably talking about referenced issues which got closed because they are duplicates of this one.
The maintainers are doing this for free and they don't get any profit from whiny people. Like with any other open source project, feel free to fix it yourself and actually give the authors something back for their work.
If some of you, like me, are tired of updating the file every time, you can use this postinstall script to apply the workaround:
let lightbox = './node_modules/react-native-navigation/android/app/src/main/java/com/reactnativenavigation/views/LightBox.java';
fs.readFile(lightbox, 'utf8', function(err, data)
{
if (err)
{
} else {
if (data.split('\n')[108].indexOf('public void destroy()') > -1 && data.split('\n')[109].indexOf('//') == -1 && data.split('\n')[109].indexOf('content.unmountReactView();') > -1){
let aux = data.split('\n');
aux[109] = '//'+aux[109];
fs.writeFile(lightbox, aux.join('\n'));
}
}
});
I am willing to debug this as soon as I have some time, because I believe with this we are causing a memory leak.
I confirm the current workaround is causing a memory leak.
The root container that uses to display the Lightbox is not unmounting properly. Every time you start a new Lightbox, a new container was created and keep in the root nodes list causing the leak. You could see this clearly by using this debugger feature
Okay guys. I am not an Android guy but I downloaded Android Studio and dug around in RNN as well as RN code and I think I have figured this out. Problem is that when LightBox
creates its ContentView
which is a ReactRootView
, RN sets a viewId and keeps its reference with it for later.
But the this line
content.setId(ViewUtils.generateViewId());
in
private void createContent(final Context context, LightBoxParams params)
creates a new viewId (a.f.a.i.k its a random integer) and assigns it again, overwriting the previous viewId RN saved to refer to it later. So when we dismiss the LightBox RN tries to deallocate but doesn't find the reference it assigned to the view and then complains that it isn't a rootView.
Commenting out said line fixes the problem for me. But I don't know about you guys. I don't even know why they needed to set an id in the first place. Maybe in RN < 0.44 ReactRootView
didn't assign one? or didn't keep track of it for later deallocation. Whatever the reason was, the fix is to not set id again.
I encourage you all to try my fix and if it really fixes the bug(I am skeptical) I will submit a PR and hope @guyca comes around to merge it.
Well, 0.45.1 launched around June, which is at the same time this commit was issued. I think this was a problem before, but RN was not complaining and cleaning the screen anyway. So, right now is just throwing the error. Can't confirm tho.
https://github.com/facebook/react-native/commit/54f7ae1c02b37773b9db2c22e03e6abc9ba8958e#diff-a04970ddd4a6df34e2245d37f78486e3
@hhunaid's solution seems to be working for me without any other issues. Will update if I notice anything strange.
@hhunaid's solution seems also to work for me but sometimes lightbox content is not displayed (there is only the lightbox background that can be touched to dismiss the lightbox, but no React component).
Same issue. It was happening about 1 every 10 clicks in my debug build but as soon as I made the release build, it is happening like 2 every 3 clicks. Almost renders the Lightbox not usuable, but I am not sure if this was a problem before.
@jay1337 @sfratini What's your RN version?
I never had this problem in dev. Didn't check in release But I am willing to try.
I am using RN 0.49.5 and I experience no issues with @hhunaid's solution, neither in debug nor in release mode.
@hhunaid :
[email protected]
[email protected]
[email protected] (patched with the fix)
Emulator Genymotion Samsung Galaxy S6 - 5.0.0 - API 21 - 1440x2560
build.gradle :
compileSdkVersion 25
buildToolsVersion "25.0.1"
I guess it has something to do with that 0.50 patch then. Can you confirm that undoing my patch fixes the view not showing up issue?
This is my dependencies:
@jay1337 Do you see any of your dependencies in my list? Maybe is another module messing around.
"dependencies": {
"clevertap-react-native": "0.1.8",
"d3-array": "1.2.1",
"d3-scale": "1.0.6",
"d3-shape": "1.2.0",
"lottie-react-native": "2.2.7",
"prop-types": "15.6.0",
"react": "16.0.0-alpha.12",
"react-native": "0.48.1",
"react-native-action-button": "2.8.0",
"react-native-app-intro": "1.1.5",
"react-native-collapsible": "0.9.0",
"react-native-config": "0.8.1",
"react-native-firebase": "3.0.5",
"react-native-google-analytics-bridge": "5.3.3",
"react-native-localization": "0.1.32",
"react-native-maps": "0.17.0",
"react-native-navigation": "1.1.210",
"react-native-open-settings": "1.0.1",
"react-native-permissions": "1.0.0",
"react-native-sentry": "0.30.0",
"react-native-storage": "0.2.2",
"react-native-swiper": "1.5.13",
"react-native-vector-icons": "4.3.0"
},
"devDependencies": {
"babel-jest": "21.0.0",
"babel-preset-react-native": "3.0.2",
"jest": "21.0.1",
"react-test-renderer": "16.0.0-alpha.12"
},
If I remove the patch from @hhunaid I sometimes get this and crashes my app:
12-05 09:21:17.655 2688 2725 E AndroidRuntime: com.facebook.react.uimanager.IllegalViewOperationException: View with tag 4 is not registered as a root view
But if I use the first workaround, and remove the line that unmounts the component, I still get the dimmed background and no lightbox sometimes. I am thinking the fact that the Lightbox starts a new root view is not helping.
My patch has nothing to do with this problem then. I thought it might have been a problem with 0.50 infinite layout rendering patch but you are sitting at RN 0.48. So its definitely something else.
@hhunaid I've made some tests and the bug I describe (content not displayed) was already there before applying your fix.
I've opened a specific issue for that : https://github.com/wix/react-native-navigation/issues/2288
@sfratini I can reproduce the bug with a simple project, RNN being the only installed module.
@sfratini This tool is more convenient to make a local patch: https://github.com/ds300/patch-package
Seems to be broken with react-native: 0.50.3 still. However @ywongweb 's suggestion to use background opacity works nicely. Granted, the Navigator.OS handling is a pain, but showModal in IOS with "screenBackgroundColor: transparent" doesn't seem to work.
So I opted to:
if ( Navigator.OS === "ios" ) {
Navigation.showLightBox({
screen: "app.ContextMenu",
passProps: {
...stuff
}
});
} else {
Navigation.showModal({
screen: "app.ContextMenu",
passProps: {
...stuff
},
animationType: "fade",
navigatorStyle: {
screenBackgroundColor: "transparent",
navBarHidden: true
}
});
}
and then displaying the modal with a
Setting the dismissModal to display "fade" animation makes the whole thing behave smoothly:
if (Platform.OS === "ios") {
Navigation.dismissLightBox();
} else {
Navigation.dismissModal({
animationType: "fade"
});
}
Too unstable for me, so I switched to regular RN modals.
Thanks @guyca for fixing this 🎉🎉🎉
Thanks @guyca !
Could you also have a look at this issue ? https://github.com/wix/react-native-navigation/issues/2288
Most helpful comment
Thanks @guyca for fixing this 🎉🎉🎉