Page MenuHomePhabricator
Differential D14313

[native] Migrate render function and finish FullScreenViewModal migration to functional component
ClosedPublic
Actions

Authored by angelika on Feb 6 2025, 5:48 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Mar 30, 5:59 PM
Unknown Object (File)
Sun, Mar 30, 6:56 AM
Unknown Object (File)
Sat, Mar 29, 4:31 PM
Unknown Object (File)
Sat, Mar 29, 4:18 AM
Unknown Object (File)
Fri, Mar 28, 11:29 PM
Unknown Object (File)
Fri, Mar 28, 4:31 PM
Unknown Object (File)
Sun, Mar 16, 6:14 PM
Unknown Object (File)
Sun, Mar 16, 5:42 PM
View All 72 Files
Subscribers
ashoat

Details

Reviewers
tomek
ashoat
Commits
rCOMM5a00b30209f9: [native] Migrate render function and finish FullScreenViewModal migration to…
Summary

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

Just copied the render function to the functional component with some trivial changes.
Renamed ConnectedFullScreenViewModal -> FullScreenViewModal
Renamed BaseProps -> Props

Depends on D14312

Test Plan

Verify the component still works.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

angelika created this revision.Feb 6 2025, 5:48 PM
angelika held this revision as a draft.
Herald added a subscriber: ashoat. · View Herald TranscriptFeb 6 2025, 5:48 PM
Harbormaster completed remote builds in B33428: Diff 46957.Feb 6 2025, 7:27 PM
angelika edited the summary of this revision. (Show Details)Feb 10 2025, 1:15 AM
angelika updated this revision to Diff 47091.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 B33507: Diff 47091.Feb 12 2025, 11:30 PM
tomek accepted this revision.Feb 17 2025, 4:22 AM
tomek added inline comments.
native/components/full-screen-view-modal.react.js
83 ↗(On Diff #47091)

I think we usually define styles below the components

154–155 ↗(On Diff #47091)

We usually define the component without the memorization and then, just before exporting it, we memoize

This revision is now accepted and ready to land.Feb 17 2025, 4:22 AM
ashoat accepted this revision.Feb 19 2025, 8:59 PM

A lovely feeling to get to the end of the stack here. Great work on all of this, @angelika!!

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

Rebase & bugfixes

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

Rebase & bugfixes

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

Rebase & bugfixes

angelika marked 2 inline comments as done.Feb 20 2025, 3:20 PM
Harbormaster failed remote builds in B33578: Diff 47197!Feb 20 2025, 6:34 PM
ashoat mentioned this in D14301: [native] Migrate buttons refs and measuring in FullScreenViewModal.Feb 20 2025, 9:58 PM
angelika updated this revision to Diff 47207.Feb 21 2025, 8:08 PM

Rebase

Harbormaster completed remote builds in B33587: Diff 47207.Feb 21 2025, 9:36 PM
angelika updated this revision to Diff 47213.Feb 21 2025, 10:15 PM

Fix rebase

Harbormaster completed remote builds in B33593: Diff 47213.Feb 21 2025, 11:23 PM
angelika updated this revision to Diff 47229.Feb 24 2025, 3:54 AM

Rebase

Harbormaster failed remote builds in B33609: Diff 47229!Feb 24 2025, 7:28 AM
This revision was landed with ongoing or failed builds.Feb 25 2025, 2:48 AM
Closed by commit rCOMM5a00b30209f9: [native] Migrate render function and finish FullScreenViewModal migration to… (authored by angelika). · Explain Why
This revision was automatically updated to reflect the committed changes.
angelika added a commit: rCOMM5a00b30209f9: [native] Migrate render function and finish FullScreenViewModal migration to….

Revision Contents
Changeset List

PathSize
native/
components/
1498 lines

Diff 47257

View Options

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

Loading...