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: #7086 Co-authored-by: Julien Brayere <[email protected]> Co-authored-by: Yogev Ben David <[email protected]>
wix · Jun 2, 2021 · 711e670 · 711e670
1 parent 1f6c8ba
commit 711e670
Show file tree

Hide file tree

Showing 12 changed files with 133 additions and 47 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -187,4 +187,4 @@
       }
     }
   }
-}
+}
diff --git 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
@@ -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<Props> {
           label="Search Bar"
           onPress={this.searchBarScreen}
         />
+        <Button
+          platform={'ios'}
+          testID={GOTO_SEARCHBAR_MODAL}
+          label="Search Bar Modal"
+          onPress={this.searchBarModal}
+        />
         <Button
           label="Toggle Navigation bar visibility"
           platform="android"
@@ -192,6 +198,8 @@ export default class Options extends React.Component<Props> {
 
   searchBarScreen = () => Navigation.push(this, Screens.SearchBar, {});
 
+  searchBarModal = () => Navigation.showModal(Screens.SearchBarModal);
+
   pushButtonsScreen = () =>
     Navigation.push(this, Screens.Buttons, {
       animations: {

diff --git a/playground/src/screens/PushedScreen.tsx b/playground/src/screens/PushedScreen.tsx
@@ -1,17 +1,17 @@
+import concat from 'lodash/concat';
 import React from 'react';
 import { BackHandler, StyleSheet, View } from 'react-native';
 import {
+  NavigationButtonPressedEvent,
   NavigationComponent,
   NavigationComponentProps,
-  NavigationButtonPressedEvent,
   Options,
 } from 'react-native-navigation';
-import concat from 'lodash/concat';
-import Navigation from '../services/Navigation';
-import Root from '../components/Root';
 import Button from '../components/Button';
-import Screens from './Screens';
+import Root from '../components/Root';
+import Navigation from '../services/Navigation';
 import testIDs from '../testIDs';
+import Screens from './Screens';
 
 const {
   PUSHED_SCREEN_HEADER,

diff --git a/playground/src/screens/Screens.ts b/playground/src/screens/Screens.ts
@@ -128,6 +128,7 @@ const Screens = {
   OrientationDetect: 'OrientationDetect',
   Search: 'Search',
   SearchBar: 'SearchBar',
+  SearchBarModal: 'SearchBarModal',
   TopBar: 'TopBar',
 };
 

diff --git a/playground/src/screens/SearchBarModal.tsx b/playground/src/screens/SearchBarModal.tsx
@@ -0,0 +1,57 @@
+import React from 'react';
+import { NavigationComponentProps } from 'react-native-navigation';
+import Button from '../components/Button';
+import Root from '../components/Root';
+import Navigation from '../services/Navigation';
+import testIDs from '../testIDs';
+
+const { SHOW_SEARCH_BAR_BTN, HIDE_SEARCH_BAR_BTN, TOP_BAR } = testIDs;
+
+interface Props extends NavigationComponentProps {}
+
+export default class SearchBarModal extends React.Component<Props> {
+  static options() {
+    return {
+      topBar: {
+        visible: true,
+        testID: TOP_BAR,
+        title: {
+          text: 'SearchBar Modal Options',
+        },
+      },
+    };
+  }
+
+  state = {
+    isAndroidNavigationBarVisible: true,
+  };
+
+  render() {
+    return (
+      <Root componentId={this.props.componentId}>
+        {/* <Button label="Hide TopBar" testID={HIDE_TOP_BAR_BTN} onPress={this.hideTopBar} /> */}
+        {/* <Button label="Show TopBar" testID={SHOW_TOP_BAR_BTN} onPress={this.showTopBar} /> */}
+        <Button label="Hide SearchBar" testID={HIDE_SEARCH_BAR_BTN} onPress={this.hideSearchBar} />
+        <Button label="Show SearchBar" testID={SHOW_SEARCH_BAR_BTN} onPress={this.showSearchBar} />
+      </Root>
+    );
+  }
+
+  hideSearchBar = () =>
+    Navigation.mergeOptions(this, {
+      topBar: {
+        searchBar: {
+          visible: false,
+        },
+      },
+    });
+
+  showSearchBar = () =>
+    Navigation.mergeOptions(this, {
+      topBar: {
+        searchBar: {
+          visible: true,
+        },
+      },
+    });
+}
diff --git a/playground/src/screens/index.tsx b/playground/src/screens/index.tsx
@@ -100,6 +100,7 @@ function registerScreens() {
   );
   Navigation.registerComponent(Screens.Search, () => require('./SearchScreen').default);
   Navigation.registerComponent(Screens.SearchBar, () => require('./SearchBar').default);
+  Navigation.registerComponent(Screens.SearchBarModal, () => require('./SearchBarModal').default);
   Navigation.registerComponent(Screens.SetRoot, () => require('./SetRootScreen').default);
   Navigation.registerComponent(
     Screens.SideMenuCenter,

diff --git a/playground/src/testIDs.ts b/playground/src/testIDs.ts
@@ -22,6 +22,7 @@ const testIDs = {
   MODAL_DISABLED_BACK_BTN: 'SHOW_MODAL_DISABLED_BACK_BTN',
   PAGE_SHEET_MODAL_BTN: 'SHOW_PAGE_SHEET_MODAL_BUTTON',
   DISMISS_MODAL_BTN: 'DISMISS_MODAL_BUTTON',
+  DISMISS_MODAL_TOPBAR_BTN: 'DISMISS_MODAL_TOPBAR_BTN',
   CHANGE_LEFT_RIGHT_COLORS: 'CHANGE_LEFT_RIGHT_COLORS',
   MODAL_SCREEN_HEADER: 'MODAL_SCREEN_HEADER',
   ALERT_BUTTON: 'ALERT_BUTTON',
@@ -72,6 +73,7 @@ const testIDs = {
   SHOW_SEARCH_BAR_BTN: 'SHOW_SEARCH_BAR_BTN',
   HIDE_SEARCH_BAR_BTN: 'HIDE_SEARCH_BAR_BTN',
   GOTO_SEARCHBAR_SCREEN: 'GOTO_SEARCHBAR_SCREEN',
+  GOTO_SEARCHBAR_MODAL: 'GOTO_SEARCHBAR_MODAL',
   GOTO_TOP_BAR_SCREEN: 'GOTO_TOP_BAR_SCREEN',
   SET_BADGE_BTN: 'SET_BADGE_BTN',
   CLEAR_BADGE_BTN: 'CLEAR_BADGE_BTN',
-Original file line number
+Diff line change
@@ Expand Up / @@ -187,4 +187,4 @@ @@
           }
         }
       }
-    }
+    }