fix(modal): fix onDismiss logic for the modal component on iOS

[bartolkaruza]

Summary

This PR fixes #26473 which was introduced by bd2b7d6

Basically componentWillUnmount for a modal is never called because it internally returns 'null' and remains mounted on iOS. For Android, this may have worked because of empty nodes being removed by the OS from the view hierarchy, but I'm not sure.

Changelog

[iOS] [Fixed] - fix onDismiss logic for the modal component on iOS

Test Plan

Before this change onDismiss is never called. After the change it is called on prop change visible from true to false.

const App = () => {
  const [isOpen, setOpen] = useState(true);

  function onDismiss() {
    console.log('dismissed');
  }

  return (
    <>
      <StatusBar barStyle="dark-content" />
      <SafeAreaView>
        <Modal visible={isOpen} animationType="slide" onDismiss={onDismiss}>
          <TouchableOpacity onPress={() => setOpen(false)}>
            <View style={{height: 500, backgroundColor: 'blue'}}>
              <Text>1</Text>
            </View>
          </TouchableOpacity>
        </Modal>
      </SafeAreaView>
    </>
  );
};

[pull-bot]

Messages

📖

📋 Verify Changelog Format - A changelog entry has the following format: [CATEGORY] [TYPE] - Message. CATEGORY may be: General iOS Android JavaScript Internal (for changes that do not need to be called out in the release notes) TYPE may be: Added, for new features. Changed, for changes in existing functionality. Deprecated, for soon-to-be removed features. Removed, for now removed features. Fixed, for any bug fixes. Security, in case of vulnerabilities. MESSAGE may answer "what and why" on a feature level. Use this to briefly tell React Native users about notable changes.

Generated by 🚫 dangerJS against c37d813

[wddwycc]

This callback is initially designed to represent UIViewController.dismiss's onCompletion on iOS. If it is changed as the timing when visible is changed from true to false, why we even need this callback, we can just inspect visible itself.

I'd prefer we revert this commit and keep everything as before.

[wddwycc]

Because iOS won't allow you to present any UIViewController when dismissing UIViewController, so this callback is the only reliable timing we can present the next UIViewController after dismissing the modal. This commit breaks this mechanism.

[bartolkaruza]

This callback is initially designed to represent UIViewController.dismiss's onCompletion on iOS. If it is changed as the timing when visible is changed from true to false, why we even need this callback, we can just inspect visible itself.

Is the timing actually different?

Oh I see what the issue is. Let me see if I can rework this to capture all cases.

[neilkimmett]

Yep that commit broke some of our flows too, because we rely on the UIViewController being dismissed before presenting another. If we could revert that would be amazing.

[bartolkaruza]

So the commit has been reverted by @grabbou and picked for the next 0.61 RC which should solve all the immediate problems for you.

a5353c0

Maybe @sammy-SC could chime in here for where you wanted to take this change? I think a working solution for both iOS and Android (keeping the mechanism in tact for iOS, as it is right now) should be easy to do.

[neilkimmett]

Amazing, thanks for the quick fix @bartolkaruza @grabbou, really appreciate it.

[bartolkaruza]

The fix is in this PR causes new issues that are worse than the original issue. A new solution to this problem needs to be found, in another PR.