Page MenuHomePhabricator
Differential D14305

[native] Migrate double tap gesture handling in FullScreenViewModal
ClosedPublic
Actions

Authored by angelika on Feb 6 2025, 5:12 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Apr 15, 8:41 AM
Unknown Object (File)
Tue, Apr 15, 8:10 AM
Unknown Object (File)
Tue, Apr 15, 2:27 AM
Unknown Object (File)
Tue, Apr 15, 1:34 AM
Unknown Object (File)
Tue, Apr 15, 12:14 AM
Unknown Object (File)
Mon, Apr 14, 8:40 PM
Unknown Object (File)
Mon, Apr 14, 4:26 AM
Unknown Object (File)
Sun, Mar 30, 10:53 AM
View All 50 Files
Subscribers
ashoat

Details

Reviewers
tomek
Commits
rCOMMe0028f81cbb3: [native] Migrate double tap gesture handling in FullScreenViewModal
Summary

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

Migrated doubleTapUpdate, getHorizontalPanSpace, getVerticalPanSpace methods.

On double tap:

  • if the current scale is > 1 then animate scale to 1 and recenter the image with timing animation
  • otherwise animate scale to 3 and recenter the image with timing animation

The math was basically copied from the original implementation, just rewritten from v1 (nodes) to v2 api (worklets).

There is one more thing: in the original implementation there is logic to stop the animation if a gesture is active. In the methods migrated in the next diffs (backdropOpacity, recenter, fling) there are also a couple of places that stop the animations when gesture is active. V2 has function cancelAnimation that cancels any animation on a shared value, so I'm going to use it later in a separate diff to cancel all animations from double tap, recenter etc.

Depends on D14304

Test Plan

Tested after migration: verify the image is zoomed in/out after a double tap.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

angelika created this revision.Feb 6 2025, 5:12 PM
angelika held this revision as a draft.
Herald added a subscriber: ashoat. · View Herald TranscriptFeb 6 2025, 5:12 PM
angelika added a child revision: D14306: [native] Migrate backdropOpacityUpdate in FullScreenViewModal.Feb 6 2025, 5:15 PM
Harbormaster completed remote builds in B33420: Diff 46949.Feb 6 2025, 6:13 PM
angelika edited the summary of this revision. (Show Details)Feb 7 2025, 5:58 AM
angelika mentioned this in D14309: [native] Cancel animations when gesture is started in FullScreenViewModal.Feb 10 2025, 1:02 AM
angelika updated this revision to Diff 47083.Feb 12 2025, 8:52 PM

Rebase & bugfixes

