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

Modal: Refresh (via dev menu) does not tear down #17986

Closed
pstanton opened this issue Feb 15, 2018 · 77 comments
Closed

Modal: Refresh (via dev menu) does not tear down #17986

pstanton opened this issue Feb 15, 2018 · 77 comments
Labels
Bug DX Issues concerning how the developer experience can be improved. Impact: Regression Describes a behavior that used to work on a prior release, but stopped working recently. Platform: Android Android applications. Platform: Windows Building on Windows.

Comments

@pstanton
Copy link

Is this a bug report?

yes

Have you read the Contributing Guidelines?

kind of

Environment

Environment:
OS: Windows 10
Node: 9.0.0
Yarn: 1.3.2
npm: 5.5.1
Watchman: Not Found
Xcode: N/A
Android Studio: Version 3.0.0.0 AI-171.4408382

Packages: (wanted => installed)
react: 16.2.0 => 16.2.0
react-native: 0.53.0 => 0.53.0

Target Platform: Android 7.0 on Samsung Galaxy S5

Steps to Reproduce

  1. create app and launch with react-native run-android
  2. trigger state which causes a modal to show
  3. launch the dev menu (via shake or keycode)
  4. select refresh

Expected Behavior

The app closes the modal and refreshes

Actual Behavior

The app presumably refreshes, but the modal stays on top and cannot be closed

Reproducible Demo


import React, { Component } from 'react';
import { Text, View, Button, Modal, StyleSheet } from 'react-native';

export default class MyComponent extends Component {
  state = {
    modalVisible: false,
  };

  openModal() {
    this.setState({modalVisible:true});
  }

  closeModal() {
    this.setState({modalVisible:false});
  }

  render() {
    return (
        <View style={styles.container}>
          <Modal
              visible={this.state.modalVisible}
              animationType={'slide'}
              onRequestClose={() => this.closeModal()}
          >
            <View style={styles.modalContainer}>
              <View style={styles.innerContainer}>
                <Text>This is content inside of modal component</Text>
                <Button
                    onPress={() => this.closeModal()}
                    title="Close modal"
                >
                </Button>
              </View>
            </View>
          </Modal>
          <Button
              onPress={() => this.openModal()}
              title="Open modal"
          />
        </View>
    );
  }
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
    justifyContent: 'center',
  },
  modalContainer: {
    flex: 1,
    justifyContent: 'center',
    backgroundColor: 'grey',
  },
  innerContainer: {
    alignItems: 'center',
  },
});
@hramos hramos added the Android label Mar 5, 2018
@allpwrfulroot
Copy link

allpwrfulroot commented Mar 8, 2018

Seeing in iOS as well. Adding a state change to take the modal down in componentWillUnmount does nothing

@pstanton pstanton changed the title Android - Modal: Refresh (via dev menu) does not tear down Modal: Refresh (via dev menu) does not tear down Mar 8, 2018
@react-native-bot react-native-bot added the Platform: Windows Building on Windows. label Mar 20, 2018
@hramos hramos removed the Platform: Windows Building on Windows. label Mar 29, 2018
@scottmas

This comment has been minimized.

@MrLoh
Copy link

MrLoh commented May 3, 2018

Is there any workaround? This also affects programmatic restarting on codepush updates for example.

@hanselsen

This comment has been minimized.

@react-native-bot

This comment has been minimized.

@pstanton

This comment has been minimized.

@ilovett
Copy link

ilovett commented May 21, 2018

This is pretty awful for developing against, but it is still possible to use the modal, so long as you close it before you reload the app.

This means you must have a way of closing the modal from the modal itself (IE: a close button), and you must close it before the app reloads... you should also disable live reload and hot reload.

@micalm

This comment has been minimized.

@jamesone

This comment has been minimized.

@theapache64

This comment has been minimized.

@lefrankleal

This comment has been minimized.

@victorvhpg
Copy link

