Update Modal's mock to not render its children when it is not visible

[AntoineDoubovetzky]

Summary

The Modal's mock always render its children (whether it is visible or not), whereas in reality the Modal renders null when the Modal is not visible.
This causes troubles when trying to test whether the Modal is visible or not. Instead of testing if the children are rendered (using getByText from React Native Testing Library for instance), we are forced to test the value of the visible prop directly (see callstack/react-native-testing-library#508 and callstack/react-native-testing-library#659).
This is not ideal because we are forced to test implementation detail and can't test from the user perspective. I also believe the mock should be as close as possible from reality.

I had 2 options:

1. Rendering the Modal without its children
2. Not rendering the Modal at all

The latter has the advantage of being closer to the reality, but I chose the former to still be able to test the Modal through the visible prop, so there is no breaking change (only snapshots update will be required).

Changelog

[General] [Changed] - Update Modal's mock to not render its children when it is not visible. ⚠️ Snapshots containing Modal with visible={false} will have to be updated.

Test Plan

I added a test case when visible is false, then updated the mock so the children are not rendered. The before / after is here:

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
ios	-	universal	n/a	--

Base commit: 3562364

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	7,705,285	-10,924
android	hermes	armeabi-v7a	7,236,467	-7,970
android	hermes	x86	8,126,260	-9,274
android	hermes	x86_64	8,091,221	-9,860
android	jsc	arm64-v8a	9,625,139	-10,131
android	jsc	armeabi-v7a	8,543,852	-7,161
android	jsc	x86	9,640,401	-8,468
android	jsc	x86_64	10,249,056	-9,047

Base commit: 3562364

[facebook-github-bot]

@lunaleaps has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[MattAgn]

Totally agree with this proposition, I've had this issue more than once where I had to use visible prop in my tests

[pierpo]

Fantastic, I've been needing this as well 😊

It is a breaking change though. It needs to be documented as such since it will break some people's tests upon upgrade!

[react-native-bot]

This pull request was successfully merged by @AntoineDoubovetzky in ec614c1.

^{When will my fix make it into a release? | Upcoming Releases}