diff --git a/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/account/signallogin/SignalLoginViewDetailsViewModel.kt b/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/account/signallogin/SignalLoginViewDetailsViewModel.kt index 5f12a464e5..d13f0bde3b 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/account/signallogin/SignalLoginViewDetailsViewModel.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/components/settings/app/account/signallogin/SignalLoginViewDetailsViewModel.kt @@ -50,10 +50,10 @@ class SignalLoginViewDetailsViewModel( SignalLoginViewDetailsScreenEvents.SaveAsPdfClicked -> { _actions.send(SignalLoginViewDetailsAction.LaunchSaveAsPdf) } - is SignalLoginViewDetailsScreenEvents.AccountIdLongClicked -> { + is SignalLoginViewDetailsScreenEvents.CopyAccountIdClicked -> { _actions.send(SignalLoginViewDetailsAction.CopyTextToClipboard(event.aci)) } - is SignalLoginViewDetailsScreenEvents.RecoveryKeyLongClicked -> { + is SignalLoginViewDetailsScreenEvents.CopyRecoveryKeyClicked -> { _actions.send(SignalLoginViewDetailsAction.CopyTextToClipboard(event.aep)) } } diff --git a/app/src/test/java/org/thoughtcrime/securesms/components/settings/app/account/signallogin/SignalLoginViewDetailsViewModelTest.kt b/app/src/test/java/org/thoughtcrime/securesms/components/settings/app/account/signallogin/SignalLoginViewDetailsViewModelTest.kt index 5fddec2a10..20569cdfa1 100644 --- a/app/src/test/java/org/thoughtcrime/securesms/components/settings/app/account/signallogin/SignalLoginViewDetailsViewModelTest.kt +++ b/app/src/test/java/org/thoughtcrime/securesms/components/settings/app/account/signallogin/SignalLoginViewDetailsViewModelTest.kt @@ -100,24 +100,24 @@ class SignalLoginViewDetailsViewModelTest { } @Test - fun `AccountIdLongClicked copies the account key to the clipboard`() = runTest(testDispatcher) { + fun `CopyAccountIdClicked copies the account key to the clipboard`() = runTest(testDispatcher) { val viewModel = SignalLoginViewDetailsViewModel(repository) val actions = mutableListOf() backgroundScope.launch { viewModel.actions.toList(actions) } - viewModel.onEvent(SignalLoginViewDetailsScreenEvents.AccountIdLongClicked("A6B28482-2E32-83D0-7F23-91360A4C2B91")) + viewModel.onEvent(SignalLoginViewDetailsScreenEvents.CopyAccountIdClicked("A6B28482-2E32-83D0-7F23-91360A4C2B91")) assertThat(actions).containsExactly(SignalLoginViewDetailsAction.CopyTextToClipboard("A6B28482-2E32-83D0-7F23-91360A4C2B91")) } @Test - fun `RecoveryKeyLongClicked copies the recovery key to the clipboard`() = runTest(testDispatcher) { + fun `CopyRecoveryKeyClicked copies the recovery key to the clipboard`() = runTest(testDispatcher) { val recoveryKey = AccountEntropyPool.generate().displayValue val viewModel = SignalLoginViewDetailsViewModel(repository) val actions = mutableListOf() backgroundScope.launch { viewModel.actions.toList(actions) } - viewModel.onEvent(SignalLoginViewDetailsScreenEvents.RecoveryKeyLongClicked(recoveryKey)) + viewModel.onEvent(SignalLoginViewDetailsScreenEvents.CopyRecoveryKeyClicked(recoveryKey)) assertThat(actions).containsExactly(SignalLoginViewDetailsAction.CopyTextToClipboard(recoveryKey)) } diff --git a/feature/registration/src/main/java/org/signal/registration/screens/signallogindetails/SignalLoginViewDetailsViewModel.kt b/feature/registration/src/main/java/org/signal/registration/screens/signallogindetails/SignalLoginViewDetailsViewModel.kt index 01478432ba..7dd243b4af 100644 --- a/feature/registration/src/main/java/org/signal/registration/screens/signallogindetails/SignalLoginViewDetailsViewModel.kt +++ b/feature/registration/src/main/java/org/signal/registration/screens/signallogindetails/SignalLoginViewDetailsViewModel.kt @@ -72,11 +72,11 @@ class SignalLoginViewDetailsViewModel( _actions.trySend(SignalLoginViewDetailsScreenActions.LaunchSaveAsPdf) } - is SignalLoginViewDetailsScreenEvents.AccountIdLongClicked -> { + is SignalLoginViewDetailsScreenEvents.CopyAccountIdClicked -> { _actions.trySend(SignalLoginViewDetailsScreenActions.CopyTextToClipboard(event.aci)) } - is SignalLoginViewDetailsScreenEvents.RecoveryKeyLongClicked -> { + is SignalLoginViewDetailsScreenEvents.CopyRecoveryKeyClicked -> { _actions.trySend(SignalLoginViewDetailsScreenActions.CopyTextToClipboard(event.aep)) } } diff --git a/feature/registration/src/test/java/org/signal/registration/screens/signallogindetails/SignalLoginViewDetailsScreenTest.kt b/feature/registration/src/test/java/org/signal/registration/screens/signallogindetails/SignalLoginViewDetailsScreenTest.kt index 6ad48b8fba..42e9e1bbb5 100644 --- a/feature/registration/src/test/java/org/signal/registration/screens/signallogindetails/SignalLoginViewDetailsScreenTest.kt +++ b/feature/registration/src/test/java/org/signal/registration/screens/signallogindetails/SignalLoginViewDetailsScreenTest.kt @@ -7,10 +7,9 @@ package org.signal.registration.screens.signallogindetails import android.app.Application import androidx.compose.ui.test.junit4.createComposeRule -import androidx.compose.ui.test.longClick import androidx.compose.ui.test.onNodeWithTag +import androidx.compose.ui.test.performClick import androidx.compose.ui.test.performScrollTo -import androidx.compose.ui.test.performTouchInput import androidx.test.core.app.ApplicationProvider import assertk.assertThat import assertk.assertions.contains @@ -44,21 +43,21 @@ class SignalLoginViewDetailsScreenTest { private val events = mutableListOf() @Test - fun `when the account key is long clicked, AccountIdLongClicked is emitted`() { + fun `when the account key copy button is clicked, CopyAccountIdClicked is emitted`() { setContent() - composeTestRule.onNodeWithTag(SignalLoginTestTags.VIEW_DETAILS_ACCOUNT_KEY_BLOCK).performScrollTo().performTouchInput { longClick() } + composeTestRule.onNodeWithTag(SignalLoginTestTags.VIEW_DETAILS_ACCOUNT_KEY_COPY_BUTTON).performScrollTo().performClick() - assertThat(events).contains(SignalLoginViewDetailsScreenEvents.AccountIdLongClicked(ACCOUNT_KEY)) + assertThat(events).contains(SignalLoginViewDetailsScreenEvents.CopyAccountIdClicked(ACCOUNT_KEY)) } @Test - fun `when the recovery key is long clicked, RecoveryKeyLongClicked is emitted`() { + fun `when the recovery key copy button is clicked, CopyRecoveryKeyClicked is emitted`() { setContent() - composeTestRule.onNodeWithTag(SignalLoginTestTags.VIEW_DETAILS_RECOVERY_KEY_BLOCK).performScrollTo().performTouchInput { longClick() } + composeTestRule.onNodeWithTag(SignalLoginTestTags.VIEW_DETAILS_RECOVERY_KEY_COPY_BUTTON).performScrollTo().performClick() - assertThat(events).contains(SignalLoginViewDetailsScreenEvents.RecoveryKeyLongClicked(RECOVERY_KEY)) + assertThat(events).contains(SignalLoginViewDetailsScreenEvents.CopyRecoveryKeyClicked(RECOVERY_KEY)) } private fun setContent() { diff --git a/feature/registration/src/test/java/org/signal/registration/screens/signallogindetails/SignalLoginViewDetailsViewModelTest.kt b/feature/registration/src/test/java/org/signal/registration/screens/signallogindetails/SignalLoginViewDetailsViewModelTest.kt index 94387d66a8..df70687aec 100644 --- a/feature/registration/src/test/java/org/signal/registration/screens/signallogindetails/SignalLoginViewDetailsViewModelTest.kt +++ b/feature/registration/src/test/java/org/signal/registration/screens/signallogindetails/SignalLoginViewDetailsViewModelTest.kt @@ -92,22 +92,22 @@ class SignalLoginViewDetailsViewModelTest { } @Test - fun `AccountIdLongClicked copies the account key to the clipboard`() = runTest(testDispatcher) { + fun `CopyAccountIdClicked copies the account key to the clipboard`() = runTest(testDispatcher) { val actions = mutableListOf() backgroundScope.launch { viewModel.actions.toList(actions) } - viewModel.onEvent(SignalLoginViewDetailsScreenEvents.AccountIdLongClicked("A6B28482-2E32-83D0-7F23-91360A4C2B91")) + viewModel.onEvent(SignalLoginViewDetailsScreenEvents.CopyAccountIdClicked("A6B28482-2E32-83D0-7F23-91360A4C2B91")) assertThat(actions).containsExactly(SignalLoginViewDetailsScreenActions.CopyTextToClipboard("A6B28482-2E32-83D0-7F23-91360A4C2B91")) } @Test - fun `RecoveryKeyLongClicked copies the recovery key to the clipboard`() = runTest(testDispatcher) { + fun `CopyRecoveryKeyClicked copies the recovery key to the clipboard`() = runTest(testDispatcher) { val recoveryKey = AccountEntropyPool.generate().displayValue val actions = mutableListOf() backgroundScope.launch { viewModel.actions.toList(actions) } - viewModel.onEvent(SignalLoginViewDetailsScreenEvents.RecoveryKeyLongClicked(recoveryKey)) + viewModel.onEvent(SignalLoginViewDetailsScreenEvents.CopyRecoveryKeyClicked(recoveryKey)) assertThat(actions).containsExactly(SignalLoginViewDetailsScreenActions.CopyTextToClipboard(recoveryKey)) } diff --git a/lib/signal-login/src/main/java/org/signal/signallogin/SignalLoginTestTags.kt b/lib/signal-login/src/main/java/org/signal/signallogin/SignalLoginTestTags.kt index 0f3547b5d7..a44a3b5941 100644 --- a/lib/signal-login/src/main/java/org/signal/signallogin/SignalLoginTestTags.kt +++ b/lib/signal-login/src/main/java/org/signal/signallogin/SignalLoginTestTags.kt @@ -16,4 +16,6 @@ object SignalLoginTestTags { const val VIEW_DETAILS_SAVE_AS_PDF_BUTTON = "signal_login_view_details_save_as_pdf_button" const val VIEW_DETAILS_ACCOUNT_KEY_BLOCK = "signal_login_view_details_account_key_block" const val VIEW_DETAILS_RECOVERY_KEY_BLOCK = "signal_login_view_details_recovery_key_block" + const val VIEW_DETAILS_ACCOUNT_KEY_COPY_BUTTON = "signal_login_view_details_account_key_copy_button" + const val VIEW_DETAILS_RECOVERY_KEY_COPY_BUTTON = "signal_login_view_details_recovery_key_copy_button" } diff --git a/lib/signal-login/src/main/java/org/signal/signallogin/viewdetails/SignalLoginViewDetailsScreen.kt b/lib/signal-login/src/main/java/org/signal/signallogin/viewdetails/SignalLoginViewDetailsScreen.kt index d8db4f6db0..1564604af7 100644 --- a/lib/signal-login/src/main/java/org/signal/signallogin/viewdetails/SignalLoginViewDetailsScreen.kt +++ b/lib/signal-login/src/main/java/org/signal/signallogin/viewdetails/SignalLoginViewDetailsScreen.kt @@ -7,9 +7,7 @@ package org.signal.signallogin.viewdetails import androidx.compose.foundation.Image import androidx.compose.foundation.background -import androidx.compose.foundation.combinedClickable import androidx.compose.foundation.layout.Arrangement -import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.BoxWithConstraints import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.Row @@ -24,6 +22,8 @@ import androidx.compose.foundation.rememberScrollState import androidx.compose.foundation.shape.RoundedCornerShape import androidx.compose.foundation.verticalScroll import androidx.compose.material3.ButtonDefaults +import androidx.compose.material3.Icon +import androidx.compose.material3.IconButton import androidx.compose.material3.MaterialTheme import androidx.compose.material3.Text import androidx.compose.runtime.Composable @@ -67,6 +67,13 @@ private const val GROUPS_PER_ROW = 4 /** The least amount of space allowed between recovery key groups before falling back to natural text wrapping. */ private val MIN_GROUP_SPACING = 12.dp +private val KEY_BLOCK_TEXT_PADDING_HORIZONTAL = 28.dp +private val KEY_BLOCK_TEXT_PADDING_VERTICAL = 20.dp + +/** Insets that center the copy button's icon on the first line of key text and put it 16dp from the block's end edge, accounting for the button's own 12dp of internal padding. */ +private val COPY_BUTTON_PADDING_TOP = 10.dp +private val COPY_BUTTON_PADDING_END = 4.dp + /** * Shows the user the full keys that make up their Signal Login and offers ways to save them. */ @@ -102,9 +109,9 @@ fun SignalLoginViewDetailsScreen( Spacer(modifier = Modifier.height(16.dp)) - Texts.SectionHeader(text = stringResource(R.string.SignalLoginViewDetailsScreen__account_key)) + Texts.SectionHeader(text = stringResource(R.string.SignalLoginViewDetailsScreen__account_id)) - KeyBlock( + AccountIdBlock( text = state.accountKey, onEvent = onEvent, modifier = Modifier.testTag(SignalLoginTestTags.VIEW_DETAILS_ACCOUNT_KEY_BLOCK) @@ -143,15 +150,27 @@ private fun MiniCard(modifier: Modifier = Modifier) { * A full credential rendered in the special monospace font on a rounded surface. */ @Composable -private fun KeyBlock( +private fun AccountIdBlock( text: String, onEvent: (SignalLoginViewDetailsScreenEvents) -> Unit, modifier: Modifier = Modifier ) { - Box(modifier = modifier.keyBlockSurface(onLongClick = { onEvent(SignalLoginViewDetailsScreenEvents.AccountIdLongClicked(text)) })) { + Row( + verticalAlignment = Alignment.Top, + modifier = modifier.keyBlockSurface() + ) { Text( text = text, - style = keyTextStyle() + style = keyTextStyle(), + modifier = Modifier + .weight(1f) + .padding(start = KEY_BLOCK_TEXT_PADDING_HORIZONTAL, top = KEY_BLOCK_TEXT_PADDING_VERTICAL, bottom = KEY_BLOCK_TEXT_PADDING_VERTICAL) + ) + + CopyButton( + contentDescription = stringResource(R.string.SignalLoginViewDetailsScreen__copy_account_id), + onClick = { onEvent(SignalLoginViewDetailsScreenEvents.CopyAccountIdClicked(text)) }, + modifier = Modifier.testTag(SignalLoginTestTags.VIEW_DETAILS_ACCOUNT_KEY_COPY_BUTTON) ) } } @@ -167,57 +186,88 @@ private fun RecoveryKeyBlock( onEvent: (SignalLoginViewDetailsScreenEvents) -> Unit, modifier: Modifier = Modifier ) { - BoxWithConstraints(modifier = modifier.keyBlockSurface(onLongClick = { onEvent(SignalLoginViewDetailsScreenEvents.RecoveryKeyLongClicked(groups.joinToString(separator = ""))) })) { - val style = keyTextStyle() - val textMeasurer = rememberTextMeasurer() - val maxWidth = constraints.maxWidth + Row( + verticalAlignment = Alignment.Top, + modifier = modifier.keyBlockSurface() + ) { + BoxWithConstraints( + modifier = Modifier + .weight(1f) + .padding(start = KEY_BLOCK_TEXT_PADDING_HORIZONTAL, top = KEY_BLOCK_TEXT_PADDING_VERTICAL, bottom = KEY_BLOCK_TEXT_PADDING_VERTICAL) + ) { + val style = keyTextStyle() + val textMeasurer = rememberTextMeasurer() + val maxWidth = constraints.maxWidth - val groupWidth = remember(groups, style) { - groups.maxOfOrNull { group -> textMeasurer.measure(text = group, style = style).size.width } ?: 0 - } + val groupWidth = remember(groups, style) { + groups.maxOfOrNull { group -> textMeasurer.measure(text = group, style = style).size.width } ?: 0 + } - val minSpacing = with(LocalDensity.current) { MIN_GROUP_SPACING.roundToPx() } - val fitsFourPerRow = groupWidth * GROUPS_PER_ROW + minSpacing * (GROUPS_PER_ROW - 1) <= maxWidth + val minSpacing = with(LocalDensity.current) { MIN_GROUP_SPACING.roundToPx() } + val fitsFourPerRow = groupWidth * GROUPS_PER_ROW + minSpacing * (GROUPS_PER_ROW - 1) <= maxWidth - if (fitsFourPerRow) { - val spacing = with(LocalDensity.current) { ((maxWidth - groupWidth * GROUPS_PER_ROW) / (GROUPS_PER_ROW - 1)).toDp() } + if (fitsFourPerRow) { + val spacing = with(LocalDensity.current) { ((maxWidth - groupWidth * GROUPS_PER_ROW) / (GROUPS_PER_ROW - 1)).toDp() } - Column { - groups.chunked(GROUPS_PER_ROW).forEach { row -> - Row( - horizontalArrangement = Arrangement.spacedBy(spacing), - modifier = Modifier.fillMaxWidth() - ) { - row.forEach { group -> - Text( - text = group, - style = style - ) + Column { + groups.chunked(GROUPS_PER_ROW).forEach { row -> + Row( + horizontalArrangement = Arrangement.spacedBy(spacing), + modifier = Modifier.fillMaxWidth() + ) { + row.forEach { group -> + Text( + text = group, + style = style + ) + } } } } + } else { + Text( + text = groups.joinToString(separator = " "), + style = style + ) } - } else { - Text( - text = groups.joinToString(separator = " "), - style = style - ) } + + CopyButton( + contentDescription = stringResource(R.string.SignalLoginViewDetailsScreen__copy_recovery_key), + onClick = { onEvent(SignalLoginViewDetailsScreenEvents.CopyRecoveryKeyClicked(groups.joinToString(separator = ""))) }, + modifier = Modifier.testTag(SignalLoginTestTags.VIEW_DETAILS_RECOVERY_KEY_COPY_BUTTON) + ) + } +} + +/** + * The button in the top-end corner of a key block that copies the key to the clipboard. + */ +@Composable +private fun CopyButton( + contentDescription: String, + onClick: () -> Unit, + modifier: Modifier = Modifier +) { + IconButton( + onClick = onClick, + modifier = modifier.padding(top = COPY_BUTTON_PADDING_TOP, end = COPY_BUTTON_PADDING_END) + ) { + Icon( + painter = SignalIcons.Copy.painter, + contentDescription = contentDescription, + tint = MaterialTheme.colorScheme.onSurfaceVariant + ) } } @Composable -private fun Modifier.keyBlockSurface(onLongClick: () -> Unit): Modifier { +private fun Modifier.keyBlockSurface(): Modifier { return this .horizontalGutters() .fillMaxWidth() .clip(RoundedCornerShape(18.dp)) .background(SignalTheme.colors.colorSurface2) - .combinedClickable( - onLongClick = onLongClick, - onClick = {} - ) - .padding(horizontal = 28.dp, vertical = 20.dp) } @Composable diff --git a/lib/signal-login/src/main/java/org/signal/signallogin/viewdetails/SignalLoginViewDetailsScreenEvents.kt b/lib/signal-login/src/main/java/org/signal/signallogin/viewdetails/SignalLoginViewDetailsScreenEvents.kt index 167fb4e42d..aee81aa33e 100644 --- a/lib/signal-login/src/main/java/org/signal/signallogin/viewdetails/SignalLoginViewDetailsScreenEvents.kt +++ b/lib/signal-login/src/main/java/org/signal/signallogin/viewdetails/SignalLoginViewDetailsScreenEvents.kt @@ -17,17 +17,17 @@ sealed class SignalLoginViewDetailsScreenEvents { /** The user chose to save the credentials as a PDF. */ data object SaveAsPdfClicked : SignalLoginViewDetailsScreenEvents() - /** User long clicked the account ID field. */ - data class AccountIdLongClicked(val aci: String) : SignalLoginViewDetailsScreenEvents() { + /** The user tapped the copy button on the account ID field. */ + data class CopyAccountIdClicked(val aci: String) : SignalLoginViewDetailsScreenEvents() { override fun toString(): String { - return "AccountIdLongClicked(aci=${aci.censor()})" + return "CopyAccountIdClicked(aci=${aci.censor()})" } } - /** User long clicked the recovery key field. */ - data class RecoveryKeyLongClicked(val aep: String) : SignalLoginViewDetailsScreenEvents() { + /** The user tapped the copy button on the recovery key field. */ + data class CopyRecoveryKeyClicked(val aep: String) : SignalLoginViewDetailsScreenEvents() { override fun toString(): String { - return "RecoveryKeyLongClicked(aep=${aep.censor()})" + return "CopyRecoveryKeyClicked(aep=${aep.censor()})" } } } diff --git a/lib/signal-login/src/main/java/org/signal/signallogin/viewdetails/SignalLoginViewDetailsViewModel.kt b/lib/signal-login/src/main/java/org/signal/signallogin/viewdetails/SignalLoginViewDetailsViewModel.kt index a04ae824ff..7cec19b421 100644 --- a/lib/signal-login/src/main/java/org/signal/signallogin/viewdetails/SignalLoginViewDetailsViewModel.kt +++ b/lib/signal-login/src/main/java/org/signal/signallogin/viewdetails/SignalLoginViewDetailsViewModel.kt @@ -63,14 +63,14 @@ class SignalLoginViewDetailsViewModel( Log.i(TAG, "Save as PDF clicked, but the flow isn't implemented yet.") } - is SignalLoginViewDetailsScreenEvents.AccountIdLongClicked -> { + is SignalLoginViewDetailsScreenEvents.CopyAccountIdClicked -> { // TODO [phonenumberless] Copy the account key to the clipboard. - Log.i(TAG, "Account key long clicked, but the copy flow isn't implemented yet.") + Log.i(TAG, "Account key copy clicked, but the copy flow isn't implemented yet.") } - is SignalLoginViewDetailsScreenEvents.RecoveryKeyLongClicked -> { + is SignalLoginViewDetailsScreenEvents.CopyRecoveryKeyClicked -> { // TODO [phonenumberless] Copy the recovery key to the clipboard. - Log.i(TAG, "Recovery key long clicked, but the copy flow isn't implemented yet.") + Log.i(TAG, "Recovery key copy clicked, but the copy flow isn't implemented yet.") } } } diff --git a/lib/signal-login/src/main/res/values/strings.xml b/lib/signal-login/src/main/res/values/strings.xml index f4a9520f52..f6f754bf25 100644 --- a/lib/signal-login/src/main/res/values/strings.xml +++ b/lib/signal-login/src/main/res/values/strings.xml @@ -14,9 +14,13 @@ Navigate back - Account key + Account ID Recovery key + + Copy account ID + + Copy recovery key Save to password manager