Apply various bits of design polish.

This commit is contained in:
Alex Hart
2026-09-02 16:11:24 -03:00
parent 171c6d3f5f
commit 5cbb46ea84
6 changed files with 146 additions and 54 deletions
@@ -25,6 +25,7 @@ import androidx.compose.foundation.layout.Column
import androidx.compose.foundation.layout.fillMaxHeight
import androidx.compose.foundation.layout.fillMaxSize
import androidx.compose.foundation.layout.fillMaxWidth
import androidx.compose.foundation.layout.heightIn
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.layout.size
import androidx.compose.foundation.shape.CircleShape
@@ -92,6 +93,12 @@ private val ZOOM_BAR_SIDE_MARGIN = 16.dp
/** How long the time and the paused label take to trade the recording red between them. */
private const val PAUSED_TRANSITION_MS = 200
/** The close and flash buttons along the top of the window, which the recording pill lines its middle up with. */
private val TOP_CONTROL_SIZE = 48.dp
/** How far the top controls sit in from the edges of the window. */
private val TOP_CONTROL_MARGIN = 16.dp
data class StringResources(
@param:StringRes val photoCaptureFailed: Int = 0,
@param:StringRes val photoProcessingFailed: Int = 0,
@@ -282,8 +289,8 @@ private fun BoxScope.StandardCameraHudContent(
onClick = { emitter(StandardCameraHudEvents.CloseClick) },
enabled = !isRecordingHeld,
modifier = modifier
.padding(16.dp)
.size(48.dp)
.padding(TOP_CONTROL_MARGIN)
.size(TOP_CONTROL_SIZE)
.fadedIn(!isRecordingHeld)
.background(colorResource(R.color.CameraHud_control_background), shape = CircleShape)
.testTag(TestTags.CAMERA_HUD_CLOSE_BUTTON)
@@ -306,7 +313,7 @@ private fun BoxScope.StandardCameraHudContent(
enabled = !isRecordingHeld,
modifier = Modifier
.align(Alignment.TopEnd)
.padding(16.dp)
.padding(TOP_CONTROL_MARGIN)
.fadedIn(!isRecordingHeld)
.rotate(iconRotation)
)
@@ -319,7 +326,7 @@ private fun BoxScope.StandardCameraHudContent(
pausedLabel = if (stringResources.recordingPaused != 0) stringResource(stringResources.recordingPaused) else null,
modifier = Modifier
.align(Alignment.TopCenter)
.padding(top = 16.dp)
.padding(top = TOP_CONTROL_MARGIN)
)
}
@@ -622,14 +629,12 @@ private fun HorizontalControlBar(
.fillMaxWidth()
.padding(bottom = 40.dp, start = 40.dp, end = 40.dp)
) {
Box(modifier = Modifier.align(Alignment.CenterEnd).rotate(iconRotation)) {
CameraSwitchButton(
onClick = { emitter(StandardCameraHudEvents.SwitchCamera) },
stringResources = stringResources,
enabled = captureButtonState != CaptureButtonState.RECORDING_HELD,
modifier = Modifier.fadedIn(captureButtonState != CaptureButtonState.RECORDING_HELD)
)
}
CameraSwitchCorner(
isRecording = captureButtonState.isRecording,
iconRotation = iconRotation,
stringResources = stringResources,
emitter = emitter
)
Box(modifier = Modifier.align(Alignment.Center).rotate(iconRotation)) {
captureSlot(captureButtonState)
}
@@ -640,6 +645,33 @@ private fun HorizontalControlBar(
}
}
/**
* The camera switch in its own corner of the bottom bar. The camera cannot be swapped out from under a running
* recording, so the button goes for as long as one runs; it is aligned into the corner rather than laid out beside
* anything, so nothing moves when it does.
*/
@Composable
private fun BoxScope.CameraSwitchCorner(
isRecording: Boolean,
iconRotation: Float,
stringResources: StringResources,
emitter: (StandardCameraHudEvents) -> Unit
) {
AnimatedVisibility(
visible = !isRecording,
enter = fadeIn(),
exit = fadeOut(),
modifier = Modifier.align(Alignment.CenterEnd)
) {
Box(modifier = Modifier.rotate(iconRotation)) {
CameraSwitchButton(
onClick = { emitter(StandardCameraHudEvents.SwitchCamera) },
stringResources = stringResources
)
}
}
}
@Composable
private fun VerticalControlBar(
flashMode: FlashMode,
@@ -667,6 +699,7 @@ private fun VerticalControlBar(
flashMode = flashMode,
emitter = emitter,
stringResources = stringResources,
isRecording = captureButtonState.isRecording,
enabled = captureButtonState != CaptureButtonState.RECORDING_HELD,
modifier = Modifier.fadedIn(captureButtonState != CaptureButtonState.RECORDING_HELD)
)
@@ -690,6 +723,7 @@ private fun FlashAndCameraTogglePill(
flashMode: FlashMode,
stringResources: StringResources,
emitter: (StandardCameraHudEvents) -> Unit,
isRecording: Boolean,
enabled: Boolean = true,
modifier: Modifier = Modifier
) {
@@ -710,16 +744,20 @@ private fun FlashAndCameraTogglePill(
)
}
IconButton(
onClick = { emitter(StandardCameraHudEvents.SwitchCamera) },
enabled = enabled,
modifier = Modifier.testTag(TestTags.CAMERA_HUD_SWITCH_BUTTON)
) {
Icon(
imageVector = SignalIcons.CameraSwitch.imageVector,
contentDescription = if (stringResources.switchCamera != 0) stringResource(stringResources.switchCamera) else null,
tint = Color.White
)
// The camera cannot be swapped out from under a running recording, so the pill gives the button up and closes
// around the flash for as long as one runs.
AnimatedVisibility(visible = !isRecording) {
IconButton(
onClick = { emitter(StandardCameraHudEvents.SwitchCamera) },
enabled = enabled,
modifier = Modifier.testTag(TestTags.CAMERA_HUD_SWITCH_BUTTON)
) {
Icon(
imageVector = SignalIcons.CameraSwitch.imageVector,
contentDescription = if (stringResources.switchCamera != 0) stringResource(stringResources.switchCamera) else null,
tint = Color.White
)
}
}
}
}
@@ -753,18 +791,25 @@ private fun RecordingDurationDisplay(
horizontalAlignment = Alignment.CenterHorizontally,
modifier = modifier
) {
// The pill is shorter than the close and flash buttons it sits between, so it is centered in a row of their height
// to put the three of them on one line.
Box(
modifier = Modifier
.background(lerp(recordingRed, pausedGray, pausedProgress), shape = CircleShape)
.padding(horizontal = 16.dp, vertical = 4.dp)
.testTag(TestTags.CAMERA_HUD_RECORDING_DURATION)
contentAlignment = Alignment.Center,
modifier = Modifier.heightIn(min = TOP_CONTROL_SIZE)
) {
Text(
text = timeText,
color = Color.White,
fontSize = 18.sp,
fontWeight = FontWeight.Medium
)
Box(
modifier = Modifier
.background(lerp(recordingRed, pausedGray, pausedProgress), shape = CircleShape)
.padding(horizontal = 16.dp, vertical = 4.dp)
.testTag(TestTags.CAMERA_HUD_RECORDING_DURATION)
) {
Text(
text = timeText,
color = Color.White,
fontSize = 18.sp,
fontWeight = FontWeight.Medium
)
}
}
if (pausedLabel != null && pausedProgress > 0f) {
@@ -791,7 +836,6 @@ private fun RecordingDurationDisplay(
private fun CameraSwitchButton(
onClick: () -> Unit,
stringResources: StringResources,
enabled: Boolean = true,
modifier: Modifier = Modifier
) {
val contentDescription = if (stringResources.switchCamera != 0) {
@@ -802,7 +846,6 @@ private fun CameraSwitchButton(
IconButton(
onClick = onClick,
enabled = enabled,
modifier = modifier
.size(52.dp)
.background(colorResource(R.color.CameraHud_control_background), shape = CircleShape)
@@ -829,7 +872,7 @@ private fun FlashToggleButton(
onClick = onToggle,
enabled = enabled,
modifier = modifier
.size(48.dp)
.size(TOP_CONTROL_SIZE)
.background(colorResource(R.color.CameraHud_control_background), shape = CircleShape)
.testTag(TestTags.CAMERA_HUD_FLASH_BUTTON)
) {
@@ -91,17 +91,30 @@ class StandardCameraHudTest {
composeTestRule.onNodeWithTag(TestTags.CAMERA_HUD_CLOSE_BUTTON).assertIsNotEnabled()
composeTestRule.onNodeWithTag(TestTags.CAMERA_HUD_FLASH_BUTTON).assertIsNotEnabled()
composeTestRule.onNodeWithTag(TestTags.CAMERA_HUD_SWITCH_BUTTON).assertIsNotEnabled()
}
/** A locked recording leaves the hand free, so nothing has to be taken away. */
/** A locked recording leaves the hand free, so nothing but the camera switch has to be taken away. */
@Test
fun `Given a recording that is locked, when displayed, then the chrome around it can still be used`() {
setContent(state = lockedRecording())
composeTestRule.onNodeWithTag(TestTags.CAMERA_HUD_CLOSE_BUTTON).assertIsEnabled()
composeTestRule.onNodeWithTag(TestTags.CAMERA_HUD_FLASH_BUTTON).assertIsEnabled()
composeTestRule.onNodeWithTag(TestTags.CAMERA_HUD_SWITCH_BUTTON).assertIsEnabled()
}
/** The camera cannot be swapped out from under a running recording, however that recording was started. */
@Test
fun `Given a recording, when displayed, then the camera switch is gone`() {
setContent(state = lockedRecording())
composeTestRule.onNodeWithTag(TestTags.CAMERA_HUD_SWITCH_BUTTON).assertDoesNotExist()
}
@Test
fun `Given a recording that is being held, when displayed, then the camera switch is gone`() {
setContent(state = heldRecording())
composeTestRule.onNodeWithTag(TestTags.CAMERA_HUD_SWITCH_BUTTON).assertDoesNotExist()
}
@Test
@@ -246,7 +259,7 @@ class StandardCameraHudTest {
setContent(state = heldRecording())
composeTestRule.onNodeWithTag(TestTags.CAMERA_HUD_CLOSE_BUTTON).performClick()
composeTestRule.onNodeWithTag(TestTags.CAMERA_HUD_SWITCH_BUTTON).performClick()
composeTestRule.onNodeWithTag(TestTags.CAMERA_HUD_FLASH_BUTTON).performClick()
assertThat(events).isEmpty()
}
@@ -332,7 +345,15 @@ class StandardCameraHudTest {
setContent(state = heldRecording())
composeTestRule.onNodeWithTag(TestTags.CAMERA_HUD_FLASH_BUTTON).assertIsNotEnabled()
composeTestRule.onNodeWithTag(TestTags.CAMERA_HUD_SWITCH_BUTTON).assertIsNotEnabled()
}
/** The pill gives the switch up and closes around the flash, the same as the bottom bar drops it. */
@Test
@Config(qualifiers = "w840dp-h1000dp")
fun `Given a window too large for the bottom bar, when a recording runs, then the pill has no camera switch`() {
setContent(state = lockedRecording())
composeTestRule.onNodeWithTag(TestTags.CAMERA_HUD_SWITCH_BUTTON).assertDoesNotExist()
}
//endregion
@@ -11,7 +11,6 @@ import androidx.compose.foundation.layout.Arrangement
import androidx.compose.foundation.layout.Box
import androidx.compose.foundation.layout.Row
import androidx.compose.foundation.layout.height
import androidx.compose.foundation.layout.heightIn
import androidx.compose.foundation.layout.padding
import androidx.compose.foundation.shape.RoundedCornerShape
import androidx.compose.material3.Icon
@@ -39,11 +38,17 @@ import org.signal.mediasend.R
import org.signal.mediasend.test.TestTags
/**
* Mirrors the legacy send button's size. Has to be stated rather than left to the [IconButtons.IconButton] default of
* 40dp: the default draws the container inside the 48dp of layout that [androidx.compose.material3.minimumInteractiveComponentSize]
* reserves, leaving a button that is 8dp smaller than it looks like it should be and smaller than the row it sits in.
* The circle the arrow is drawn in. [IconButtons.IconButton] keeps the 48dp of layout that
* [androidx.compose.material3.minimumInteractiveComponentSize] reserves around it, so what a finger can hit stays a full
* touch target wide while the button reads as 40dp.
*/
private val NextButtonSize = 48.dp
private val NextButtonSize = 40.dp
/**
* The bar itself, stated rather than left to what is inside it: the icon buttons it carries each reserve a 48dp touch
* target, which would otherwise make the bar 4dp taller than it is meant to be.
*/
private val InputBarHeight = 44.dp
/**
* Because we need to be able to support stuff like mentions, styled text, and custom emoji, we need to allow
@@ -88,7 +93,7 @@ fun AddAMessageRow(
modifier = Modifier
.background(color = MaterialTheme.colorScheme.surfaceVariant, shape = RoundedCornerShape(24.dp))
.weight(1f)
.heightIn(min = 44.dp)
.height(InputBarHeight)
.then(
if (viewOnce) {
// A view-once send cannot carry a body, so the row becomes a static label rather than an entry point.
@@ -123,7 +128,7 @@ fun AddAMessageRow(
message?.takeIf { it.isNotBlank() } ?: stringResource(if (isReply) R.string.AddAMessageRow__add_a_reply else R.string.AddAMessageRow__message),
Modifier
.weight(1f)
.height(44.dp)
.height(InputBarHeight)
.padding(end = if (viewOnceAvailable) 0.dp else 16.dp, top = 10.dp, bottom = 10.dp)
)
}
@@ -558,15 +558,20 @@ private fun MediaToolbar(
}
MediaEditControl(faded = faded, modifier = modifier) {
val isSmall = rememberWindowBreakpoint() is WindowBreakpoint.Small
// Beyond a phone the toolbar runs down the window's end edge rather than along the bottom, so it has to be held off
// of that edge whichever kind of media it is built for.
val sideRailPadding = if (isSmall) Modifier else Modifier.padding(end = 24.dp)
when (focusedEditorState) {
is EditorState.Image -> {
val breakpoint = rememberWindowBreakpoint()
val modifier = if (breakpoint is WindowBreakpoint.Small) {
val toolbarModifier = if (isSmall) {
Modifier
.navigationBarsPadding()
.padding(horizontal = 16.dp)
} else {
Modifier.padding(end = 24.dp)
sideRailPadding
}
imageController?.let {
@@ -575,14 +580,14 @@ private fun MediaToolbar(
state = state,
editorState = focusedEditorState,
onEvent = onEvent,
modifier = modifier
modifier = toolbarModifier
.then(if (isTextEditing) Modifier.imePadding() else Modifier),
enabled = !faded
)
}
}
else -> MediaEditorToolbar {
else -> MediaEditorToolbar(modifier = sideRailPadding) {
MediaEditorToolbarSharedButtons(
state = state,
editorState = focusedEditorState,
@@ -98,12 +98,12 @@ internal fun NextButton(
.align(Alignment.TopCenter)
.heightIn(min = NEXT_COUNT_HEIGHT)
.widthIn(min = NEXT_COUNT_HEIGHT)
.background(color = recipientChatColor ?: MaterialTheme.colorScheme.primaryContainer, shape = CircleShape)
.background(color = recipientChatColor ?: MaterialTheme.colorScheme.primary, shape = CircleShape)
.padding(horizontal = NEXT_COUNT_HORIZONTAL_PADDING)
) {
Text(
text = selectedMediaCount.toString(),
color = if (recipientChatColor != null) SignalTheme.colors.colorOnCustom else MaterialTheme.colorScheme.onPrimaryContainer,
color = if (recipientChatColor != null) SignalTheme.colors.colorOnCustom else MaterialTheme.colorScheme.onPrimary,
style = MaterialTheme.typography.labelSmall,
maxLines = 1,
modifier = Modifier.testTag(TestTags.MEDIA_SEND_MEDIA_COUNT)
@@ -6,12 +6,15 @@
package org.signal.mediasend.screens.edit
import android.app.Application
import androidx.compose.ui.geometry.Offset
import androidx.compose.ui.test.click
import androidx.compose.ui.test.junit4.createComposeRule
import androidx.compose.ui.test.longClick
import androidx.compose.ui.test.onNodeWithTag
import androidx.compose.ui.test.onNodeWithText
import androidx.compose.ui.test.performClick
import androidx.compose.ui.test.performTouchInput
import androidx.compose.ui.unit.dp
import androidx.test.core.app.ApplicationProvider
import org.junit.Assert.assertEquals
import org.junit.Assert.assertNull
@@ -75,6 +78,21 @@ class AddAMessageRowTest {
assertEquals(emptyList<MediaEditScreenEvents>(), events)
}
/**
* The button draws smaller than a finger, so the 48dp of touch target reserved around the circle it draws is the part
* worth pinning down: a tap just off the circle still counts as one.
*/
@Test
fun `when a tap lands off the send button but inside its touch target, then the flow still advances`() {
setContent()
composeTestRule.onNodeWithTag(TestTags.ADD_A_MESSAGE_NEXT_BUTTON).performTouchInput {
click(Offset(width / 2f, height + 3.dp.toPx()))
}
assertEquals(1, nextClicks)
}
@Test
fun `Given a blank message, when the row is displayed, then the placeholder is shown`() {
setContent(message = " ")