Paths

Table of Contentst

Differential D14221

[native] Migrate orientation logic in CameraModal to functional component
ClosedPublic
Actions

Authored by angelika on Jan 20 2025, 12:09 PM.

Tags

None

Referenced Files

None

Subscribers

Details

Reviewers

Commits

rCOMMc6b74e27e8d7: [native] Migrate orientation logic in CameraModal to functional component

Summary

https://linear.app/comm/issue/ENG-8152/convert-cameramodal-to-reanimated-2-syntax

When entering camera modal, we unlock the orientation.
When exiting camera modal, we lock the orientation to portrait.

Test Plan

Verify that orientation is handled correctly when showing camera modal:

when entering in horizontal orientation it's switched to horizontal
when exiting in horizontal orientation it's locked to portrait

Diff Detail

Repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

angelika created this revision.Jan 20 2025, 12:09 PM

angelika held this revision as a draft.

Herald added a subscriber: ashoat. · View Herald TranscriptJan 20 2025, 12:09 PM

angelika added a child revision: D14222: [native] Migrate flashMode in CameraModal to functional component.Jan 20 2025, 12:11 PM

Harbormaster completed remote builds in B33265: Diff 46718.Jan 20 2025, 12:27 PM

angelika published this revision for review.Jan 20 2025, 1:24 PM

tomek requested changes to this revision.Jan 20 2025, 2:29 PM

tomek added inline comments.

native/media/camera-modal.react.js
1181–1187 ↗	(On Diff #46718)	Is it guaranteed that the orientation gets locked on unmount? I think we should return a function from this effect that does that when appropriate.

This revision now requires changes to proceed.Jan 20 2025, 2:29 PM

Rebase & review changes

angelika added inline comments.Jan 22 2025, 12:45 AM

native/media/camera-modal.react.js
1181–1187 ↗	(On Diff #46718)	From my testing it looks like the orientation is locked because isActive turns to false before modal is closed. But just in case I added the code to lock the orientation on unmount,

Harbormaster completed remote builds in B33294: Diff 46751.Jan 22 2025, 12:53 AM

tomek accepted this revision.Jan 22 2025, 10:39 AM

tomek added inline comments.

native/media/camera-modal.react.js
1181–1187 ↗	(On Diff #46718)	The added code is correct but is a bit tricky. The cleanup function will be run before every dependency change. So there are two cases: When `isActive` becomes true, we will lock the orientation in the cleanup, and then unlock it in the effect. When `isActive` becomes false, we will lock the orientation in the cleanup, and then lock again in the effect. So eventually, the state will be correct, but maybe we should add an `if` in the cleanup checking `isActive`?

This revision is now accepted and ready to land.Jan 22 2025, 10:39 AM

Review changes

Harbormaster completed remote builds in B33318: Diff 46783.Jan 22 2025, 3:35 PM

Closed by commit rCOMMc6b74e27e8d7: [native] Migrate orientation logic in CameraModal to functional component. · Explain WhyJan 22 2025, 5:39 PM

This revision was automatically updated to reflect the committed changes.

angelika added a commit: rCOMMc6b74e27e8d7: [native] Migrate orientation logic in CameraModal to functional component.

angelika mentioned this in D14298: [native] Migrate orientation logic in FullScreenViewModal to functional component.Feb 20 2025, 10:32 PM

Revision Contents
Changeset List

Path

Size

native/

media/

camera-modal.react.js

48 lines

Diff 46784

native/media/camera-modal.react.js

Loading...