Page MenuHomePhabricator
Differential D14312

[native] Migrate animated styles to Reanimated v2 API in FullScreenViewModal
ClosedPublic
Actions

Authored by angelika on Feb 6 2025, 5:46 PM.
Tags
None
Referenced Files
Unknown Object (File)
Thu, Mar 20, 4:57 PM
Unknown Object (File)
Wed, Mar 19, 4:55 AM
Unknown Object (File)
Wed, Mar 19, 4:53 AM
Unknown Object (File)
Wed, Mar 19, 4:47 AM
Unknown Object (File)
Wed, Mar 19, 4:46 AM
Unknown Object (File)
Wed, Mar 19, 3:32 AM
Unknown Object (File)
Wed, Mar 19, 3:30 AM
Unknown Object (File)
Mon, Mar 17, 11:06 PM
View All 71 Files
Subscribers
ashoat

Details

Reviewers
tomek
Commits
rCOMM0452eeb23353: [native] Migrate animated styles to Reanimated v2 API in FullScreenViewModal
Summary

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

  • move styles and calculating styles to useAnimatedStyle
  • use positionV2 instead of position from overlay context

Also from this diff on the component should work correctly.

Depends on D14311

Test Plan

Verify the image is zoomed in/out, panned correctly.
Verify the buttons have correct opacity.
Verify the backdrop has correct opacity.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