"react-native": "0.55.4"
this is happening here :(
2018_06_23_17_11_53

@Montaguti

This comment has been minimized.

@MarcelZOI

This comment has been minimized.

@Thyoity

This comment has been minimized.

@PiotrKujawa
Copy link

I guess here's the fix e5c2a66 and will be available in 0.56

@gwidener
Copy link

That fix is also in 0.55.4, and the problem remains (Android emulation at least). It's a blocker for hot/live reloading, therefore for any decent development environment. Can't use this package until it's fixed.

@gwidener
Copy link

The above workaround doesn't work. The only workaround I've found is to auto-close the Modal in the dev build after 7 seconds, otherwise it's too easy to hang it. Once it was hung so bad that I had to cold-boot the emulator. This is a must-fix! Note it affects all Modals based on the RN Modal.

@chadhobson

This comment has been minimized.

@hramos hramos added Impact: Regression Describes a behavior that used to work on a prior release, but stopped working recently. Platform: Windows Building on Windows. Platform: Android Android applications. and removed ⏪Old Version labels Jul 24, 2018
@Normence

This comment has been minimized.

@xXFracXx

This comment has been minimized.

@nishantWM
Copy link

nishantWM commented Jun 10, 2019

@PiotrKujawa Actually the pr you mentioned is not the fix, but the cause!

The root cause is the removal of this line

  public void onHostPause() {	  
-    // We dismiss the dialog and reconstitute it onHostResume
-    dismiss();	
  }

When we do reload, actually we do a pause, so the dismiss() in onHostPause() can be triggered. But since this pr merged into 0.53, there was no dismiss anymore. This bug emerged since then.

@mdvacca Could you recall why you removed this line? I could not see how that was related to Modal not disappearing when navigating to another activity? Do you expect the modal should persist when navigation to another activity?

Adding this line helps. Any ideas why this is required?

@rajatgng
Copy link

please then somwone tell how to fix it

@AmruthPillai
Copy link

Is there no way to fix this? Still an issue :(

@Inovassist-dev
Copy link

Are you guys kidding that, after so many RN versions and month since this issue was open, nobody made a fix?

@xydian
Copy link

xydian commented Aug 2, 2019

@EhsanSarshar its not that easy unfortunately, read the entire thread for details

@malvinder
Copy link

Confirming, still an issue in RN 0.60.4

1 similar comment
@abidulrmdn
Copy link

abidulrmdn commented Sep 25, 2019

Confirming, still an issue in RN 0.60.4

@syntax-e
Copy link

We're running into this issue as well.

@acoutts
Copy link

acoutts commented Oct 15, 2019

I just upgraded to 0.61.2 and the new fast refresh feature doesn't have this issue. Curious if anyone else can confirm as well.

Nevermind it's definitely still an issue even with fast refresh.

@danielkv
Copy link

danielkv commented Nov 5, 2019

No solution for this yet?

@jedashford
Copy link

This is also an issue for those of us using codepush. When the app refreshes the js bundle, the previous state modal still shows and completely locks the app.

@vjsingh
Copy link

vjsingh commented Dec 10, 2019

Very frustrating, even more so if you don't know what's causing it. Appears to happen when I have nested Modals. Closing the first modal without closing the second (nested) modal also makes the simulator freeze.

rickhanlonii pushed a commit that referenced this issue Dec 20, 2019
Summary:
Fixes #17986

See above issue

After apply this change:
![ezgif-4-45d9add85b74](https://user-images.githubusercontent.com/615282/70987576-2520ad00-20fb-11ea-9b90-c9a7839824a5.gif)

## Changelog

[Android] [Fixed] - Fix android modal not disappear when reload
Pull Request resolved: #27542

Test Plan: Open a modal and do a refresh to see whether it disappears

Differential Revision: D19178803

Pulled By: mdvacca

fbshipit-source-id: 61894927fc481650804b2196df06a80c16b64e6c
@i-am-nikola
Copy link

Confirming, still an issue in RN 0.61.5
23/12/2019

@sunnylqm
Copy link
Contributor

@hongdung6992 The fix will land on 0.62-rc1

@finntenzor
Copy link

A compatibility way to fix this problem if you have to use RN 0.60~0.61.(At least work for me)

Extend the ReactModalHostView and override the onHostPause method. Then use the new class instead of origin one. You may have to copy many Java files and js files from source code.

// Fix the ReactModalHostView
public class ReactModalHostFixedView extends ReactModalHostView {
    private Method dismissMethod;

    public ReactModalHostFixedView(Context context) {
        super(context);
    }

    @Override
    public void onHostPause() {
        super.onHostPause();
        Method dismiss = this.getDismissMethod();
        try {
            dismiss.invoke(this);
        } catch (InvocationTargetException e) {
            Log.e("Demo", "Cant invoke dismiss(InvocationTargetException )");
        } catch (IllegalAccessException e) {
            Log.e("Demo", "Cant invoke dismiss(IllegalAccessException )");
        }
    }

    protected Method getDismissMethod() {
        if (dismissMethod == null) {
            try {
                Method dismiss = ReactModalHostView.class.getDeclaredMethod("dismiss");
                dismiss.setAccessible(true);
                dismissMethod = dismiss;
            } catch (NoSuchMethodException e) {
                Log.e("Demo", "No dismiss method");
            }
        }
        return dismissMethod;
    }
}
// Fix the ReactModalHostManager 
public class ReactModalHostFixedManager extends ReactModalHostManager {
    public static final String REACT_CLASS = "RCTModalHostFixedView";

    @Override
    public String getName() {
        return REACT_CLASS;
    }

    @Override
    protected ReactModalHostView createViewInstance(ThemedReactContext reactContext) {
        return new ReactModalHostFixedView(reactContext);
    }
}
// Create you own package and register it in "MainApplication.java"
// See https://facebook.github.io/react-native/docs/native-components-android
public class ReactModalHostFixedPackage implements  ReactPackage {
    @Override
    public List<NativeModule> createNativeModules(ReactApplicationContext reactContext)
    {
        return Collections.emptyList();
    }

    @Override
    public List<ViewManager> createViewManagers(ReactApplicationContext reactContext)
    {
        List<ViewManager> managers = new ArrayList<>();
        managers.add(new ReactModalHostFixedManager());
        return managers;
    }
}
// You should copy the RCTModalHostViewNativeComponent.js to your project
// from node_modules/react-native/Libraries/Modal/RCTModalHostViewNativeComponent.js

// The preceding codes were omitted
// Notice you have to rewrite the import statements

// Use fixed view if the platform is android
const Platform = require('react-native/Utilities/Platform');
const paperComponentName =
  Platform.OS === 'android' ? 'RCTModalHostFixedView' : 'RCTModalHostView';

// export the fixed view
// NOTICE: for RN 0.61.5
export default (codegenNativeComponent<NativeProps>('ModalHostView', {
  interfaceOnly: true,
  paperComponentName,
}): NativeComponentType<NativeProps>);
// You should copy the RCTModalHostViewNativeComponent.js to your project
// from node_modules/react-native/Libraries/Modal/RCTModalHostViewNativeComponent.js

// The preceding codes were omitted
// Notice you have to rewrite the import statements

// Use fixed view if the platform is android
const Platform = require('react-native/Utilities/Platform');
const paperComponentName =
  Platform.OS === 'android' ? 'RCTModalHostFixedView' : 'RCTModalHostView';

// export the fixed view
// NOTICE: for RN 0.60.5
module.exports = ((requireNativeComponent(
  paperComponentName,
): any): RCTModalHostViewNativeType);
// You should copy the Modal.js to your project
// from node_modules/react-native/Libraries/Modal/Modal.js

// Use fixed view above
// The other codes were omitted
import RCTModalHostView from './RCTModalHostViewNativeComponent.js';

It works for me.(Only test on android simulator and my android phone for RN 0.61.5 and RN 0.60.5)

@alzalabany
Copy link

alzalabany commented Feb 21, 2020

same problem still in 61.5, i solved it somehow by adding key prop to modal.

<Modal visible={modalVisible} key="authmodal">
</Modal>

you can use any string in key, this fixs duplicate problem in android both emulator and devices.

note: same problem also exists in release version android, not just during debug mode.

@DibPuelma
Copy link

@alzalabany this does not solves the issue in my case :(

Anyone with another workaround? I am using Code Push and the app is stuck whenever a modal is shown and the code is reloaded

@DibPuelma
Copy link

I found a workaround. Just don't use the modal when the Platform.OS === 'android'. It loses some functionalities like the animation, but you can program does manually if needed. Here is some code I'm using

<TouchableHighlight
style={{
elevation: 5,
zIndex: 10,
}}
disabled={!backdropAllowed}
onPress={backdropAllowed ? onClose : () => {}}>
<SafeAreaView style={{
height: '100%',
display: 'flex',
alignItems: 'center',
justifyContent: 'center',
backgroundColor: 'rgba(0, 0, 0, 0.6)',
}}>
{withSkip &&
<View style={{
width: '100%',
alignItems: 'flex-end',
position: 'relative',
top: 10,
right: 0,
}}>

<Text style={{
fontWeight: 'bold',
color: colors.lightGray,
fontSize: fontSizes.h4
}}>
OMITIR



}
<TouchableWithoutFeedback
disabled={!backdropAllowed} onPress={() => {}} >
<KeyboardAvoidingView behavior="position" style={{width: '90%', marginBottom: 20}}>
{children}



Hope it helps. If anyone needs more help or has any other ideas, just tag me.

@WouterFlorijn
Copy link

@sunnylqm do you know if it would be possible to merge this fix into 0.61 as well? And is there a time frame for a stable 0.62?

@sunnylqm
Copy link
Contributor

@WouterFlorijn For release related questions, https://github.com/react-native-community/releases/issues

@nthchild1
Copy link

It's been almost another whole year and this hasn't been fixed!

@sunnylqm
Copy link
Contributor

sunnylqm commented Jul 1, 2020

@TKY2048 It's fixed in 0.62

@Daniel-Griffiths
Copy link

Daniel-Griffiths commented Sep 9, 2020

For anyone else who is using Expo and is stuck on RN 0.61, this fixed this issue for me:

<Modal ...yourProps>
  <TouchableWithoutFeedback onPressOut={(e) => {
    if (e.nativeEvent.locationY < 0) {
      handleModal(false)
    }}
  >
    <View>
      ...modal content
    </View>
  </TouchableWithoutFeedback>
</Modal>

The TouchableWithoutFeedback fixes the problem.

KusStar pushed a commit to KusStar/react-native that referenced this issue May 23, 2021
Summary:
Fixes facebook#17986

See above issue

After apply this change:
![ezgif-4-45d9add85b74](https://user-images.githubusercontent.com/615282/70987576-2520ad00-20fb-11ea-9b90-c9a7839824a5.gif)

## Changelog

[Android] [Fixed] - Fix android modal not disappear when reload
Pull Request resolved: facebook#27542

Test Plan: Open a modal and do a refresh to see whether it disappears

Differential Revision: D19178803

Pulled By: mdvacca

fbshipit-source-id: 61894927fc481650804b2196df06a80c16b64e6c
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug DX Issues concerning how the developer experience can be improved. Impact: Regression Describes a behavior that used to work on a prior release, but stopped working recently. Platform: Android Android applications. Platform: Windows Building on Windows.
Projects
None yet
Development

Successfully merging a pull request may close this issue.