Page MenuHomePhabricator
Differential D14309

[native] Cancel animations when gesture is started in FullScreenViewModal
ClosedPublic
Actions

Authored by angelika on Feb 6 2025, 5:42 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Apr 16, 3:27 PM
Unknown Object (File)
Wed, Apr 16, 1:37 AM
Unknown Object (File)
Wed, Apr 16, 12:07 AM
Unknown Object (File)
Tue, Apr 15, 10:56 PM
Unknown Object (File)
Tue, Apr 15, 10:20 PM
Unknown Object (File)
Tue, Apr 15, 10:08 PM
Unknown Object (File)
Tue, Apr 15, 8:07 PM
Unknown Object (File)
Tue, Apr 15, 7:28 PM
View All 71 Files
Subscribers
ashoat

Details

Reviewers
tomek
Commits
rCOMM3e75cc6b16f4: [native] Cancel animations when gesture is started in FullScreenViewModal
Summary

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

As mentioned here: https://phab.comm.dev/D14305 in the original implementation there are a couple of places that stop animation if gesture is active. In this diff I added this logic.

The builds will fail due to some leftover reanimated v1 code. I'll remove it in the next diffs for diffs readability.

Depends on D14308

Test Plan

Test after migration: try to start a gesture when animation is in progress

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

angelika created this revision.Feb 6 2025, 5:42 PM
angelika held this revision as a draft.
Herald added a subscriber: ashoat. · View Herald TranscriptFeb 6 2025, 5:42 PM
angelika added a child revision: D14310: [native] Remove leftover Reanimated v1 code in FullScreenViewModal.Feb 6 2025, 5:42 PM
Harbormaster failed remote builds in B33424: Diff 46953!Feb 6 2025, 5:54 PM
angelika edited the summary of this revision. (Show Details)Feb 10 2025, 1:02 AM
angelika updated this revision to Diff 47087.Feb 12 2025, 8:53 PM

Rebase & bugfixes

angelika published this revision for review.Feb 12 2025, 9:10 PM
angelika edited the summary of this revision. (Show Details)
Harbormaster failed remote builds in B33503: Diff 47087!Feb 12 2025, 9:21 PM
tomek accepted this revision.Feb 17 2025, 4:00 AM
tomek mentioned this in D14308: [native] Migrate fling functionality in FullScreenViewModal.

Worth checking if this diff causes an issue from a comment https://phab.comm.dev/D14308#399134.

This revision is now accepted and ready to land.Feb 17 2025, 4:01 AM
ashoat mentioned this in D14305: [native] Migrate double tap gesture handling in FullScreenViewModal.Feb 19 2025, 4:34 PM
ashoat added a comment.Feb 19 2025, 4:42 PM

I'd be surprised if this diff was the cause, as here we cancel animations at the start of a pan or pinch, but in the video we see an issue at the conclusion of a pan.

angelika updated this revision to Diff 47191.Feb 20 2025, 2:15 PM

Rebase & bugfixes

Harbormaster failed remote builds in B33572: Diff 47191!Feb 20 2025, 2:42 PM
angelika updated this revision to Diff 47203.Feb 21 2025, 8:08 PM

Rebase

Harbormaster failed remote builds in B33583: Diff 47203!Feb 21 2025, 8:23 PM
angelika updated this revision to Diff 47209.Feb 21 2025, 10:14 PM

Fix rebase

Harbormaster failed remote builds in B33589: Diff 47209!Feb 21 2025, 10:28 PM
angelika updated this revision to Diff 47225.Feb 24 2025, 3:53 AM

Rebase

Harbormaster failed remote builds in B33605: Diff 47225!Feb 24 2025, 4:19 AM
This revision was landed with ongoing or failed builds.Feb 25 2025, 2:48 AM
Closed by commit rCOMM3e75cc6b16f4: [native] Cancel animations when gesture is started in FullScreenViewModal (authored by angelika). · Explain Why
This revision was automatically updated to reflect the committed changes.
angelika added a commit: rCOMM3e75cc6b16f4: [native] Cancel animations when gesture is started in FullScreenViewModal.

Revision Contents
Changeset List

PathSize
native/
components/
20 lines

Diff 47225

View Options

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

