From 984a98d181fbd8c92b88eb7b7e2f20bd4632662f Mon Sep 17 00:00:00 2001 From: Alex Hart Date: Wed, 26 Aug 2026 14:42:53 -0300 Subject: [PATCH] Fix lock button in locking state. --- .../org/signal/core/ui/compose/SignalIcons.kt | 1 + .../main/res/drawable/symbol_lock_fill_24.xml | 9 +++++++ .../org/signal/camera/hud/CaptureButton.kt | 18 ++++++++++--- .../signal/camera/hud/CaptureButtonState.kt | 9 +++++-- .../camera/hud/RecordingActionButtons.kt | 27 ++++++++++++++++--- .../signal/camera/hud/StandardCameraHud.kt | 11 +++++++- .../camera/hud/CaptureButtonStateTest.kt | 13 +++++++++ 7 files changed, 77 insertions(+), 11 deletions(-) create mode 100644 core/ui/src/main/res/drawable/symbol_lock_fill_24.xml diff --git a/core/ui/src/main/java/org/signal/core/ui/compose/SignalIcons.kt b/core/ui/src/main/java/org/signal/core/ui/compose/SignalIcons.kt index 44d84572a9..cafcf4dab8 100644 --- a/core/ui/src/main/java/org/signal/core/ui/compose/SignalIcons.kt +++ b/core/ui/src/main/java/org/signal/core/ui/compose/SignalIcons.kt @@ -69,6 +69,7 @@ enum class SignalIcons(private val icon: SignalIcon) : SignalIcon by icon { Keyboard(icon(R.drawable.ic_keyboard_24)), Link(icon(R.drawable.symbol_link_24)), Lock(icon(R.drawable.symbol_lock_24)), + LockFill(icon(R.drawable.symbol_lock_fill_24)), Maximize(icon(R.drawable.symbol_maximize_24)), Mic(icon(R.drawable.symbol_mic_24)), MobileNextDisplay(icon(R.drawable.symbol_mobile_next_display_48)), diff --git a/core/ui/src/main/res/drawable/symbol_lock_fill_24.xml b/core/ui/src/main/res/drawable/symbol_lock_fill_24.xml new file mode 100644 index 0000000000..f9b296bac3 --- /dev/null +++ b/core/ui/src/main/res/drawable/symbol_lock_fill_24.xml @@ -0,0 +1,9 @@ + + + diff --git a/feature/camera/src/main/java/org/signal/camera/hud/CaptureButton.kt b/feature/camera/src/main/java/org/signal/camera/hud/CaptureButton.kt index 4c3aea44c9..ccfa9c78ec 100644 --- a/feature/camera/src/main/java/org/signal/camera/hud/CaptureButton.kt +++ b/feature/camera/src/main/java/org/signal/camera/hud/CaptureButton.kt @@ -171,6 +171,9 @@ private val CaptureButtonState.innerShape: CaptureButtonInnerShape * @param onLongPressEnd Callback when long press ends (video recording stop) * @param onZoomChange Callback for zoom level changes during recording (0f to 1f) * @param onLock Callback when a drag has reached the lock, asking for the recording to run unheld + * @param onOverLockChanged Callback when a drag arrives over the lock or leaves it again, so the lock can show that + * lifting there is what takes it. The circle this button carries there arrives underneath it, so the lock is what is + * seen at the end of the drag. * @param lockOffset Where the lock sits relative to this button's center, in pixels of this button's own frame. * [Offset.Zero] for a recording that has no lock to be dragged to. * @param modifier Modifier to be applied to the button @@ -183,6 +186,7 @@ fun CaptureButton( onLongPressEnd: () -> Unit, onZoomChange: (Float) -> Unit, onLock: () -> Unit = {}, + onOverLockChanged: (Boolean) -> Unit = {}, lockOffset: Offset = Offset.Zero, modifier: Modifier = Modifier ) { @@ -200,6 +204,7 @@ fun CaptureButton( val currentOnLongPressEnd by rememberUpdatedState(onLongPressEnd) val currentOnZoomChange by rememberUpdatedState(onZoomChange) val currentOnLock by rememberUpdatedState(onLock) + val currentOnOverLockChanged by rememberUpdatedState(onOverLockChanged) val currentLockOffset by rememberUpdatedState(lockOffset) // A drag toward the lock takes the shape part of the way to what it will be once it gets there, so the button shows @@ -299,10 +304,14 @@ fun CaptureButton( val wasOverLock = overLock overLock = isOverLock(pointer.position, wasOverLock) - // Taking hold is felt as it happens, so the finger knows it has arrived without having to commit to find - // out. Only the crossing plays, not every event that follows it. - if (overLock && !wasOverLock) { - haptics.performHapticFeedback(LockSnapHaptic) + // Taking hold is felt as it happens, and the lock is told so it can show it, so the finger knows it has + // arrived without having to commit to find out. Only the crossing is reported, not every event after it. + if (overLock != wasOverLock) { + currentOnOverLockChanged(overLock) + + if (overLock) { + haptics.performHapticFeedback(LockSnapHaptic) + } } if (!pointer.pressed) { @@ -347,6 +356,7 @@ fun CaptureButton( } finally { isPressed = false lockProgress = 0f + currentOnOverLockChanged(false) } } }, diff --git a/feature/camera/src/main/java/org/signal/camera/hud/CaptureButtonState.kt b/feature/camera/src/main/java/org/signal/camera/hud/CaptureButtonState.kt index e75bdfe460..96df136b08 100644 --- a/feature/camera/src/main/java/org/signal/camera/hud/CaptureButtonState.kt +++ b/feature/camera/src/main/java/org/signal/camera/hud/CaptureButtonState.kt @@ -55,11 +55,16 @@ enum class CaptureButtonState { enum class GallerySlotContent { GALLERY, LOCK, + + /** The lock with a thumb over it, which says that lifting there is what takes it. */ + LOCK_ENGAGED, PAUSE; companion object { - fun of(captureButtonState: CaptureButtonState): GallerySlotContent = when (captureButtonState) { - CaptureButtonState.RECORDING_HELD -> LOCK + + /** @param isOverLock Whether a drag from the capture button has reached the lock, which only it has any use for. */ + fun of(captureButtonState: CaptureButtonState, isOverLock: Boolean = false): GallerySlotContent = when (captureButtonState) { + CaptureButtonState.RECORDING_HELD -> if (isOverLock) LOCK_ENGAGED else LOCK CaptureButtonState.RECORDING_LOCKED -> PAUSE CaptureButtonState.PHOTO, CaptureButtonState.VIDEO -> GALLERY } diff --git a/feature/camera/src/main/java/org/signal/camera/hud/RecordingActionButtons.kt b/feature/camera/src/main/java/org/signal/camera/hud/RecordingActionButtons.kt index ed36293bc0..c1cc054e5c 100644 --- a/feature/camera/src/main/java/org/signal/camera/hud/RecordingActionButtons.kt +++ b/feature/camera/src/main/java/org/signal/camera/hud/RecordingActionButtons.kt @@ -41,12 +41,22 @@ private val ActionIconSize = 24.dp * recording is held, which puts it within reach of the finger already on the capture button. * * There is nothing to tap: sliding onto it is what takes the offer up. + * + * @param isEngaged Whether a thumb has arrived over it, which it answers in the recording red so that letting go there + * reads as taking the offer rather than as landing on an offer still being made. It is swapped for rather than + * animated in place, so the two looks are told apart the same way every other control in this corner is. */ @Composable -fun RecordingLockButton(modifier: Modifier = Modifier) { - RecordingActionButton(modifier = modifier.testTag(TestTags.CAMERA_HUD_LOCK_BUTTON)) { +fun RecordingLockButton( + isEngaged: Boolean = false, + modifier: Modifier = Modifier +) { + RecordingActionButton( + backgroundColor = colorResource(if (isEngaged) R.color.CameraHud_control_red_background else R.color.CameraHud_control_background), + modifier = modifier.testTag(TestTags.CAMERA_HUD_LOCK_BUTTON) + ) { Icon( - imageVector = SignalIcons.Lock.imageVector, + imageVector = if (isEngaged) SignalIcons.LockFill.imageVector else SignalIcons.Lock.imageVector, contentDescription = null, tint = Color.White, modifier = Modifier.size(ActionIconSize) @@ -107,6 +117,7 @@ fun RecordingPauseButton( @Composable private fun RecordingActionButton( modifier: Modifier = Modifier, + backgroundColor: Color = colorResource(R.color.CameraHud_control_background), onClick: (() -> Unit)? = null, content: @Composable () -> Unit ) { @@ -115,7 +126,7 @@ private fun RecordingActionButton( modifier = modifier .size(RecordingActionButtonSize) .clip(CircleShape) - .background(colorResource(R.color.CameraHud_control_background), CircleShape) + .background(backgroundColor, CircleShape) .then(if (onClick != null) Modifier.clickable(onClick = onClick) else Modifier) ) { content() @@ -130,6 +141,14 @@ private fun RecordingLockButtonPreview() { } } +@NightPreview +@Composable +private fun RecordingLockButtonEngagedPreview() { + Previews.Preview { + RecordingLockButton(isEngaged = true) + } +} + @NightPreview @Composable private fun RecordingPauseButtonPreview() { diff --git a/feature/camera/src/main/java/org/signal/camera/hud/StandardCameraHud.kt b/feature/camera/src/main/java/org/signal/camera/hud/StandardCameraHud.kt index 355e3e827e..95424c780c 100644 --- a/feature/camera/src/main/java/org/signal/camera/hud/StandardCameraHud.kt +++ b/feature/camera/src/main/java/org/signal/camera/hud/StandardCameraHud.kt @@ -416,17 +416,19 @@ private enum class RequestedRecording { /** * What stands in the gallery's corner. Everything it can hold is the same circle, so each fades into the next in place. * + * @param isOverLock Whether a drag from the capture button has reached the lock, which the lock answers for itself. * @param onLockCenterChanged Where the lock sits in the root's frame, which is one end of the drag that takes it. */ @Composable private fun GallerySlot( captureButtonState: CaptureButtonState, isRecordingPaused: Boolean, + isOverLock: Boolean, emitter: (StandardCameraHudEvents) -> Unit, onLockCenterChanged: (Offset) -> Unit ) { AnimatedContent( - targetState = GallerySlotContent.of(captureButtonState), + targetState = GallerySlotContent.of(captureButtonState, isOverLock), transitionSpec = { CameraHudMotion.swap }, label = "GallerySlotContent", modifier = Modifier.onGloballyPositioned { onLockCenterChanged(it.boundsInRoot().center) } @@ -438,6 +440,7 @@ private fun GallerySlot( ) GallerySlotContent.LOCK -> RecordingLockButton() + GallerySlotContent.LOCK_ENGAGED -> RecordingLockButton(isEngaged = true) GallerySlotContent.PAUSE -> RecordingPauseButton( isPaused = isRecordingPaused, onClick = { emitter(StandardCameraHudEvents.RecordingPauseToggled) } @@ -483,11 +486,16 @@ private fun CameraControls( var captureButtonCenter by remember { mutableStateOf(Offset.Zero) } var lockCenter by remember { mutableStateOf(Offset.Zero) } + // Whether the thumb dragging from the capture button has reached the lock. The lock is the one that shows it: the + // circle the drag carries there arrives underneath it, since this corner is drawn after the capture button. + var isOverLock by remember { mutableStateOf(false) } + val gallery: @Composable (CaptureButtonState) -> Unit = remember { movableContentOf { captureButtonState -> GallerySlot( captureButtonState = captureButtonState, isRecordingPaused = currentIsRecordingPaused, + isOverLock = isOverLock, emitter = currentEmitter, onLockCenterChanged = { lockCenter = it } ) @@ -539,6 +547,7 @@ private fun CameraControls( requestedRecording = RequestedRecording.UNHELD currentEmitter(StandardCameraHudEvents.VideoCaptureLocked) }, + onOverLockChanged = { isOverLock = it }, onTap = { captureButtonState.tapRequest?.let { request(it) } }, onLongPressStart = { if (!captureButtonState.isRecording) { diff --git a/feature/camera/src/test/java/org/signal/camera/hud/CaptureButtonStateTest.kt b/feature/camera/src/test/java/org/signal/camera/hud/CaptureButtonStateTest.kt index 62eb115a5d..e331d4b263 100644 --- a/feature/camera/src/test/java/org/signal/camera/hud/CaptureButtonStateTest.kt +++ b/feature/camera/src/test/java/org/signal/camera/hud/CaptureButtonStateTest.kt @@ -103,11 +103,24 @@ class CaptureButtonStateTest { assertThat(GallerySlotContent.of(CaptureButtonState.RECORDING_HELD)).isEqualTo(GallerySlotContent.LOCK) } + /** The thumb covers the lock on the way to it, so the lock answering for itself is all there is to go by. */ + @Test + fun `Given a thumb over the lock, when the gallery's corner is filled, then the engaged lock is what fills it`() { + assertThat(GallerySlotContent.of(CaptureButtonState.RECORDING_HELD, isOverLock = true)).isEqualTo(GallerySlotContent.LOCK_ENGAGED) + } + @Test fun `Given a recording that is locked, when the gallery's corner is filled, then the pause is what fills it`() { assertThat(GallerySlotContent.of(CaptureButtonState.RECORDING_LOCKED)).isEqualTo(GallerySlotContent.PAUSE) } + /** Only a held recording has a lock on offer, so nothing else has any use for a thumb being over one. */ + @Test + fun `Given no lock on offer, when a thumb is reported over one, then the corner is filled as it would have been`() { + assertThat(GallerySlotContent.of(CaptureButtonState.PHOTO, isOverLock = true)).isEqualTo(GallerySlotContent.GALLERY) + assertThat(GallerySlotContent.of(CaptureButtonState.RECORDING_LOCKED, isOverLock = true)).isEqualTo(GallerySlotContent.PAUSE) + } + private fun stateOf( captureButtonMode: CaptureButtonMode, isRecording: Boolean,