iOS: trigger didUpdateDimensions event when resizing without changing traits

[jpdriver]

Summary:

• when using the useWindowDimensions() hook in a component, it's possible for the hook to report incorrect sizes and not update as frequently as it should
• this is most applicable to apps built for iPad and macOS
• closes Dimensions "change" event does not fire on Apple Silicon/M1 (running iPad-built app) until window is blurred #36118

Existing Behavior - https://youtu.be/NcV6kEDg20E

• either when resizing a React Native app to a different Size Class or changing the Appearance, we dispatch an RCTUserInterfaceStyleDidChangeNotification notification
• these are then handled in the interfaceFrameDidChange method of RCTDeviceInfo
  • this results in a didUpdateDimensions Device Event, which in turn updates the results of useWindowDimensions()
  • see RCTDeviceInfo.mm#L217-L232
  • and Dimensions.js#L119-L124

🐛 However 🐛

• if you are resizing without triggering a UITraitCollection change, the Dimensions reported by useWindowDimensions() can become stale, until you either:-
  • hit a certain width that is considered a different Size Class
  • change the Appearance
  • background then resume the app
  • make the app full-screen

Changed Behavior - https://youtu.be/w9BevpZ2y08

• added a new RCTRootViewFrameDidChangeNotification notification
  • the thinking here is to avoid additional overhead by re-using the same RCTUserInterfaceStyleDidChangeNotification
  • maybe it's overkill?
• the new notifications are sent from an override of setFrame on RCTRootView
• the new notifications call the same interfaceFrameDidChange method of RCTDeviceInfo
• Dimensions are now reported correctly when resizing; even within the same Size Class

Changelog:

[IOS] [FIXED] - Dimensions could be reported incorrectly when resizing iPad or macOS apps

Test Plan:

Reproduction: https://github.com/jpdriver/Dimensions

or to recreate it yourself:-

• Generate a new project
• Change App.tsx

import * as React from 'react';
import {View, Text, useWindowDimensions} from 'react-native';

export default function App() {
  const {width, height} = useWindowDimensions();

  return (
    <View style={{flex: 1, justifyContent: 'center', alignItems: 'center'}}>
      <Text>Width: {width}</Text>
      <Text>Height: {height}</Text>
    </View>
  );
}

• Open the iOS project in Xcode and enable iPad support
• Enable the iPad app to be used in any orientation
• Run the app
• Enable Stage Manager
• Drag one of the corners to resize the app

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	8,963,966	+6
android	hermes	armeabi-v7a	n/a	--
android	hermes	x86	n/a	--
android	hermes	x86_64	n/a	--
android	jsc	arm64-v8a	9,554,299	-8
android	jsc	armeabi-v7a	n/a	--
android	jsc	x86	n/a	--
android	jsc	x86_64	n/a	--

Base commit: 10a076f
Branch: main

[facebook-github-bot]

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

[javache]

This only works for React Native applications with a single root view, or where the rootview takes up the entire window frame.

useWindowsDimensions represents the size of the window/display your application is hosted in, not the size of your React root.

[jpdriver]

right -- but this PR doesn't change anything about RCTDimensions, they way we report them, or the useWindowDimensions hook..

• (if anyone reading through needs context, see RCTUIUtils.m#L12-L31)

all this PR does is dispatch the existing didUpdateDimensions events more often.. is there a problem doing that?

@javache would you prefer if we moved that postNotification somewhere other than setFrame? 🤔

[javache]

Ah, I misunderstood. You're using a root view frame change as a signal that the window dimensions might have changed (but you're not actually using the rootview's frame to update those dimensions!).

That makes more sense :) Ideally, yes, we'd find a place other than setFrame to call this from. Are there any better callbacks we could leverage for this?

[jpdriver]

You're using a root view frame change as a signal that the window dimensions might have changed

Yes, correct -- that's the intent here.

Are there any better callbacks we could leverage for this?

Some other things I learnt while looking into this:-

• UIKit views are not guaranteed to be KVO-compliant, so we can't observe the frame directly
• Overriding layoutSubviews seems to be generally accepted by the iOS community as the place for logic like this; though in the context of a React Native component tree I figured that probably gets called a lot? 😅

---

maybe layoutSubviews could be a better place, provided we track the old dimensions and do nothing unless they've actually changed?

this could be similar to how reactSetFrame works on RCTView

• see RCTView.m#L779-L789

@javache what do you think?

[javache]

reactSetFrame is just our wrapper to setFrame which is called as part of the mount.

What I meant with my question is: is there any other callback given to your application, other than the setFrame on the root, to signal its window dimenions have changed? E.g. if you break in the native debugger on that setFrame call, is there any higher-level hook (eg on AppDelegate) that we should hook into. If not, this approach seems fine.

[jpdriver]

is there any higher-level hook (eg on AppDelegate) that we should hook into

@javache I doubled checked but nope this is as high as we can get 🙇🏻‍♂️

is there anything else I need to do for this PR?

[javache]

is there any higher-level hook (eg on AppDelegate) that we should hook into

@javache I doubled checked but nope this is as high as we can get 🙇🏻‍♂️

is there anything else I need to do for this PR?

Could you expand the full stack there? Just curious if there's any other UIKit symbols there that could be relevant.

If nothing useful shows up there, I'm ok to pull this in.

[jpdriver]

I think this is all just UIKit innards right? 😅

[javache]

