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: add visionOS support #2025

Merged
merged 6 commits into from
Feb 15, 2024
Merged

Conversation

okwasniewski
Copy link
Contributor

@okwasniewski okwasniewski commented Feb 1, 2024

Description

This PR adds visionOS support. Changes are mostly around the same places where tvOS had ifdefs.

Changes

  • Added visionos support

Screenshots / GIFs

CleanShot.2024-01-30.at.15.59.15.mp4

Test code and steps to reproduce

  1. I've initialized new visionOS project inside of the repo and linked it (same as TVOS example)
  2. react-native-safearea-context supports visionOS from version 4.9.0

Note: I didn't add new visionOS example (as they are many of them already) but soon react-native-test-app will support visionOS so that might be a solution to test visionOS in the long run

Checklist

  • Ensured that CI passes

@tboba
Copy link
Member

tboba commented Feb 12, 2024

@okwasniewski Hi there, thank you for this PR! 🎉
I've managed to create a repro with the Example app and unfortunately I've found some things:

  1. On entering the screen I'm getting the error from assertViewControllerBasedStatusBarAppearenceSet method (RNSScreenWindowTraits). I believe that ifdefing that method should be enough (or maybe we should leave it as it is? Does VisionOS actually have a status bar? 😅). I'm also having the same error on iOS Simulator in the same project, so maybe it's my fault that I see this error (I haven't changed Info.plist file), but I don't know if we should assert the status bar on VisionOS.
8mb.video-WD2-m2XIZwK3.mp4
  1. I don't know if the transparent modal should look like on the video below. Usually, transparent modals are used to appear with the previous UIView kept in the hierarchy, behind the modal view - Unlike contained transparent modal, which are working correctly in this case, I can't see same thing with transparent modals 🤔 what do you think about it? Could this be a bug in the VisionOS itself? Just for the context, transparent modals are displayed as UIModalPresentationOverFullScreen.
8mb.video-M7Q-pq1t4gem.mp4
  1. After clicking on the title of the screen, its style changes (to the default? I think).
Screen.Recording.2024-02-12.at.18.44.51.mov

And I think that's mostly all! Beside those things, I am quite astonished that screen animations and search bar actually works! Great job!
If you need the repro with the screens example on VisionOS, I can publish it!

@okwasniewski
Copy link
Contributor Author

Hey @tboba, Thanks for taking the time to check this PR. It would be great if you could share the repo 🙏

@tboba
Copy link
Member

tboba commented Feb 13, 2024

@okwasniewski cool! Here's the repo: https://github.com/tboba/screens-visionos-example
I've removed some of the functionalities (restarting the app, gesture-handler) to make this app working very quickly, so I believe only the Gestures and Go Back Animation will not work there

@okwasniewski
Copy link
Contributor Author

@tboba

visionOS doesn't have status bar so I added ifdefs to this method 😅 The error might be silenced after adding this key to Info.plist but I don't think it makes sense to show this warning.

I saw the same behaviour in RN Core - Im not sure why it is showing this way but it looks more like a bug in visionOS than in screens. Let's leave it as is for now and adjust when/if Apple provides a better way to do it.

Hah this is interesting, I think Apple didn't intended to change for visionOS.. Maybe we can show a warning when user attempts to change it?

@tboba
Copy link
Member

tboba commented Feb 13, 2024

@okwasniewski I'm concerned if we should allow changing the header's style by clicking (or tapping? I only heard that the visionOS's cursor is being controlled by the eye :D) on it at all 😅

@okwasniewski
Copy link
Contributor Author

@tboba I meant we should ignore changing header color for visionOS (it should be always set to white). I guess what's happening is that the view refreshes when the user taps on it.

Let's show warning or just ignore changing color when someone (developer) attempts to change it.

@tboba
Copy link
Member

tboba commented Feb 13, 2024

@okwasniewski, yeah, I would also assume that 😄
Yeah, in case of changing title's color let's throw a warning and ignore setting the style for VisionOS 👍

@okwasniewski
Copy link
Contributor Author

@tboba I added ifdefs to ignore changing the header title color, I found that adding warnings was really annoying as they were triggered all the time - I think it will be worth adding to documentation that changing color is ignored for this platform

@tboba
Copy link
Member

tboba commented Feb 14, 2024

@okwasniewski hmm, on the JS side we have the package with warnOnce method that will throw the warn only once in the whole app runtime when the given condition is met. You can find the examples of the usage of it in NativeStackView.tsx file. What do you think about it? Maybe it is worth trying? I believe you will need to use that method in HeaderConfig.tsx file

@okwasniewski
Copy link
Contributor Author

@tboba I've added warnOnce() to HeaderConfig.tsx 👍🏻

ios/RNSScreenStackHeaderConfig.mm Outdated Show resolved Hide resolved
@okwasniewski
Copy link
Contributor Author

@tboba Not sure why Android e2e is failing but it looks like a flaky e2e test

@tboba
Copy link
Member

tboba commented Feb 14, 2024

@okwasniewski Yup, unfortunately Github Runners are not that performant to assert that some of the components from the example are visible, resulting in test fails 😕 I'll run this test once or twice and it should be green!

Copy link
Member

@tboba tboba left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks!

Copy link
Member

@tomekzaw tomekzaw left a comment

Choose a reason for hiding this comment

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

LGTM!

@tboba tboba merged commit 0a10a07 into software-mansion:main Feb 15, 2024
6 checks passed
ja1ns pushed a commit to WiseOwlTech/react-native-screens that referenced this pull request Oct 9, 2024
## Description

This PR adds visionOS support. Changes are mostly around the same places
where tvOS had `ifdefs`.

## Changes

- Added `visionos` support

## Screenshots / GIFs

https://github.com/software-mansion/react-native-screens/assets/52801365/7618d537-1e31-4cde-9610-da109295d800

## Test code and steps to reproduce

1. I've initialized new visionOS project inside of the repo and linked
it (same as TVOS example)
2. `react-native-safearea-context` supports visionOS from version
`4.9.0`

_Note: I didn't add new visionOS example (as they are many of them
already) but soon `react-native-test-app` will support visionOS so that
might be a solution to test visionOS in the long run_

## Checklist

- [x] Ensured that CI passes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants