Skip to content

Commit

Permalink
Fix onDismiss in Modal
Browse files Browse the repository at this point in the history
Summary:
# Disclaimer:
I might be missing something as the solution I implemented here seems like something that was considered by original author. If this solution isn't good, I have a plan B.

# Problem:
`onDismiss` prop isn't being called once the modal is dismissed, this diff fixes it.

Also I've noticed that `onDismiss` is meant to only work on iOS, why is that? By landing this diff, it'll be called on Android as well so we need to change the docs (https://facebook.github.io/react-native/docs/modal.html#ondismiss).

## Video that shows the problem
Following code is in playground.js P70222409 which just increments number everytime onDismiss is called

{F166303269}

Reviewed By: shergin

Differential Revision: D16109536

fbshipit-source-id: 3fba56f5671912387b217f03b613dffd89614c9d
  • Loading branch information
sammy-SC authored and facebook-github-bot committed Jul 29, 2019
1 parent 5ec382d commit bd2b7d6
Show file tree
Hide file tree
Showing 5 changed files with 4 additions and 97 deletions.
24 changes: 2 additions & 22 deletions Libraries/Modal/Modal.js
Expand Up @@ -12,20 +12,13 @@

const AppContainer = require('../ReactNative/AppContainer');
const I18nManager = require('../ReactNative/I18nManager');
const NativeEventEmitter = require('../EventEmitter/NativeEventEmitter');
import NativeModalManager from './NativeModalManager';
const Platform = require('../Utilities/Platform');
const React = require('react');
const PropTypes = require('prop-types');
const ScrollView = require('../Components/ScrollView/ScrollView');
const StyleSheet = require('../StyleSheet/StyleSheet');
const View = require('../Components/View/View');

import RCTModalHostView from './RCTModalHostViewNativeComponent';
const ModalEventEmitter =
Platform.OS === 'ios' && NativeModalManager != null
? new NativeEventEmitter(NativeModalManager)
: null;

import type EmitterSubscription from '../vendor/emitter/EmitterSubscription';
import type {ViewProps} from '../Components/View/ViewPropTypes';
Expand Down Expand Up @@ -175,22 +168,9 @@ class Modal extends React.Component<Props> {
};
}

componentDidMount() {
if (ModalEventEmitter) {
this._eventSubscription = ModalEventEmitter.addListener(
'modalDismissed',
event => {
if (event.modalID === this._identifier && this.props.onDismiss) {
this.props.onDismiss();
}
},
);
}
}

componentWillUnmount() {
if (this._eventSubscription) {
this._eventSubscription.remove();
if (this.props.onDismiss != null) {
this.props.onDismiss();
}
}

Expand Down
8 changes: 0 additions & 8 deletions Libraries/Modal/RCTModalHostViewNativeComponent.js
Expand Up @@ -78,14 +78,6 @@ type NativeProps = $ReadOnly<{|
*/
onShow?: ?DirectEventHandler<null>,

/**
* The `onDismiss` prop allows passing a function that will be called once
* the modal has been dismissed.
*
* See https://facebook.github.io/react-native/docs/modal.html#ondismiss
*/
onDismiss?: ?BubblingEventHandler<null>,

/**
* Deprecated. Use the `animationType` prop instead.
*/
Expand Down
10 changes: 2 additions & 8 deletions React/Views/RCTModalHostViewManager.m
Expand Up @@ -10,7 +10,6 @@
#import "RCTBridge.h"
#import "RCTModalHostView.h"
#import "RCTModalHostViewController.h"
#import "RCTModalManager.h"
#import "RCTShadowView.h"
#import "RCTUtils.h"

Expand Down Expand Up @@ -81,15 +80,10 @@ - (void)presentModalHostView:(RCTModalHostView *)modalHostView withViewControlle

- (void)dismissModalHostView:(RCTModalHostView *)modalHostView withViewController:(RCTModalHostViewController *)viewController animated:(BOOL)animated
{
dispatch_block_t completionBlock = ^{
if (modalHostView.identifier) {
[[self.bridge moduleForClass:[RCTModalManager class]] modalDismissed:modalHostView.identifier];
}
};
if (_dismissalBlock) {
_dismissalBlock([modalHostView reactViewController], viewController, animated, completionBlock);
_dismissalBlock([modalHostView reactViewController], viewController, animated, nil);
} else {
[viewController.presentingViewController dismissViewControllerAnimated:animated completion:completionBlock];
[viewController.presentingViewController dismissViewControllerAnimated:animated completion:nil];
}
}

Expand Down
17 changes: 0 additions & 17 deletions React/Views/RCTModalManager.h

This file was deleted.

42 changes: 0 additions & 42 deletions React/Views/RCTModalManager.m

This file was deleted.

0 comments on commit bd2b7d6

Please sign in to comment.