angelika published this revision for review.Feb 12 2025, 9:07 PM
Harbormaster completed remote builds in B33499: Diff 47083.Feb 12 2025, 10:10 PM
tomek accepted this revision.Feb 17 2025, 3:39 AM
This revision is now accepted and ready to land.Feb 17 2025, 3:39 AM
ashoat added inline comments.Feb 19 2025, 4:28 PM
native/components/full-screen-view-modal.react.js
1193–1195 ↗(On Diff #47083)

It seems the old system was designed to work with other animations happening at the same time:

  1. Instead of just assigning the animation result to curScale, curX, and curY, we seem to have calculated the deltas, and then applied them directly. This would allow us to combine the results of this animation with another currently running one. How complicated would adding support for something like this be?
  2. In the old code, it seems that we would stop the animation if a pinch or pan gesture became active. This could be achieved now by adding something to pinchStart and panStart, right? Is it too complicated to try and support?

As in prior work, I'd like to reemphasize that it's your responsibility as the diff author to identify changes you've made from previous behavior, and to highlight and explain them to the diff reviewer.

ashoat added inline comments.Feb 19 2025, 4:31 PM
native/components/full-screen-view-modal.react.js
1193–1195 ↗(On Diff #47083)

Regarding 2, I suppose that pinchUpdate and panUpdate handle most of this, by setting curX, curY, and curScale.

The only difference is that in panUpdate, we don't set curScale. Should we call cancelAnimation(curScale) in panStart? (Might make sense to add other cancelAnimations for clarity, but will defer to you on that.)

ashoat added inline comments.Feb 19 2025, 4:34 PM
native/components/full-screen-view-modal.react.js
1193–1195 ↗(On Diff #47083)

Upon further review, 2 is handled in D14309.

Only remaining question is part 1 here

angelika updated this revision to Diff 47187.Feb 20 2025, 2:13 PM

Rebase & bugfixes

angelika added inline comments.Feb 20 2025, 2:55 PM
native/components/full-screen-view-modal.react.js
1193–1195 ↗(On Diff #47083)

About part 1:
Adding support for this would be complicated as there is no simple way of running a couple animations at once but consider that this is an animation after a double click. What other animation can be run while/after user double clicks the image? To double click user needs to stop doing anything for a moment and any animation will finish in the meantime.

About part 2, I've mentioned it in the description of this diff:

There is one more thing: in the original implementation there is logic to stop the animation if a gesture is active. In the methods migrated in the next diffs (backdropOpacity, recenter, fling) there are also a couple of places that stop the animations when gesture is active. V2 has function cancelAnimation that cancels any animation on a shared value, so I'm going to use it later in a separate diff to cancel all animations from double tap, recenter etc.

ashoat added a comment.Feb 20 2025, 9:45 PM

Adding support for this would be complicated as there is no simple way of running a couple animations at once but consider that this is an animation after a double click. What other animation can be run while/after user double clicks the image? To double click user needs to stop doing anything for a moment and any animation will finish in the meantime.

This is a fair point. Thanks for explaining!

About part 2, I've mentioned it in the description of this diff:

Sorry – my apologies for missing this.

angelika updated this revision to Diff 47221.Feb 24 2025, 3:52 AM

Rebase

Harbormaster failed remote builds in B33601: Diff 47221!Feb 24 2025, 5:51 AM
This revision was landed with ongoing or failed builds.Feb 25 2025, 2:48 AM
Closed by commit rCOMMe0028f81cbb3: [native] Migrate double tap gesture handling in FullScreenViewModal (authored by angelika). · Explain Why
This revision was automatically updated to reflect the committed changes.
angelika added a commit: rCOMMe0028f81cbb3: [native] Migrate double tap gesture handling in FullScreenViewModal.
angelika mentioned this in rCOMM3e75cc6b16f4: [native] Cancel animations when gesture is started in FullScreenViewModal.

Revision Contents
Changeset List

PathSize
native/
components/
204 lines

Diff 47249

View Options

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

Show All 21 Lines
import { SafeAreaView } from 'react-native-safe-area-context'; import { SafeAreaView } from 'react-native-safe-area-context';
import { useIsLoggedInToAuthoritativeKeyserver } from 'lib/hooks/account-hooks.js'; import { useIsLoggedInToAuthoritativeKeyserver } from 'lib/hooks/account-hooks.js';
import { setActiveSessionRecoveryActionType } from 'lib/keyserver-conn/keyserver-conn-types.js'; import { setActiveSessionRecoveryActionType } from 'lib/keyserver-conn/keyserver-conn-types.js';
import { usePersistedStateLoaded } from 'lib/selectors/app-state-selectors.js'; import { usePersistedStateLoaded } from 'lib/selectors/app-state-selectors.js';
import { isLoggedIn } from 'lib/selectors/user-selectors.js'; import { isLoggedIn } from 'lib/selectors/user-selectors.js';
import { recoveryFromReduxActionSources } from 'lib/types/account-types.js'; import { recoveryFromReduxActionSources } from 'lib/types/account-types.js';
import { useDispatch } from 'lib/utils/redux-utils.js'; import { useDispatch } from 'lib/utils/redux-utils.js';
import { usingCommServicesAccessToken } from 'lib/utils/services-utils.js'; import {
usingCommServicesAccessToken,
usingRestoreFlow,
} from 'lib/utils/services-utils.js';
import { splashBackgroundURI } from './background-info.js'; import { splashBackgroundURI } from './background-info.js';
import FullscreenSIWEPanel from './fullscreen-siwe-panel.react.js'; import FullscreenSIWEPanel from './fullscreen-siwe-panel.react.js';
import LogInPanel from './log-in-panel.react.js'; import LogInPanel from './log-in-panel.react.js';
import type { LogInState } from './log-in-panel.react.js'; import type { LogInState } from './log-in-panel.react.js';
import LoggedOutStaffInfo from './logged-out-staff-info.react.js'; import LoggedOutStaffInfo from './logged-out-staff-info.react.js';
import { authoritativeKeyserverID } from '../authoritative-keyserver.js'; import { authoritativeKeyserverID } from '../authoritative-keyserver.js';
import KeyboardAvoidingView from '../components/keyboard-avoiding-view.react.js'; import KeyboardAvoidingView from '../components/keyboard-avoiding-view.react.js';
▲ Show 20 Lines • Show All 452 Lines • ▼ Show 20 Lines const buttonsViewStyle = React.useMemo(
[styles.buttonContainer, buttonsViewOpacity], [styles.buttonContainer, buttonsViewOpacity],
); );
const buttons = React.useMemo(() => { const buttons = React.useMemo(() => {
if (mode.curMode !== 'prompt') { if (mode.curMode !== 'prompt') {
return null; return null;
} }
const signInButtons = []; const signInButtons = [];
if (!usingRestoreFlow) {
signInButtons.push( signInButtons.push(
<TouchableOpacity <TouchableOpacity
onPress={onPressLogIn} onPress={onPressLogIn}
style={classicAuthButtonStyle} style={classicAuthButtonStyle}
activeOpacity={0.6} activeOpacity={0.6}
key="login-form" key="login-form"
> >
<Text style={classicAuthButtonTextStyle}>Sign in</Text> <Text style={classicAuthButtonTextStyle}>Sign in</Text>
</TouchableOpacity>, </TouchableOpacity>,
); );
if (__DEV__) { }
if (__DEV__ || usingRestoreFlow) {
const buttonText = usingRestoreFlow ? 'Sign in' : 'Sign in (QR)';
signInButtons.push( signInButtons.push(
<TouchableOpacity <TouchableOpacity
onPress={onPressQRCodeSignIn} onPress={onPressQRCodeSignIn}
style={classicAuthButtonStyle} style={classicAuthButtonStyle}
activeOpacity={0.6} activeOpacity={0.6}
key="qr-code-login" key="qr-code-login"
> >
<Text style={classicAuthButtonTextStyle}>Sign in (QR)</Text> <Text style={classicAuthButtonTextStyle}>{buttonText}</Text>
</TouchableOpacity>, </TouchableOpacity>,
); );
} }
return ( let siweSection = null;
<AnimatedView style={buttonsViewStyle}> if (!usingRestoreFlow) {
<LoggedOutStaffInfo /> siweSection = (
<>
<TouchableOpacity <TouchableOpacity
onPress={onPressSIWE} onPress={onPressSIWE}
style={siweAuthButtonStyle} style={siweAuthButtonStyle}
activeOpacity={0.6} activeOpacity={0.6}
> >
<View style={styles.siweIcon}> <View style={styles.siweIcon}>
<EthereumLogo /> <EthereumLogo />
</View> </View>
<Text style={siweAuthButtonTextStyle}>Sign in with Ethereum</Text> <Text style={siweAuthButtonTextStyle}>Sign in with Ethereum</Text>
</TouchableOpacity> </TouchableOpacity>
<View style={styles.siweOr}> <View style={styles.siweOr}>
<View style={styles.siweOrLeftHR} /> <View style={styles.siweOrLeftHR} />
<Text style={styles.siweOrText}>or</Text> <Text style={styles.siweOrText}>or</Text>
<View style={styles.siweOrRightHR} /> <View style={styles.siweOrRightHR} />
</View> </View>
</>
);
}
return (
<AnimatedView style={buttonsViewStyle}>
<LoggedOutStaffInfo />
{siweSection}
<View style={styles.signInButtons}>{signInButtons}</View> <View style={styles.signInButtons}>{signInButtons}</View>
<View style={styles.registerButtons}> <View style={styles.registerButtons}>
<TouchableOpacity <TouchableOpacity
onPress={onPressNewRegister} onPress={onPressNewRegister}
style={classicAuthButtonStyle} style={classicAuthButtonStyle}
activeOpacity={0.6} activeOpacity={0.6}
key="new" key="new"
> >
▲ Show 20 Lines • Show All 110 Lines • Show Last 20 Lines