Skip to content

Commit

Permalink
Fix dismissing modals with sideMenu (wix#7369)
Browse files Browse the repository at this point in the history
Dismissing a modal that has a `sideMenu` layout exposed a bug we have where we look for the component in the top ViewController presented children, with `sideMenu` it's not enough because its children are not components. 
Instead, We should recursively look for that component in all of the top viewController children tree.

Closes wix#7367
  • Loading branch information
yogevbd authored Nov 24, 2021
1 parent 5cc967b commit 709ddb8
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 2 deletions.
9 changes: 9 additions & 0 deletions e2e/Modals.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -203,4 +203,13 @@ describe('modal', () => {

await expect(elementByLabel('Toggle declared modal')).toBeVisible();
});

it.e2e('dismiss modal with side menu', async () => {
await elementById(TestIDs.MODAL_COMMANDS_BTN).tap();
await elementById(TestIDs.SHOW_SIDE_MENU_MODAL).tap();
await expect(elementByLabel('StatusBar Options')).toBeVisible();
await elementById(TestIDs.DISMISS_MODAL_TOPBAR_BTN).tap();
await expect(elementByLabel('StatusBar Options')).not.toBeVisible();
await expect(elementByLabel('Modal Commands')).toBeVisible();
});
});
3 changes: 1 addition & 2 deletions lib/ios/RNNModalManager.m
Original file line number Diff line number Diff line change
Expand Up @@ -177,8 +177,7 @@ - (void)removePendingNextModalIfOnTop:(RNNTransitionCompletionBlock)completion
_dismissModalTransitionDelegate;
}

if ((modalToDismiss == topPresentedVC ||
[[topPresentedVC childViewControllers] containsObject:modalToDismiss])) {
if ((modalToDismiss == topPresentedVC || [topPresentedVC findViewController:modalToDismiss])) {
[self dismissSearchController:modalToDismiss];
[modalToDismiss
dismissViewControllerAnimated:animated
Expand Down
8 changes: 8 additions & 0 deletions playground/src/screens/ModalCommandsScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ const {
SHOW_MODAL_PROMISE_RESULT,
MODAL_DISMISSED_LISTENER_RESULT,
DISMISS_MODAL_PROMISE_RESULT,
SHOW_SIDE_MENU_MODAL,
} = testIDs;

interface State {
Expand Down Expand Up @@ -51,6 +52,11 @@ export default class ModalScreen extends NavigationComponent<NavigationComponent
{this.state?.dismissModalPromiseResult || ''}
</Text>
<Button label="Show Modal" testID={MODAL_BTN} onPress={this.showModal} />
<Button
label="Show Side Menu Modal"
testID={SHOW_SIDE_MENU_MODAL}
onPress={this.showSideMenuModal}
/>
</Root>
);
}
Expand Down Expand Up @@ -85,4 +91,6 @@ export default class ModalScreen extends NavigationComponent<NavigationComponent
});
});
};

showSideMenuModal = () => Navigation.showModal(Screens.StatusBar);
}
1 change: 1 addition & 0 deletions playground/src/screens/ModalScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ export default class ModalScreen extends NavigationComponent<Props, State> {
overlay: { interceptTouchOutside: false },
});
};

toggleModal = () => this.setState({ modalVisible: !this.state.modalVisible });

showModalWithTransition = () => {
Expand Down
1 change: 1 addition & 0 deletions playground/src/testIDs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ const testIDs = {
DISMISS_MODAL_BTN: 'DISMISS_MODAL_BUTTON',
DISMISS_REACT_MODAL_BTN: 'DISMISS_REACT_MODAL_BUTTON',
DISMISS_MODAL_TOPBAR_BTN: 'DISMISS_MODAL_TOPBAR_BTN',
SHOW_SIDE_MENU_MODAL: 'SHOW_SIDE_MENU_MODAL',
CHANGE_LEFT_RIGHT_COLORS: 'CHANGE_LEFT_RIGHT_COLORS',
MODAL_SCREEN_HEADER: 'MODAL_SCREEN_HEADER',
ALERT_BUTTON: 'ALERT_BUTTON',
Expand Down

0 comments on commit 709ddb8

Please sign in to comment.