react-native-screens icon indicating copy to clipboard operation
react-native-screens copied to clipboard

fix(IOS): Fix non dismissible modal presentation

Open lodev09 opened this issue 1 month ago • 3 comments

Description

This PR fixes an issue with the modal presentation chain when non-dismissible external modals (e.g., third-party modals like TrueSheet) are present in the view controller hierarchy.

Previously, when a non-dismissible modal was encountered, the stack didn't properly update the root controller for subsequent modal presentations, which could cause modals to be presented from the wrong controller or fail to dismiss modals that were presented by the non-dismissible modal.

Changes

  • Enhanced non-dismissible modal handling in RNSScreenStack.mm:
  • Refactored the dismissal check into a separate boolean variable for better readability
  • Added an else branch to handle the case when firstModalToBeDismissed is non-dismissible
  • When a non-dismissible modal is detected, the changeRootController is now updated to point to that modal
  • Added logic to check if the non-dismissible modal itself has presented any owned modals, and properly dismisses them before presenting new ones

Screenshots / GIFs

Before

Sheet will not present, will get warning in XCode.

After

https://github.com/user-attachments/assets/90d8534b-7115-47f0-88ba-3707737da16c

Test code and steps to reproduce

To test this change with a non-dismissible third-party modal (e.g., TrueSheet):

1. First, ensure your third-party modal implements the protocol (library author's responsibility):

// In your third-party library's view controller (e.g., TrueSheet)
#import "RNSDismissibleModalProtocol.h"

@interface YourModalViewController : UIViewController <RNSDismissibleModalProtocol>
@end

@implementation YourModalViewController

- (BOOL)isDismissible {
  return NO; // Mark as non-dismissible
}

@end

2. Then test with this example:

import React from 'react';
import { View, Button } from 'react-native';
import { createNativeStackNavigator } from '@react-navigation/native-stack';
import { TrueSheet } from '@lodev09/react-native-true-sheet'; // Example third-party modal

const Stack = createNativeStackNavigator();

function HomeScreen({ navigation }) {
  const sheet = React.useRef<TrueSheet>(null);

  return (
    <View style={{ flex: 1, justifyContent: 'center', alignItems: 'center' }}>
      <Button title="Show TrueSheet" onPress={() => sheet.current?.present()} />
      <TrueSheet ref={sheet}>
        <Button 
          title="Open React Navigation Modal" 
          onPress={() => navigation.navigate('Modal')} 
        />
      </TrueSheet>
    </View>
  );
}

function ModalScreen() {
  return (
    <View style={{ flex: 1, justifyContent: 'center', alignItems: 'center' }}>
      <Text>This is a modal</Text>
    </View>
  );
}

function App() {
  return (
    <Stack.Navigator>
      <Stack.Screen name="Home" component={HomeScreen} />
      <Stack.Screen 
        name="Modal" 
        component={ModalScreen} 
        options={{ presentation: 'modal' }} 
      />
    </Stack.Navigator>
  );
}
  1. Open the TrueSheet (non-dismissible modal that implements RNSDismissibleModalProtocol)
  2. From within the sheet, open a React Navigation modal
  3. The modal should present correctly from the non-dismissible modal
  4. Subsequent modal presentations and dismissals should work as expected
  5. The TrueSheet should remain visible and not be dismissed

Checklist

  • [x] Included code example that can be used to test this change
  • [ ] Updated TS types
  • [ ] Updated documentation:
    • [ ] https://github.com/software-mansion/react-native-screens/blob/main/guides/GUIDE_FOR_LIBRARY_AUTHORS.md
    • [ ] https://github.com/software-mansion/react-native-screens/blob/main/native-stack/README.md
    • [ ] https://github.com/software-mansion/react-native-screens/blob/main/src/types.tsx
    • [ ] https://github.com/software-mansion/react-native-screens/blob/main/src/native-stack/types.tsx
  • [x] Ensured that CI passes

lodev09 avatar Nov 23 '25 13:11 lodev09

@kkafar an alternative approach would be to create another method to the protocol to return a ViewController that should update the root controller to keep things safe. Let me know if that's better. Thank you!

lodev09 avatar Nov 23 '25 14:11 lodev09

Can we merge this @kligarski @kkafar? This should be straightforward and backwards compatible. Tested and works in a production app. Thank you!

FYI @erickreutz

lodev09 avatar Dec 01 '25 16:12 lodev09

Hey, sorry for very late response here & on other PRs. I'll look into this after we release 4.19.0 (planned for upcoming week).

kkafar avatar Dec 05 '25 09:12 kkafar