From 711e670f749b4808a330f775b33975eb8b53b94a Mon Sep 17 00:00:00 2001 From: Julien Brayere Date: Wed, 2 Jun 2021 14:26:44 +0200 Subject: [PATCH] Fix dismiss modal not hiding properly with searchBar on iOS (#7087) The problem was that `dismissModal` first dismissed the UISearchController and not the actual modal. Fixed by manually dismiss the search controller first and only then dismiss the modal. Closes: https://github.com/wix/react-native-navigation/issues/7086 Co-authored-by: Julien Brayere Co-authored-by: Yogev Ben David --- e2e/BackButton.test.js | 41 ++++++---------- e2e/Modals.test.js | 7 ++- e2e/SearchBar.test.js | 24 +++++++++- lib/ios/RNNModalManager.m | 7 +++ package.json | 2 +- playground/src/commons/Processors.ts | 14 ++++-- playground/src/screens/OptionsScreen.tsx | 14 ++++-- playground/src/screens/PushedScreen.tsx | 10 ++-- playground/src/screens/Screens.ts | 1 + playground/src/screens/SearchBarModal.tsx | 57 +++++++++++++++++++++++ playground/src/screens/index.tsx | 1 + playground/src/testIDs.ts | 2 + 12 files changed, 133 insertions(+), 47 deletions(-) create mode 100644 playground/src/screens/SearchBarModal.tsx diff --git a/e2e/BackButton.test.js b/e2e/BackButton.test.js index 09cc3ae69dc..fac74a1b7a0 100644 --- a/e2e/BackButton.test.js +++ b/e2e/BackButton.test.js @@ -1,7 +1,6 @@ -import Utils from './Utils'; -import TestIDs from '../playground/src/testIDs'; +import { default as TestIDs, default as testIDs } from '../playground/src/testIDs'; import Android from './AndroidUtils'; -import testIDs from '../playground/src/testIDs'; +import Utils from './Utils'; const { elementByLabel, elementById } = Utils; @@ -17,22 +16,16 @@ describe('Back Button', () => { await elementById(TestIDs.PUSH_DISABLED_BACK_BTN).tap(); await elementById(TestIDs.CLEAR_OVERLAY_EVENTS_BTN).tap(); await elementById(TestIDs.BACK_BUTTON).tap(); - await expect( - elementByLabel('navigationButtonPressed | RNN.back') - ).toBeVisible(); - await expect( - elementById(testIDs.PUSHED_SCREEN_HEADER) - ).toBeVisible(); - }) + await expect(elementByLabel('navigationButtonPressed | RNN.back')).toBeVisible(); + await expect(elementById(testIDs.PUSHED_SCREEN_HEADER)).toBeVisible(); + }); it('pops and does not dispatch event', async () => { await elementById(TestIDs.PUSH_BTN).tap(); await elementById(TestIDs.CLEAR_OVERLAY_EVENTS_BTN).tap(); await elementById(TestIDs.BACK_BUTTON).tap(); - await expect( - elementByLabel('navigationButtonPressed | RNN.back') - ).toBeNotVisible(); - }) + await expect(elementByLabel('navigationButtonPressed | RNN.back')).toBeNotVisible(); + }); it('toggle visibility', async () => { await elementById(TestIDs.TOGGLE_BACK_BUTTON_VISIBILITY).tap(); @@ -47,13 +40,9 @@ describe('Back Button', () => { await elementById(TestIDs.PUSH_DISABLED_HARDWARE_BACK_BTN).tap(); await elementById(TestIDs.CLEAR_OVERLAY_EVENTS_BTN).tap(); Android.pressBack(); - await expect( - elementByLabel('navigationButtonPressed | RNN.hardwareBackButton') - ).toBeVisible(); - await expect( - elementById(testIDs.PUSHED_SCREEN_HEADER) - ).toBeVisible(); - }) + await expect(elementByLabel('navigationButtonPressed | RNN.hardwareBackButton')).toBeVisible(); + await expect(elementById(testIDs.PUSHED_SCREEN_HEADER)).toBeVisible(); + }); it(':android: hardware button pops and does not dispatch event', async () => { await elementById(TestIDs.PUSH_BTN).tap(); @@ -62,18 +51,14 @@ describe('Back Button', () => { await expect( elementByLabel('navigationButtonPressed | RNN.hardwareBackButton') ).toBeNotVisible(); - await expect( - elementById(testIDs.PUSHED_SCREEN_HEADER) - ).toBeNotVisible(); - }) + await expect(elementById(testIDs.PUSHED_SCREEN_HEADER)).toBeNotVisible(); + }); it(':android: hardware back should not dismiss modal and dispatch event', async () => { await elementById(TestIDs.MODAL_DISABLED_BACK_BTN).tap(); await expect(elementByLabel('Modal')).toBeVisible(); Android.pressBack(); await expect(elementByLabel('Modal')).toBeVisible(); - await expect( - elementByLabel('navigationButtonPressed | RNN.hardwareBackButton') - ).toBeVisible(); + await expect(elementByLabel('navigationButtonPressed | RNN.hardwareBackButton')).toBeVisible(); }); }); diff --git a/e2e/Modals.test.js b/e2e/Modals.test.js index e497e2010c7..90defc17c49 100644 --- a/e2e/Modals.test.js +++ b/e2e/Modals.test.js @@ -1,12 +1,12 @@ -import Utils from './Utils'; import TestIDs from '../playground/src/testIDs'; import Android from './AndroidUtils'; +import Utils from './Utils'; -const {elementByLabel, elementById, sleep} = Utils; +const { elementByLabel, elementById, sleep } = Utils; describe('modal', () => { beforeEach(async () => { - await device.launchApp({newInstance: true}); + await device.launchApp({ newInstance: true }); await elementById(TestIDs.NAVIGATION_TAB).tap(); await elementById(TestIDs.MODAL_BTN).tap(); }); @@ -111,7 +111,6 @@ describe('modal', () => { await expect(elementByLabel('Pushed Screen')).toBeVisible(); }); - it('present modal multiple times', async () => { await elementById(TestIDs.DISMISS_MODAL_BTN).tap(); await elementById(TestIDs.MODAL_BTN).tap(); diff --git a/e2e/SearchBar.test.js b/e2e/SearchBar.test.js index 43f900db7f3..0d1b5e75451 100644 --- a/e2e/SearchBar.test.js +++ b/e2e/SearchBar.test.js @@ -1,5 +1,5 @@ -import Utils from './Utils'; import TestIDs from '../playground/src/testIDs'; +import Utils from './Utils'; const { elementById, elementByTraits } = Utils; @@ -17,3 +17,25 @@ describe(':ios: SearchBar', () => { await expect(elementByTraits(['searchField'])).toBeNotVisible(); }); }); + +describe(':ios: SearchBar Modal', () => { + beforeAll(async () => { + await device.launchApp({ newInstance: true }); + await elementById(TestIDs.OPTIONS_TAB).tap(); + await elementById(TestIDs.GOTO_SEARCHBAR_MODAL).tap(); + }); + + it('show and hide search bar', async () => { + await elementById(TestIDs.SHOW_SEARCH_BAR_BTN).tap(); + await expect(elementByTraits(['searchField'])).toBeVisible(); + await elementById(TestIDs.HIDE_SEARCH_BAR_BTN).tap(); + await expect(elementByTraits(['searchField'])).toBeNotVisible(); + }); + + it('searching then exiting works', async () => { + await elementById(TestIDs.SHOW_SEARCH_BAR_BTN).tap(); + await elementByTraits(['searchField']).replaceText('foo'); + await elementById(TestIDs.DISMISS_MODAL_TOPBAR_BTN).tap(); + await expect(elementById(TestIDs.OPTIONS_TAB)).toBeVisible(); + }); +}); diff --git a/lib/ios/RNNModalManager.m b/lib/ios/RNNModalManager.m index 38143fb0e1a..a586882189d 100644 --- a/lib/ios/RNNModalManager.m +++ b/lib/ios/RNNModalManager.m @@ -154,6 +154,7 @@ - (void)removePendingNextModalIfOnTop:(RNNTransitionCompletionBlock)completion { if (modalToDismiss == topPresentedVC || [[topPresentedVC childViewControllers] containsObject:modalToDismiss]) { + [self dismissSearchController:modalToDismiss]; [modalToDismiss dismissViewControllerAnimated:[optionsWithDefault.animations.dismissModal.exit.enable withDefault:YES] @@ -182,6 +183,12 @@ - (void)removePendingNextModalIfOnTop:(RNNTransitionCompletionBlock)completion { } } +- (void)dismissSearchController:(UIViewController *)modalToDismiss { + if ([modalToDismiss.presentedViewController.class isSubclassOfClass:UISearchController.class]) { + [modalToDismiss.presentedViewController dismissViewControllerAnimated:NO completion:nil]; + } +} + - (void)dismissedModal:(UIViewController *)viewController { [_presentedModals removeObject:[viewController topMostViewController]]; [_eventHandler dismissedModal:viewController.presentedComponentViewController]; diff --git a/package.json b/package.json index 3e972c6e1c6..3279a1ac2a8 100644 --- a/package.json +++ b/package.json @@ -187,4 +187,4 @@ } } } -} \ No newline at end of file +} diff --git a/playground/src/commons/Processors.ts b/playground/src/commons/Processors.ts index ec05bf7b997..067b010a7f4 100644 --- a/playground/src/commons/Processors.ts +++ b/playground/src/commons/Processors.ts @@ -1,14 +1,17 @@ +import { merge } from 'lodash'; import { + CommandName, + Layout, + LayoutComponent, Navigation, - OptionsTopBar, NavigationButtonPressedEvent, - Layout, - CommandName, Options, - LayoutComponent, + OptionsTopBar, } from 'react-native-navigation'; -import { merge } from 'lodash'; import flags from '../flags'; +import testIDs from '../testIDs'; + +const { DISMISS_MODAL_TOPBAR_BTN } = testIDs; const colors = [ '#fff1e6', @@ -37,6 +40,7 @@ const addDismissModalProcessor = () => { if (!topBar.leftButtons) { topBar.leftButtons = [ { + testID: DISMISS_MODAL_TOPBAR_BTN, id: 'dismissModalButton', icon: require('../../img/x.png'), color: 'black', diff --git a/playground/src/screens/OptionsScreen.tsx b/playground/src/screens/OptionsScreen.tsx index eab3dd29aeb..da9dafe91d8 100644 --- a/playground/src/screens/OptionsScreen.tsx +++ b/playground/src/screens/OptionsScreen.tsx @@ -1,11 +1,10 @@ import React from 'react'; import { NavigationComponentProps } from 'react-native-navigation'; - -import Root from '../components/Root'; import Button from '../components/Button'; +import Root from '../components/Root'; import Navigation from '../services/Navigation'; -import Screens from './Screens'; import testIDs from '../testIDs'; +import Screens from './Screens'; const { CHANGE_TITLE_BTN, @@ -18,6 +17,7 @@ const { SET_REACT_TITLE_VIEW, GOTO_BUTTONS_SCREEN, GOTO_SEARCHBAR_SCREEN, + GOTO_SEARCHBAR_MODAL, } = testIDs; interface Props extends NavigationComponentProps {} @@ -73,6 +73,12 @@ export default class Options extends React.Component { label="Search Bar" onPress={this.searchBarScreen} /> +