Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: allow custom assignment of rootView to rootViewController #37873

Closed
wants to merge 4 commits into from
Closed

feat: allow custom assignment of rootView to rootViewController #37873

wants to merge 4 commits into from

Conversation

Gregoirevda
Copy link
Contributor

@Gregoirevda Gregoirevda commented Jun 14, 2023

Summary:

To use a native Drawer on iPad, I can override createRootViewController to create a UISplitViewController instead of a UIViewController, but I then need to assign the rootView with

[splitViewController setViewController:mainVC forColumn:UISplitViewControllerColumnSecondary];

which I can currently only do by copy pasting the entire didFinishLaunchingWithOptions and only replacing the assignment

    rootViewController.view = rootView;

In an attempt of making it easier for developers to use a native drawer in iOS, being able to override the assignment would make it easier.

Changelog:

[iOS] [ADDED] - added override method with default implementation

Test Plan

Tested on iPad iOS 16 simulator

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 14, 2023
@analysis-bot
Copy link

analysis-bot commented Jun 14, 2023

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,755,253 -2,417
android hermes armeabi-v7a 8,067,827 -2,409
android hermes x86 9,247,854 -2,410
android hermes x86_64 9,096,988 -2,417
android jsc arm64-v8a 9,316,459 -2,412
android jsc armeabi-v7a 8,506,358 -2,414
android jsc x86 9,379,947 -2,416
android jsc x86_64 9,633,190 -2,416

Base commit: b0485be
Branch: main

Copy link
Contributor

@cipolleschi cipolleschi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Gregoirevda! Thank you so much for this PR. I'm super excited that you took some time to add this new feature to RCTAppDelegate. 😄

I left a few comments to align it more to standard iOS conventions, but overall the approach looks good to me!

@Gregoirevda
Copy link
Contributor Author

Hi @cipolleschi
Thanks for the review, should be fine now with changes applied

Copy link
Contributor

@cipolleschi cipolleschi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good to me! Thanks again for the enhancement!

@facebook-github-bot
Copy link
Contributor

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

@github-actions
Copy link

This pull request was successfully merged by @Gregoirevda in abc6e1c.

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

@github-actions github-actions bot added the Merged This PR has been merged. label Jun 15, 2023
@facebook-github-bot
Copy link
Contributor

@cipolleschi merged this pull request in abc6e1c.

adamaveray added a commit to adamaveray/react-native that referenced this pull request Jun 16, 2023
* main: (135 commits)
  translation auto-update for i18n/twilight.config.json on master
  Interop: Introduce Bridge proxy
  Remove okhttp internal util usage (facebook#37843)
  Update debian to fix CI while updating Node (facebook#37841)
  fix: foreground ripple crash on api < 23 (facebook#37901)
  Re-add the top level LICENSE file (facebook#37916)
  Deploy 0.209.0 to xplat (facebook#37921)
  Re-enable direct debugging with JSC on iOS 16.4+ (facebook#37914)
  add emitObjectProp in parser primitives (facebook#37904)
  Make React-utils its own pod (facebook#37659)
  feat: allow custom assignment of rootView to rootViewController (facebook#37873)
  Switch xplat prettier config to hermes plugin (facebook#37915)
  Set iOS AppState to inactive when app is launching (facebook#37690)
  Use `fileExists` in replace_hermes script (facebook#37911)
  (docs): fix license url (facebook#37909)
  Revert D46719890: Re-enable direct debugging with JSC on iOS 16.4+
  Re-enable direct debugging with JSC on iOS 16.4+ (facebook#37874)
  Fix component type references in xplat (facebook#37903)
  Remove usage of passthroughAnimatedPropExplicitValues in ScrollViewStickyHeader (facebook#37867)
  test runtime lifecycle callback (facebook#37897)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants