Page MenuHomePhabricator
Differential D14300

[native] Migrate gestures to react-native-gesture-handler v2 API in FullScreenViewModal
ClosedPublic
Actions

Authored by angelika on Feb 6 2025, 5:05 PM.
Tags
None
Referenced Files
F6240555: D14300.diff
Thu, Apr 24, 11:02 PM
Unknown Object (File)
Wed, Apr 16, 3:14 PM
Unknown Object (File)
Wed, Apr 16, 11:11 AM
Unknown Object (File)
Wed, Apr 16, 10:07 AM
Unknown Object (File)
Wed, Apr 16, 9:51 AM
Unknown Object (File)
Wed, Apr 16, 9:42 AM
Unknown Object (File)
Wed, Apr 16, 6:03 AM
Unknown Object (File)
Wed, Apr 16, 4:59 AM
View All 56 Files
Subscribers
ashoat

Details

Reviewers
tomek
Commits
rCOMM35649a6fa388: [native] Migrate gestures to react-native-gesture-handler v2 API in…
Summary

https://linear.app/comm/issue/ENG-8150/convert-fullscreenviewmodal-to-reanimated-2-syntax

Replace old gesture handlers API with new GestureDetector API. We can simultaneously handle pan and pinch gestures, but they're exclusive with taps.

At this point the component is broken and will be gradually fixed in the next diffs.

Depends on D14299

Test Plan

Tested gestures after migration.

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

angelika created this revision.Feb 6 2025, 5:05 PM
angelika held this revision as a draft.
Herald added a subscriber: ashoat. · View Herald TranscriptFeb 6 2025, 5:05 PM
angelika added a child revision: D14301: [native] Migrate buttons refs and measuring in FullScreenViewModal.Feb 6 2025, 5:07 PM
Harbormaster completed remote builds in B33415: Diff 46944.Feb 6 2025, 5:28 PM
angelika edited the summary of this revision. (Show Details)Feb 7 2025, 5:16 AM
angelika edited the test plan for this revision. (Show Details)
angelika updated this revision to Diff 47078.Feb 12 2025, 8:50 PM

Rebase & bugfixes

angelika published this revision for review.Feb 12 2025, 9:05 PM
Harbormaster completed remote builds in B33494: Diff 47078.Feb 12 2025, 9:21 PM
tomek accepted this revision.Feb 13 2025, 10:36 AM
tomek added inline comments.
native/components/full-screen-view-modal.react.js
1229–1240

This doesn't seem to depend on any of the props or state - can we define it outside the component?

native/flow-typed/npm/react-native-gesture-handler_v2.x.x.js
823–829

Should this be readonly and exact?

This revision is now accepted and ready to land.Feb 13 2025, 10:36 AM
ashoat added inline comments.Feb 13 2025, 10:30 PM
native/components/full-screen-view-modal.react.js
1236

We were previously using Gesture.Simultaneous for all of the gestures, so this is changing something. But I guess it makes sense... the taps can't be simultaneous with other gestures.

native/flow-typed/npm/react-native-gesture-handler_v2.x.x.js
823–829

If we're not confident that the list is exhaustive, then we can consider inexact (...) instead of exact ({| |}). In libdefs, it's best to be unambiguous about these two (ie. avoid ambiguous { } syntax) since different Flow environments can pick between these two defaults (we default to exact).

I'd also note that Array<Gesture> would probably be better represented by $ReadOnlyArray<Gesture>

angelika updated this revision to Diff 47182.Feb 20 2025, 2:12 PM

Rebase & bugfixes

angelika marked 3 inline comments as done.Feb 20 2025, 2:41 PM
angelika added inline comments.
native/components/full-screen-view-modal.react.js
1229–1240

This doesn't depend yet, it will depend in future diffs on callbacks like panStart, pinchEnd, etc.

angelika updated this revision to Diff 47216.Feb 24 2025, 3:50 AM

Rebase

Harbormaster completed remote builds in B33596: Diff 47216.Feb 24 2025, 4:48 AM
Closed by commit rCOMM35649a6fa388: [native] Migrate gestures to react-native-gesture-handler v2 API in… (authored by angelika). · Explain WhyFeb 25 2025, 2:48 AM
This revision was automatically updated to reflect the committed changes.
angelika added a commit: rCOMM35649a6fa388: [native] Migrate gestures to react-native-gesture-handler v2 API in….

Revision Contents
Changeset List

PathSize
native/
components/
56 lines
flow-typed/
npm/
5 lines

Diff 47078

View Options

native/components/full-screen-view-modal.react.js

locals { locals {
identity_service_image_tag = local.is_staging ? "0.41" : "0.41" identity_service_image_tag = local.is_staging ? "0.42" : "0.41"
identity_service_server_image = "commapp/identity-server:${local.identity_service_image_tag}" identity_service_server_image = "commapp/identity-server:${local.identity_service_image_tag}"
identity_service_container_name = "identity-server" identity_service_container_name = "identity-server"
# Port that the container is listening on # Port that the container is listening on
identity_service_container_grpc_port = 50054 identity_service_container_grpc_port = 50054
identity_sc_port_name = "identity-service-ecs-grpc" identity_sc_port_name = "identity-service-ecs-grpc"
identity_sc_dns_name = "identity-service" identity_sc_dns_name = "identity-service"
▲ Show 20 Lines • Show All 301 Lines • Show Last 20 Lines
View Options

native/flow-typed/npm/react-native-gesture-handler_v2.x.x.js

locals { locals {
identity_service_image_tag = local.is_staging ? "0.41" : "0.41" identity_service_image_tag = local.is_staging ? "0.42" : "0.41"
identity_service_server_image = "commapp/identity-server:${local.identity_service_image_tag}" identity_service_server_image = "commapp/identity-server:${local.identity_service_image_tag}"
identity_service_container_name = "identity-server" identity_service_container_name = "identity-server"
# Port that the container is listening on # Port that the container is listening on
identity_service_container_grpc_port = 50054 identity_service_container_grpc_port = 50054
identity_sc_port_name = "identity-service-ecs-grpc" identity_sc_port_name = "identity-service-ecs-grpc"
identity_sc_dns_name = "identity-service" identity_sc_dns_name = "identity-service"
▲ Show 20 Lines • Show All 301 Lines • Show Last 20 Lines