search

software-mansion / react-native-gesture-handler

Declarative API exposing platform native touch and gesture system to React Native.
https://docs.swmansion.com/react-native-gesture-handler/
MIT License
6.13k stars 982 forks source link

Make osx example utilise the common example app #3055

Closed latekvo closed 1 month ago

latekvo commented 3 months ago

Description

This PR makes the MacOS example utilise the common example app. This change will allow for more in-depth testing of MacOS functionalities.

For now, it's still work in progress

Test plan

latekvo commented 3 months ago

I hope it won't have much conflicts with #2919 😅

I hope there won't be any, App.tsx should remain minimally changed with just a couple of styles differing, while the rest of the changes should be out of scope of #2919

Regardless, this PR is still WIP, and only works on my end due to some modifications to App.tsx structure, as I'm still working out some rendering inconsistencies, which are hopefully caused by default styling differences.

latekvo commented 3 months ago

As of now, the common examples are working on MacOS, and I am still working on completely removing the MacOSExample directory, but that may prove challenging as currently we're relying on the babel.config.js to make the entire app work on MacOS, which unfortunately would prevent other platforms from using the examples if we were to naively marge both of these folders in the state that they are.

image

latekvo commented 2 months ago

A quick update on progress and what'll happen next with this PR.

With the current state of MacOS support, I don't believe it is worth it to keep trying to make it work with Expo in this PR, it has proved a bit too challenging for what it does as it is currently not officially supported.

I've previously discussed this with @tsapeta and we couldn't find a working MacOS + Expo configuration.

I think the best course of action right now would be to revert progress back to somewhere around 02a96fa, and to cherry-pick some fixes that happened later, for example the build settings fix.

Summary

What has been done:

What couldn't be done:

(cc @m-bert)

m-bert commented 2 months ago

What has been done:

have a single app for both MacOS and the rest of the platforms

I think this is what we wanted to achieve 😅

What couldn't be done:

run both MacOS and iOS from a single xcode project load MacOS example off the same metro instance as Android, iOS and Web runs off

First one is definitely not a problem. As for second, I have not looked deeply into this configuration, but before you had to start separate metro server for web, so it is also not a big deal.

I think the best course of action right now would be to revert progress back to somewhere around 02a96fa, and to cherry-pick some fixes that happened later, for example the build settings fix.

Sounds good 😄

latekvo commented 2 months ago

PR functional as of 7717de6

currently only working on making the CI tests pass.

j-piasecki commented 2 months ago

At the moment the example app on iOS doesn't build: https://github.com/software-mansion/react-native-gesture-handler/actions/runs/10941427525. (I've also triggered Android just to be sure: https://github.com/software-mansion/react-native-gesture-handler/actions/runs/10941919130).

Also, assets are not loading in the macOS app:

Screenshot 2024-09-19 at 15 19 55

I would assume this isn't expected, but I also wasn't following the PR that closely.

latekvo commented 2 months ago

At the moment the example app on iOS doesn't build

I'm working on it. It's caused by presence of react-native-macos package in example, which we cannot remove just now.

Also, assets are not loading in the macOS app I would assume this isn't expected, but I also wasn't following the PR that closely.

As far as I know, image assets are not available on macos. I've tested this by trying to load png images into Image components, while their presence was detected by require, none showed up. This is likely caused by asset linking issues on macos, i think the existing scripts have iOS paths hardcoded when they're trying to copy assets from project files to xcode Resources folder, but i'd have to doublecheck to be sure. This is most likely the case for custom font assets, but I might be incorrectly associating image loading issues to the same problem.

I'll keep you updated.

(cc @j-piasecki)

j-piasecki commented 2 months ago

As far as I know, image assets are not available on macos. I've tested this by trying to load png images into Image components, while their presence was detected by require, none showed up.

Were the images under the example app or MacOSExample? This may be caused by the fact that you're effectively loading files from outside the app since it's not a monorepo.

Regarding failing Android build, there are multiple versions of @react-native/gradle-plugin in the project

Screenshot 2024-09-19 at 16 31 21

Vs on main:

Screenshot 2024-09-19 at 16 35 10

It may be related (it's possible that iOS failures are caused by a similar problem, though I don't remember any package that could cause it there from the top of my head).

latekvo commented 2 months ago

Regarding failing Android build, there are multiple versions of @react-native/gradle-plugin in the project

Since these problems are caused by react-native and react-native-macos discrepancy, we'll have to wait for this issue to get resolved, it's blocking us from bumping react-native-macos to 0.74.

latekvo commented 2 months ago

@j-piasecki Image loading fixed in 2d62e3a and 896f9be.

image image

latekvo commented 2 months ago

since 747bb71 iOS and Android should now both build successfully. I've ran these two [Android, iOS] workflows just to be sure:

Test Android build Test iOS build

latekvo commented 1 month ago

https://github.com/user-attachments/assets/da53c5d3-ec4b-4283-a852-e576dc811f9e

Fixed header being squashed in these commits: e3c098d, 6a6f885, 38f861d (cherry-picked from this branch)

latekvo commented 1 month ago

Removed chevrons in 61455cf, couldn't find a way to properly load their custom font. Tried:

All other known bugs have been resolved by now as far as i know.

Please let me know if there're any other changes you'd like to see, or if it's ok to merge this PR.

m-bert commented 1 month ago

Okay, I've looked into Draggable example and it seems to be the problem with NativeViewGestureHandler (something else than I expected). I'll dive into it later.

Also what I've noticed is that sometimes app gets stuck at bundling overlay:

image