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, 6:29 AM

Rebase & review changes

angelika added inline comments.Jan 21 2025, 4:45 PM

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 21 2025, 4:53 PM

tomek accepted this revision.Jan 22 2025, 2: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, 2:39 AM

Review changes

Harbormaster completed remote builds in B33318: Diff 46783.Jan 22 2025, 7:35 AM

Closed by commit rCOMMc6b74e27e8d7: [native] Migrate orientation logic in CameraModal to functional component (authored by angelika). · Explain WhyJan 22 2025, 9:39 AM

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, 2:32 PM

Revision Contents
Changeset List

Path

Size

native/

media/

camera-modal.react.js

48 lines

Diff 46784

View Options

native/media/camera-modal.react.js

Show First 20 Lines • Show All 90 Lines • ▼ Show 20 Lines	Ok(Self {
.try_into()		.try_into()
.expect("Channels vec size doesn't match array size"),		.expect("Channels vec size doesn't match array size"),
})		})
}		}

fn get_channel(		fn get_channel(
&self,		&self,
id_hash: impl std::hash::Hash,		id_hash: impl std::hash::Hash,
) -> Result<lapin::Channel, lapin::Error> {		) -> Option<lapin::Channel> {
let channel_idx = Self::channel_idx_for_hash(id_hash);		let channel_idx = Self::channel_idx_for_hash(id_hash);
let channel = self.channels[channel_idx].clone();		let channel = self.channels[channel_idx].clone();
let channel_id = channel.id();		let channel_id = channel.id();
tracing::trace!(channel_id, channel_idx, "Retrieving AMQP Channel");		tracing::trace!(channel_id, channel_idx, "Retrieving AMQP Channel");
Ok(channel)
		if channel.status().connected() {
		return Some(channel);
		}
		warn!(channel_id, channel_idx, "Channel is dead!");
		None
		}

		async fn reset_channel(
		&mut self,
		id_hash: impl std::hash::Hash,
		) -> Result<lapin::Channel, lapin::Error> {
		let channel_idx = Self::channel_idx_for_hash(id_hash);

		let existing_channel = &self.channels[channel_idx];
		if existing_channel.status().connected() {
		return Ok(existing_channel.clone());
		}

		let new_channel = self.conn.create_channel().await?;
		debug!(
		"Channel for idx={channel_idx} was recreated, new Id={}",
		new_channel.id()
		);
		self.channels[channel_idx] = new_channel.clone();
		Ok(new_channel)
}		}

fn is_connected(&self) -> bool {		fn is_connected(&self) -> bool {
self.conn.status().connected()		self.conn.status().connected()
}		}

fn channel_idx_for_hash(id_hash: impl std::hash::Hash) -> usize {		fn channel_idx_for_hash(id_hash: impl std::hash::Hash) -> usize {
// We have channel pool and want to distribute them between connected		// We have channel pool and want to distribute them between connected
Show All 22 Lines	pub async fn connect() -> Result<Self, lapin::Error> {
let inner = Arc::new(RwLock::new(conn));		let inner = Arc::new(RwLock::new(conn));
info!("Connected to AMQP endpoint: {}", &CONFIG.amqp_uri);		info!("Connected to AMQP endpoint: {}", &CONFIG.amqp_uri);

Ok(Self { inner })		Ok(Self { inner })
}		}

pub async fn channel(		pub async fn channel(
&self,		&self,
id_hash: impl std::hash::Hash,		id_hash: impl std::hash::Hash + Clone,
) -> Result<lapin::Channel, lapin::Error> {		) -> Result<lapin::Channel, lapin::Error> {
if !self.is_connected().await {		if !self.is_connected().await {
warn!("AMQP disconnected while retrieving channel");		warn!("AMQP disconnected while retrieving channel");
self.reset_conn().await?;		self.reset_conn().await?;
}		}
self.inner.read().await.get_channel(id_hash)
		{
		let inner = self.inner.read().await;
		if let Some(channel) = inner.get_channel(id_hash.clone()) {
		return Ok(channel);
		}
		}

		self.inner.write().await.reset_channel(id_hash).await
}		}

async fn reset_conn(&self) -> Result<(), lapin::Error> {		async fn reset_conn(&self) -> Result<(), lapin::Error> {
debug!("Resetting connection...");		debug!("Resetting connection...");
let mut inner = self.inner.write().await;		let mut inner = self.inner.write().await;
if !inner.is_connected() {		if !inner.is_connected() {
let new_conn = ConnectionInner::new().await?;		let new_conn = ConnectionInner::new().await?;
*inner = new_conn;		*inner = new_conn;
▲ Show 20 Lines • Show All 41 Lines • Show Last 20 Lines

[native] Migrate orientation logic in CameraModal to functional componentClosedPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 46784

native/media/camera-modal.react.js

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

Revision Contents
Changeset List