I think the right approach would be to hook into windowScene:didUpdateCoordinateSpace:interfaceOrientation:traitCollection: to get this information, however it doesn't look like we're setting up a WindowScene explicitly in the app template right now.

I'm sorry to push back on this, but we want to avoid having spurious events from across the codebase to set this piece of shared state. In an app with multiple rootviews, we'd be sending this event quite frequently for no valid reason.

Feel free to pull this setFrame handler in your RCTRootView subclass (eg. manually emitting RCTUserInterfaceStyleDidChangeNotification there)

[jpdriver]

it doesn't look like we're setting up a WindowScene explicitly in the app template right now

could introducing a SceneDelegate to the Hello World template be something the team would be open to? 🙏🏻

that would ultimately solve the issue with useWindowDimensions() for most RN apps (or at least the ones that use the OSS template!)

if that's of interest, I'd be happy to open a new PR to try it 🙇🏻‍♂️

[javache]

I think that's the right approach longer-term, but I don't know what the other implications of that are. @cipolleschi?

[cipolleschi]

Hi there, happy to chime in! :D

So, most of the logic for the template/AppDelegate is actually wrapped in the RCTAppDelegate on which we have full control.

So, whether we use the Scene based approach or the old approach, is now an implementation detail.

If we need to expose some method to the final users, for example if we want to allow them to customize some Scene specific logic, we would have to expose that in the RCTAppDelegate.h.

My only concern here is that, AFAIK, React Native is not set up to work with multiple Scenes, right now, so changing this could have implications on other OSS projects we are not aware of and we cannot control or check internally.

But Scene based apps are the new standard apple was pushing, so I'd say that yes, we should have a PR which promotes the Scene APIs in the RCTAppDelegate.

Possibly, we should split the PR in two:

1. Add the Scene support in the RCTAppDelegate
2. Fix the didUpdateDimensions issue.

How does this sound? Does it makes sense to you, guys?

[jpdriver]

ok so just to confirm: we'll introduce something like an RCTSceneDelegate first and then for

2. Fix the didUpdateDimensions issue.

we're saying we want to hook into windowScene:didUpdateCoordinateSpace:interfaceOrientation:traitCollection: instead of setFrame right?

[cipolleschi]

Why do you think that we need a RCTSceneDelegate? Can we use the same RCTAppDelegate as SceneDelegate? So the users won't have to deal with another class and they can override an RCTAppDelegate method if they have to customize the behavior.
How does this sound?

[jpdriver]

@cipolleschi ah I thought we'd need a separate RCTSceneDelegate from other modern-ish iOS apps where I've seen both an AppDelegate and a SceneDelegate.

I've updated this PR so the existing RCTAppDelegate itself is the UISceneDelegate too.

one minor nit about this approach -- it won't work for iOS < 13.0, whereas setFrame goes even further back in terms of compatibility..

should we pause work on this until 0.73 and the iOS minor version is set to 13? 🤔

[cipolleschi]

one minor nit about this approach -- it won't work for iOS < 13.0, whereas setFrame goes even further back in terms of compatibility..

should we pause work on this until 0.73 and the iOS minor version is set to 13? 🤔

Well spotted! If the issue is not pressing and can wait for 0.73, that would be the best option, probably.
Main is already on a 13.4 min requirement, so we can actually land this on main and it will be automatically included in 0.73.

If we need this in 0.72 also, we can send a local commit there to fix this behavior.

What do you think?

[jpdriver]

(accidentally closed because I didn't realize these commits were on main in my fork 🤦🏻‍♂️)

@cipolleschi can we land this? It would be great to have it in 0.73+

I don't think we need to worry too much about back-porting to other releases.

[cipolleschi]

@NickGerleman could you reimport this PR whenever you can? 🙏

[facebook-github-bot]

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

[github-actions]

This pull request was successfully merged by @jpdriver in 61861d2.

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

[Saadnajmi]

Chiming in after the fact, I appreciate the existence of this PR for React Native macOS as RCTDeviceInfo is where we have a lot of diffs trying to support RootView / it's window being as resizable as you want. Hopefully will make things better in 0.73 there too :D

[Saadnajmi]

Following up again, I have a few questions about this PR, as I'd like to expand on it for React Native macOS and/or make some changes to the iOS side:

1. Since RCTRootViewFrameDidChangeNotification is now actually signaling window changes, does it makes sense to rename the notification?

2. I see we moved away from overriding RCTRootView/RCTSurfaceHostingViews' setFrame method. However, it is possible for the root view's window size to change while the root view frame remains constant (namely on macOS, but also possibly visionOS). In cases as such, we probably want to still fire a RCTRootViewFrameDidChangeNotification style notification (maybe just... that one if we decide to rename it) so that RCTDeviceInfo can listen to it and useWindowDimensions() can update. Would we be open to re-introducing that override? @javache / @jpdriver FYI

^ Edit: I actually can't think of many cases where the root view size changes but the window doesn't in real world applications. And even such, we have a [RCTRootViewDelegate rootviewDidChangeIntrinsicSize] message we can subscribe to for that, so I think that's a moot point. It would be nice if we could listen to window size changes at the root view / instance level, but I understand the feedback if we don't want to override setFrame there.

For context, I currently emit the same notification from RCTRootContentView on React Native macOS, but would like to refactor that. https://github.com/microsoft/react-native-macos/blob/dbbfafc3b1931aa070f8dd131db14539bb3e539c/packages/react-native/React/Base/RCTRootContentView.m#L79C1-L82C1