angelika created this revision.Feb 6 2025, 5:46 PM
angelika held this revision as a draft.
Herald added a subscriber: ashoat. · View Herald TranscriptFeb 6 2025, 5:46 PM
angelika added a child revision: D14313: [native] Migrate render function and finish FullScreenViewModal migration to functional component.Feb 6 2025, 5:48 PM
Harbormaster completed remote builds in B33427: Diff 46956.Feb 6 2025, 7:18 PM
angelika added inline comments.Feb 10 2025, 1:10 AM
native/components/full-screen-view-modal.react.js
834–835 ↗(On Diff #46956)

You can't use props object in a worklet, because it will cause a weird crash (Cannot add new property '__reanimatedHostObjectRef'). So we destructure it here.

angelika edited the summary of this revision. (Show Details)Feb 10 2025, 1:11 AM
angelika edited the summary of this revision. (Show Details)
angelika updated this revision to Diff 47090.Feb 12 2025, 8:54 PM

Rebase & bugfixes

angelika published this revision for review.Feb 12 2025, 9:11 PM
Harbormaster completed remote builds in B33506: Diff 47090.Feb 12 2025, 11:20 PM
tomek accepted this revision.Feb 17 2025, 4:18 AM
tomek added inline comments.
native/components/full-screen-view-modal.react.js
887 ↗(On Diff #47090)

Nit

This revision is now accepted and ready to land.Feb 17 2025, 4:18 AM
ashoat added inline comments.Feb 19 2025, 8:49 PM
native/components/full-screen-view-modal.react.js
895–902 ↗(On Diff #47090)

Nit

904–922 ↗(On Diff #47090)

Could probably be simplified to avoid the return keyboard here too, but it's more subjective

angelika updated this revision to Diff 47194.Feb 20 2025, 2:16 PM

Rebase & bugfixes

angelika marked 2 inline comments as done.Feb 20 2025, 3:19 PM
angelika added inline comments.
native/components/full-screen-view-modal.react.js
904–922 ↗(On Diff #47090)

I prefer the version with return. It's longer but imho more readable.

Harbormaster failed remote builds in B33575: Diff 47194!Feb 20 2025, 6:06 PM
angelika updated this revision to Diff 47206.Feb 21 2025, 8:08 PM

Rebase

Harbormaster failed remote builds in B33586: Diff 47206!Feb 21 2025, 8:29 PM
angelika updated this revision to Diff 47212.Feb 21 2025, 10:15 PM

Fix rebase

Harbormaster completed remote builds in B33592: Diff 47212.Feb 21 2025, 11:10 PM
angelika updated this revision to Diff 47228.Feb 24 2025, 3:54 AM

Rebase

This revision was landed with ongoing or failed builds.Feb 25 2025, 2:48 AM
Closed by commit rCOMM0452eeb23353: [native] Migrate animated styles to Reanimated v2 API in FullScreenViewModal (authored by angelika). · Explain Why
This revision was automatically updated to reflect the committed changes.
angelika added a commit: rCOMM0452eeb23353: [native] Migrate animated styles to Reanimated v2 API in FullScreenViewModal.

Revision Contents
Changeset List

PathSize
native/
components/
256 lines

Diff 47256

View Options

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

Show All 32 Lines (threadInfo, messageStore) => {
if (!thread || !viewerID || thread.messageIDs.length === 0) { if (!thread || !viewerID || thread.messageIDs.length === 0) {
return { return {
lastUpdatedAtLeastTime: threadInfo.creationTime, lastUpdatedAtLeastTime: threadInfo.creationTime,
lastUpdatedAtMostTime: threadInfo.creationTime, lastUpdatedAtMostTime: threadInfo.creationTime,
lastUpdatedTime: () => Promise.resolve(threadInfo.creationTime), lastUpdatedTime: () => Promise.resolve(threadInfo.creationTime),
}; };
} }
const mostRecentMessage = messageStore.messages[thread.messageIDs[0]];
const lastUpdatedAtMostTime =
mostRecentMessage?.time ?? threadInfo.creationTime;
// We default to the most recent message time. // We default to the most recent message time.
// - This default will only be relevant in the case that // - This default will only be relevant in the case that
// getLastUpdatedTime is implemented for every message in the loop // getLastUpdatedTime is implemented for every message in the loop
// below, and doesn't return a number for any of them. Otherwise // below, and doesn't return a number for any of them. Otherwise
// lastUpdatedAtLeastTime will be set to the most recent message that // lastUpdatedAtLeastTime will be set to the most recent message that
// returns a number, or whose message spec doesn't implement // returns a number, or whose message spec doesn't implement
// getLastUpdatedTime. // getLastUpdatedTime.
// - We default to the most recent message time because the best // - We default to the most recent message time because the best
// alternative is the thread creation time, but this is likely very // alternative is the thread creation time, but this is likely very
// far in the past. // far in the past.
const mostRecentMessage = messageStore.messages[thread.messageIDs[0]]; let lastUpdatedAtLeastTime = lastUpdatedAtMostTime;
let lastUpdatedAtLeastTime =
mostRecentMessage?.time ?? threadInfo.creationTime;
const getLastUpdatedTimeParams = { const getLastUpdatedTimeParams = {
threadInfo, threadInfo,
viewerID, viewerID,
getMessageAuthor: (messageID: string) => getMessageAuthor: (messageID: string) =>
baseGetMessageAuthor(messageID, messageStore), baseGetMessageAuthor(messageID, messageStore),
}; };
let lastUpdatedTime: ?() => Promise<?number>; let lastUpdatedTime: ?() => Promise<?number>;
let highestTimestamp: ?number;
for (const messageID of thread.messageIDs) { for (const messageID of thread.messageIDs) {
const messageInfo = messageStore.messages[messageID]; const messageInfo = messageStore.messages[messageID];
if (!messageInfo) { if (!messageInfo) {
continue; continue;
} }
// We call getLastUpdatedTime on the message spec. It can return either // We call getLastUpdatedTime on the message spec. It can return either
// ?number or () => Promise<?number>. If the message spec doesn't // ?number or () => Promise<?number>. If the message spec doesn't
// implement getLastUpdatedTime, then we default to messageInfo.time. // implement getLastUpdatedTime, then we default to messageInfo.time.
const { getLastUpdatedTime } = messageSpecs[messageInfo.type]; const { getLastUpdatedTime } = messageSpecs[messageInfo.type];
const lastUpdatedTimeResult = getLastUpdatedTime const lastUpdatedTimeResult = getLastUpdatedTime
? getLastUpdatedTime(messageInfo, getLastUpdatedTimeParams) ? getLastUpdatedTime(messageInfo, getLastUpdatedTimeParams)
: messageInfo.time; : messageInfo.time;
// We only need to consider the first positive number result because
// thread.messageIDs is ordered chronologically.
if (
!highestTimestamp &&
lastUpdatedTimeResult &&
typeof lastUpdatedTimeResult === 'number'
) {
highestTimestamp = lastUpdatedTimeResult;
}
// We rely on the fact that thread.messageIDs is ordered chronologically // We rely on the fact that thread.messageIDs is ordered chronologically
// (newest first) to chain together lastUpdatedTime. An older message's // (newest first) to chain together lastUpdatedTime. An older message's
// lastUpdatedTime is only considered if all of the newer messages // lastUpdatedTime is only considered if all of the newer messages
// return falsey. // return falsey.
const prevLastUpdatedTime = lastUpdatedTime; const prevLastUpdatedTime = lastUpdatedTime;
lastUpdatedTime = async () => { lastUpdatedTime = async () => {
if (prevLastUpdatedTime) { if (prevLastUpdatedTime) {
const earlierChecks = await prevLastUpdatedTime(); const earlierChecks = await prevLastUpdatedTime();
Show All 27 Lines (threadInfo, messageStore) => {
const earlierChecks = await lastUpdatedTime(); const earlierChecks = await lastUpdatedTime();
if (earlierChecks) { if (earlierChecks) {
return earlierChecks; return earlierChecks;
} }
} }
return lastUpdatedAtLeastTime; return lastUpdatedAtLeastTime;
}; };
const lastUpdatedAtMostTime = highestTimestamp ?? threadInfo.creationTime;
return { return {
lastUpdatedAtLeastTime, lastUpdatedAtLeastTime,
lastUpdatedAtMostTime, lastUpdatedAtMostTime,
lastUpdatedTime: lastUpdatedWithFallback, lastUpdatedTime: lastUpdatedWithFallback,
}; };
}, },
[viewerID, baseGetMessageAuthor], [viewerID, baseGetMessageAuthor],
); );
} }
export { useGetLastUpdatedTimes }; export { useGetLastUpdatedTimes };