Show First 20 Lines • Show All 155 Lines • ▼ Show 20 Lines
}; };
type Props = { type Props = {
...BaseProps, ...BaseProps,
// Redux state // Redux state
+dimensions: DerivedDimensionsInfo, +dimensions: DerivedDimensionsInfo,
// withOverlayContext // withOverlayContext
+overlayContext: ?OverlayContextType, +overlayContext: ?OverlayContextType,
+isActive: boolean, +isActive: boolean,
};
type State = {
+closeButtonEnabled: boolean, +closeButtonEnabled: boolean,
+actionLinksEnabled: boolean, +actionLinksEnabled: boolean,
}; +updateCloseButtonEnabled: ([number]) => void,
class FullScreenViewModal extends React.PureComponent<Props, State> { +updateActionLinksEnabled: ([number]) => void,
state: State = {
closeButtonEnabled: true,
actionLinksEnabled: true,
}; };
class FullScreenViewModal extends React.PureComponent<Props> {
closeButton: ?React.ElementRef<TouchableOpacityInstance>; closeButton: ?React.ElementRef<TouchableOpacityInstance>;
mediaIconsContainer: ?React.ElementRef<typeof View>; mediaIconsContainer: ?React.ElementRef<typeof View>;
closeButtonX: Value = new Value(-1); closeButtonX: Value = new Value(-1);
closeButtonY: Value = new Value(-1); closeButtonY: Value = new Value(-1);
closeButtonWidth: Value = new Value(0); closeButtonWidth: Value = new Value(0);
closeButtonHeight: Value = new Value(0); closeButtonHeight: Value = new Value(0);
closeButtonLastState: Value = new Value(1); closeButtonLastState: Value = new Value(1);
mediaIconsX: Value = new Value(-1); mediaIconsX: Value = new Value(-1);
▲ Show 20 Lines • Show All 472 Lines • ▼ Show 20 Lines return block([
], ],
), ),
), ),
set(this.actionLinksLastState, actionLinksState), set(this.actionLinksLastState, actionLinksState),
set(this.closeButtonLastState, closeButtonState), set(this.closeButtonLastState, closeButtonState),
set(wasZoomed, isZoomed), set(wasZoomed, isZoomed),
set(lastTapX, singleTapX), set(lastTapX, singleTapX),
set(lastTapY, singleTapY), set(lastTapY, singleTapY),
call([eq(curCloseButtonOpacity, 1)], this.setCloseButtonEnabled), call([eq(curCloseButtonOpacity, 1)], this.props.updateCloseButtonEnabled),
call([eq(curActionLinksOpacity, 1)], this.setActionLinksEnabled), call([eq(curActionLinksOpacity, 1)], this.props.updateActionLinksEnabled),
]); ]);
} }
doubleTapUpdate( doubleTapUpdate(
// Inputs // Inputs
doubleTapState: Node, doubleTapState: Node,
doubleTapX: Node, doubleTapX: Node,
doubleTapY: Node, doubleTapY: Node,
▲ Show 20 Lines • Show All 316 Lines • ▼ Show 20 Lines const mediaIconsButtonStyle = {
opacity: this.actionLinksOpacity, opacity: this.actionLinksOpacity,
}; };
let saveButton; let saveButton;
if (saveContentCallback) { if (saveContentCallback) {
saveButton = ( saveButton = (
<TouchableOpacity <TouchableOpacity
onPress={saveContentCallback} onPress={saveContentCallback}
disabled={!this.state.actionLinksEnabled} disabled={!this.props.actionLinksEnabled}
style={styles.mediaIconButtons} style={styles.mediaIconButtons}
> >
<SWMansionIcon name="save" style={styles.mediaIcon} /> <SWMansionIcon name="save" style={styles.mediaIcon} />
<Text style={styles.mediaIconText}>Save</Text> <Text style={styles.mediaIconText}>Save</Text>
</TouchableOpacity> </TouchableOpacity>
); );
} }
let copyButton; let copyButton;
if (Platform.OS === 'ios' && copyContentCallback) { if (Platform.OS === 'ios' && copyContentCallback) {
copyButton = ( copyButton = (
<TouchableOpacity <TouchableOpacity
onPress={copyContentCallback} onPress={copyContentCallback}
disabled={!this.state.actionLinksEnabled} disabled={!this.props.actionLinksEnabled}
style={styles.mediaIconButtons} style={styles.mediaIconButtons}
> >
<SWMansionIcon name="copy" style={styles.mediaIcon} /> <SWMansionIcon name="copy" style={styles.mediaIcon} />
<Text style={styles.mediaIconText}>Copy</Text> <Text style={styles.mediaIconText}>Copy</Text>
</TouchableOpacity> </TouchableOpacity>
); );
} }
Show All 26 Lines const view = (
</View> </View>
<SafeAreaView style={styles.buttonsOverlay}> <SafeAreaView style={styles.buttonsOverlay}>
<View style={styles.fill}> <View style={styles.fill}>
<Animated.View <Animated.View
style={[styles.closeButtonContainer, closeButtonStyle]} style={[styles.closeButtonContainer, closeButtonStyle]}
> >
<TouchableOpacity <TouchableOpacity
onPress={this.close} onPress={this.close}
disabled={!this.state.closeButtonEnabled} disabled={!this.props.closeButtonEnabled}
onLayout={this.onCloseButtonLayout} onLayout={this.onCloseButtonLayout}
ref={this.closeButtonRef} ref={this.closeButtonRef}
> >
<Text style={styles.closeButton}>×</Text> <Text style={styles.closeButton}>×</Text>
</TouchableOpacity> </TouchableOpacity>
</Animated.View> </Animated.View>
{mediaActionButtons} {mediaActionButtons}
</View> </View>
▲ Show 20 Lines • Show All 41 Lines • ▼ Show 20 Lines return (
</PinchGestureHandler> </PinchGestureHandler>
); );
} }
close = () => { close = () => {
this.props.navigation.goBackOnce(); this.props.navigation.goBackOnce();
}; };
setCloseButtonEnabled = ([enabledNum]: [number]) => {
const enabled = !!enabledNum;
if (this.state.closeButtonEnabled !== enabled) {
this.setState({ closeButtonEnabled: enabled });
}
};
setActionLinksEnabled = ([enabledNum]: [number]) => {
const enabled = !!enabledNum;
if (this.state.actionLinksEnabled !== enabled) {
this.setState({ actionLinksEnabled: enabled });
}
};
closeButtonRef = ( closeButtonRef = (
closeButton: ?React.ElementRef<typeof TouchableOpacity>, closeButton: ?React.ElementRef<typeof TouchableOpacity>,
) => { ) => {
this.closeButton = (closeButton: any); this.closeButton = (closeButton: any);
}; };
mediaIconsRef = (mediaIconsContainer: ?React.ElementRef<typeof View>) => { mediaIconsRef = (mediaIconsContainer: ?React.ElementRef<typeof View>) => {
this.mediaIconsContainer = mediaIconsContainer; this.mediaIconsContainer = mediaIconsContainer;
▲ Show 20 Lines • Show All 116 Lines • ▼ Show 20 Lines ) {
}, [isActive]); }, [isActive]);
React.useEffect(() => { React.useEffect(() => {
return () => { return () => {
Orientation.lockToPortrait(); Orientation.lockToPortrait();
}; };
}, []); }, []);
const [closeButtonEnabled, setCloseButtonEnabled] = React.useState(true);
const [actionLinksEnabled, setActionLinksEnabled] = React.useState(true);
const updateCloseButtonEnabled = React.useCallback(
([enabledNum]: [number]) => {
tomekUnsubmitted
Not Done
Inline Actions

We should prefer Array type

tomek: We should prefer `Array` type
angelikaAuthorUnsubmitted
Done
Inline Actions

This was just copied from the original code and the [number] argument will be replaced with number argument in https://phab.comm.dev/D14304 anyway

angelika: This was just copied from the original code and the `[number]` argument will be replaced with…
const enabled = !!enabledNum;
if (closeButtonEnabled !== enabled) {
setCloseButtonEnabled(enabled);
}
},
[closeButtonEnabled],
);
const updateActionLinksEnabled = React.useCallback(
([enabledNum]: [number]) => {
const enabled = !!enabledNum;
if (actionLinksEnabled !== enabled) {
setActionLinksEnabled(enabled);
}
},
[actionLinksEnabled],
);
return ( return (
<FullScreenViewModal <FullScreenViewModal
{...props} {...props}
dimensions={dimensions} dimensions={dimensions}
overlayContext={overlayContext} overlayContext={overlayContext}
isActive={isActive} isActive={isActive}
closeButtonEnabled={closeButtonEnabled}
actionLinksEnabled={actionLinksEnabled}
updateCloseButtonEnabled={updateCloseButtonEnabled}
updateActionLinksEnabled={updateActionLinksEnabled}
/> />
); );
}); });
export default ConnectedFullScreenViewModal; export default ConnectedFullScreenViewModal;