Remove most of `Modal` state updates on Android

[j-piasecki]

Summary

On the old architecture, when using a Modal with no animation and translucent status bar, a layout flash is noticeable when the modal is opened. Here's the video of this happening (it's visible for one frame):

https://user-images.githubusercontent.com/21055725/211768547-d3ff49b9-2ce6-4f36-9f0f-91ad102fab28.mov

On the new architecture, the issue is more problematic as the modal keeps the wrong height:

https://user-images.githubusercontent.com/21055725/211768801-df57b295-fa04-4bd8-af35-0057553bf15b.mov

Simply removing the state updates for Modal seems to be solving the problem (at the cost of the modal appearing one frame later), however, I'm not that familiar with the code so it may have some unintended consequences. Updating the implementation of ModalHostHelper::getModalHostSize may be a better solution.

Issue relevant to this PR (with more details): #35804

Changelog

[Android] [Fixed] - Fixed wrong modal height when statusBarTranslucent is set to true

Test Plan

I've tested the changes on the following snippet:

import React, {useState} from 'react';
import {View, Button, Modal} from 'react-native';

const App = () => {
  const [open, setOpen] = useState(false);

  return (
    <View style={{flex: 1, backgroundColor: 'yellow'}}>
      <Button
        title="Open"
        onPress={() => {
          setOpen(true);
        }}
      />

      <Modal
        visible={open}
        animationType="none"
        statusBarTranslucent={true}>
        <View style={{flex: 1, backgroundColor: 'red'}}>
          <Button
            title="Close"
            onPress={() => {
              setOpen(false);
            }}
          />
        </View>
      </Modal>
    </View>
  );
};

export default App;

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	9,042,404	-289
android	hermes	armeabi-v7a	8,292,200	-287
android	hermes	x86	9,559,010	-286
android	hermes	x86_64	9,401,072	-296
android	jsc	arm64-v8a	9,601,309	-545
android	jsc	armeabi-v7a	8,728,432	-553
android	jsc	x86	9,688,805	-543
android	jsc	x86_64	9,934,878	-538

Base commit: 3be3731f488e3b70509c0a0810e1b2cd0dcfbf46 Branch: main

[jacdebug]

From blame looks like this is added for fixing some app freezes, here the comment from original commit,

In some cases, the size of the content view changes before we add views to the Modal. That means that the size of that view will not be set through the onSizeChanged method. This can result in some apps apparently freezing, since the dialog is created, but there are no actual views in it. For that reason, we still need the ModalHostShadowNode to set the size of the initial view, so that by the time the view gets added it already has the correct size. There's a new helper class so that we can reuse the modal size computation.

cc @cortinico ref: D3892795

[cortinico]

cc @cortinico ref: D3892795

I'm not sure that's the right diff number @jacdebug

[jacdebug]

cc @cortinico ref: D3892795

I'm not sure that's the right diff number @jacdebug

Ah sorry missing the context, its the change where original fix was added.

[cjshearer]

I'm impacted by this and would like to see it merged. Anything holding this up (other than conflicts)?

[kelset]

@cortinico @jacdebug is there anything specific blocking this?

or @j-piasecki could rebase/address conflict and then it'd be ready to go?

[cortinico]

@cortinico @jacdebug is there anything specific blocking this?

Nope this should be good to go. Let's rebase and merge it

[github-actions[bot]]

	Fails
:no_entry_sign:	:clipboard: Missing Changelog - Please add a Changelog to your PR description. See Changelog format

Generated by :no_entry_sign: dangerJS against ff19cd08df1f9b598fff857d34900975e305bbfa

[facebook-github-bot]

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

[lyswhut]

Any news? Hope to fix it in v0.72

I enabled the new architecture and set the StatusBartransLucent as true, and then got the wrong modal height

[github-actions[bot]]

This PR is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 7 days.

[j-piasecki]

I still have it at the back of my head, I'll try to get back to it shortly when I have some time.