Page MenuHomePhabricator
Differential D14307

[native] Migrate recenter functionality in FullScreenViewModal
ClosedPublic
Actions

Authored by angelika on Feb 6 2025, 5:17 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Apr 6, 4:02 AM
Unknown Object (File)
Sat, Apr 5, 9:51 PM
Unknown Object (File)
Sat, Apr 5, 4:47 PM
Unknown Object (File)
Sat, Apr 5, 9:10 AM
Unknown Object (File)
Sat, Apr 5, 6:56 AM
Unknown Object (File)
Fri, Apr 4, 10:26 AM
Unknown Object (File)
Fri, Apr 4, 5:24 AM
Unknown Object (File)
Thu, Apr 3, 4:44 AM
View All 57 Files
Subscribers
ashoat

Details

Reviewers
tomek
ashoat
Commits
rCOMM87db6a222d12: [native] Migrate recenter functionality in FullScreenViewModal
Summary

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

The recenter function triggers if there is no interaction (after some gesture ends and there is no animation). If the image is panned or zoomed out so there is some space left, the image is zoomed in to 1 (if necessary) and moved.

Depends on D14306

Test Plan

Tested after migration: verify the image is recentered after pan ended when zoomed out

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

angelika created this revision.Feb 6 2025, 5:17 PM
angelika held this revision as a draft.
Herald added a subscriber: ashoat. · View Herald TranscriptFeb 6 2025, 5:17 PM
angelika added a child revision: D14308: [native] Migrate fling functionality in FullScreenViewModal.Feb 6 2025, 5:39 PM
Harbormaster completed remote builds in B33422: Diff 46951.Feb 6 2025, 6:32 PM
angelika updated this revision to Diff 47085.Feb 12 2025, 8:52 PM

Rebase & bugfixes

angelika edited the summary of this revision. (Show Details)Feb 12 2025, 8:59 PM
angelika published this revision for review.Feb 12 2025, 9:09 PM
Harbormaster completed remote builds in B33501: Diff 47085.Feb 12 2025, 10:30 PM
tomek accepted this revision.Feb 17 2025, 3:52 AM
tomek added inline comments.
native/components/full-screen-view-modal.react.js
1179–1187 ↗(On Diff #47085)

It looks like in the original code this if was split into 3, for each value respectively. Should we do the same here?

This revision is now accepted and ready to land.Feb 17 2025, 3:52 AM
ashoat mentioned this in D14306: [native] Migrate backdropOpacityUpdate in FullScreenViewModal.Feb 19 2025, 5:51 PM
ashoat added inline comments.Feb 19 2025, 6:02 PM
native/components/full-screen-view-modal.react.js
1076 ↗(On Diff #47085)

I think "reset animation" is confusing naming here, as that could mean a lot of things. How about isRunningDoubleTapZoomAnimation?

1164 ↗(On Diff #47085)

In the old system, we would stop the recentering animations when activeInteraction flipped to true.

In the new system, when this happens due to pinchActive or isRunningResetAnimation, it looks like all three animations will be cancelled because new values are set for curScale, curX, and curY.

But in the case of panActive and isRunningDismissAnimation, it appears that a new value is not set for curScale. Does this mean the animation will continue? Do we need to cancel it some other way?

ashoat mentioned this in D14308: [native] Migrate fling functionality in FullScreenViewModal.Feb 19 2025, 8:36 PM
angelika updated this revision to Diff 47189.Feb 20 2025, 2:14 PM

Rebase & bugfixes

angelika marked 2 inline comments as done.Feb 20 2025, 3:06 PM
angelika planned changes to this revision.Feb 20 2025, 3:20 PM
angelika updated this revision to Diff 47201.Feb 21 2025, 8:07 PM

Rebase

This revision is now accepted and ready to land.Feb 21 2025, 8:07 PM
angelika added inline comments.Feb 21 2025, 8:25 PM
native/components/full-screen-view-modal.react.js
1164 ↗(On Diff #47085)

Copied the comment from other diff, because it also applies here:

I added cancelAnimation(curScale) when dismissing from pan. Now fling should be stopped when activeInteraction is true. In case activeInteraction is triggered by:

panActive turning to true: we explicitly cancel all animations in panStart
pinchActive turning to true: we explicitly cancel all animation in pinchStart
isRunningDismissAnimation: curX and curY are overridden and we explicitly cancel curScale
isRunningDoubleTapZoomAnimation: curX, curY and curScale are are overridden
This also applies to the recenter animation.

Harbormaster completed remote builds in B33581: Diff 47201.Feb 21 2025, 8:31 PM
ashoat accepted this revision.Feb 21 2025, 11:47 PM
angelika updated this revision to Diff 47223.Feb 24 2025, 3:52 AM

Rebase

Harbormaster completed remote builds in B33603: Diff 47223.Feb 24 2025, 6:08 AM
Closed by commit rCOMM87db6a222d12: [native] Migrate recenter functionality in FullScreenViewModal (authored by angelika). · Explain WhyFeb 25 2025, 2:48 AM
This revision was automatically updated to reflect the committed changes.
angelika added a commit: rCOMM87db6a222d12: [native] Migrate recenter functionality in FullScreenViewModal.

Revision Contents
Changeset List

PathSize
native/
components/
111 lines

Diff 47251

View Options

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

// @flow // @flow
import t from 'tcomb'; import t from 'tcomb';
import type { TInterface, TUnion } from 'tcomb'; import type { TInterface, TUnion } from 'tcomb';
import { mixedRawThreadInfoValidator } from 'lib/permissions/minimally-encoded-thread-permissions-validators.js'; import { mixedRawThreadInfoValidator } from 'lib/permissions/minimally-encoded-raw-thread-info-validators.js';
import { mediaValidator } from 'lib/types/media-types.js'; import { mediaValidator } from 'lib/types/media-types.js';
import { import {
rawMessageInfoValidator, rawMessageInfoValidator,
messageTruncationStatusesValidator, messageTruncationStatusesValidator,
} from 'lib/types/message-types.js'; } from 'lib/types/message-types.js';
import { userSurfacedPermissionValidator } from 'lib/types/thread-permission-types.js'; import { userSurfacedPermissionValidator } from 'lib/types/thread-permission-types.js';
import { threadTypes } from 'lib/types/thread-types-enum.js'; import { threadTypes } from 'lib/types/thread-types-enum.js';
import { import {
▲ Show 20 Lines • Show All 325 Lines • Show Last 20 Lines