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):

old_arch.mov

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

new_arch.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: 3be3731
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]

	Fails
🚫	📋 Missing Changelog - Please add a Changelog to your PR description. See Changelog format

Generated by 🚫 dangerJS against ff19cd0

[facebook-github-bot]

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

[sammy-SC]

I think the renderer needs this information. What if React Native's viewport is not full screen but only partially covers the screen? In that case, we need to feed the information about available size to React Native so it knows that whatever is within the modal, is not constrained by RN's viewport size.

[WoLewicki]

Is there another direction that we could go to resolve the issue then @sammy-SC ?

[sammy-SC]

I would try to investigate why is the height wrong in the new architecture. The height seems to be off exactly by the height of the status bar. Could it be that ModalHostHelper.getModalHostSize(view.getContext()) returns height of React Native's view port instead of the desired size of the modal? Where is the height calculated for the old architecture and why is it correct? Maybe we missed something in the implementation for the new architecture.

[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]

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.