Skip to content

Messaging: add shortcuts for copying phone numbers #251

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Olangu wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Messaging: add shortcuts for copying phone numbers #251

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .idea/vcs.xml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions ...ing/ui/conversation/messages/ui/message/rendering/BaseConversationMessageRenderingTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ internal abstract class BaseConversationMessageRenderingTest {
protected val onExternalUriClick = mockk<(String) -> Unit>(relaxed = true)
protected val onMessageClick = mockk<() -> Unit>(relaxed = true)
protected val onMessageLongClick = mockk<() -> Unit>(relaxed = true)
protected val onPhoneNumberCopy = mockk<(String) -> Unit>(relaxed = true)
protected val onResendClick = mockk<() -> Unit>(relaxed = true)
protected val onSimSelectorClick = mockk<() -> Unit>(relaxed = true)

Expand Down Expand Up @@ -67,6 +68,7 @@ internal abstract class BaseConversationMessageRenderingTest {
onMessageAvatarClick = onAvatarClick,
onMessageDownloadClick = onDownloadClick,
onMessageLongClick = onMessageLongClick,
onPhoneNumberCopy = onPhoneNumberCopy,
onMessageResendClick = onResendClick,
onSimSelectorClick = onSimSelectorClick,
)
Expand Down
134 changes: 134 additions & 0 deletions ...om/android/messaging/ui/common/components/participant/ParticipantQuickActionsPopupTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
package com.android.messaging.ui.common.components.participant

import androidx.compose.material.icons.Icons
import androidx.compose.material.icons.filled.Person
import androidx.compose.ui.semantics.SemanticsActions
import androidx.compose.ui.test.assertIsDisplayed
import androidx.compose.ui.test.junit4.createComposeRule
import androidx.compose.ui.test.onNodeWithContentDescription
import androidx.compose.ui.test.onNodeWithTag
import androidx.compose.ui.test.onNodeWithText
import androidx.compose.ui.test.performClick
import androidx.compose.ui.test.performSemanticsAction
import com.android.common.test.helpers.targetContext
import com.android.messaging.R
import com.android.messaging.ui.core.AppTheme
import kotlinx.collections.immutable.ImmutableList
import kotlinx.collections.immutable.persistentListOf
import org.junit.Assert.assertEquals
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.robolectric.RobolectricTestRunner

@RunWith(RobolectricTestRunner::class)
internal class ParticipantQuickActionsPopupTest {

@get:Rule
val composeTestRule = createComposeRule()

@Test
fun copyAreaClick_withOneTarget_copiesPhoneNumber() {
var copiedPhoneNumber: String? = null
setPopup(
targets = persistentListOf(target("Ada", "+1 555 0001")),
onPhoneNumberCopy = { copiedPhoneNumber = it },
)

composeTestRule
.onNodeWithTag(PARTICIPANT_QUICK_ACTIONS_COPY_AREA_TEST_TAG)
.performClick()

assertEquals("+1 555 0001", copiedPhoneNumber)
}

@Test
fun copyAreaLongClick_withOneTarget_copiesPhoneNumber() {
var copiedPhoneNumber: String? = null
setPopup(
targets = persistentListOf(target("Ada", "+1 555 0001")),
onPhoneNumberCopy = { copiedPhoneNumber = it },
)

composeTestRule
.onNodeWithTag(PARTICIPANT_QUICK_ACTIONS_COPY_AREA_TEST_TAG)
.performSemanticsAction(SemanticsActions.OnLongClick)

assertEquals("+1 555 0001", copiedPhoneNumber)
}

@Test
fun copyAreaClick_withMultipleTargets_showsChooserAndCopiesSelection() {
var copiedPhoneNumber: String? = null
setPopup(
targets = persistentListOf(
target("Ada", "+1 555 0001"),
target("Grace", "+1 555 0002"),
),
onPhoneNumberCopy = { copiedPhoneNumber = it },
)

composeTestRule
.onNodeWithTag(PARTICIPANT_QUICK_ACTIONS_COPY_AREA_TEST_TAG)
.performClick()
composeTestRule
.onNodeWithText("Grace")
.assertIsDisplayed()
.performClick()

assertEquals("+1 555 0002", copiedPhoneNumber)
}

@Test
fun actionButtonClick_doesNotCopyPhoneNumber() {
var messageClicks = 0
var copyClicks = 0
setPopup(
targets = persistentListOf(target("Ada", "+1 555 0001")),
onMessageClick = { messageClicks++ },
onPhoneNumberCopy = { copyClicks++ },
)

composeTestRule
.onNodeWithContentDescription(
targetContext.getString(R.string.action_send_message),
)
.performClick()

assertEquals(1, messageClicks)
assertEquals(0, copyClicks)
}

private fun setPopup(
targets: ImmutableList<PhoneNumberCopyTarget>,
onMessageClick: () -> Unit = {},
onPhoneNumberCopy: (String) -> Unit,
) {
composeTestRule.setContent {
AppTheme {
ParticipantQuickActionsPopup(
visible = true,
avatarUri = null,
displayName = "Conversation",
subtitle = null,
fallbackIcon = Icons.Default.Person,
fallbackLabel = "C",
onDismiss = {},
onMessageClick = onMessageClick,
onCallClick = {},
onContactClick = {},
onInfoClick = {},
colorSeedCode = null,
phoneNumberCopyTargets = targets,
onPhoneNumberCopy = onPhoneNumberCopy,
)
}
}
composeTestRule.waitForIdle()
}

private fun target(displayName: String, phoneNumber: String) = PhoneNumberCopyTarget(
displayName = displayName,
phoneNumber = phoneNumber,
)
}
50 changes: 50 additions & 0 deletions ...ui/conversation/messages/ui/message/rendering/ConversationMessageBubbleInteractionTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package com.android.messaging.ui.conversation.messages.ui.message.rendering

import androidx.compose.ui.semantics.SemanticsActions
import androidx.compose.ui.test.assertIsSelected
import androidx.compose.ui.test.onNodeWithTag
import androidx.compose.ui.test.onNodeWithText
import androidx.compose.ui.test.performClick
import androidx.compose.ui.test.performSemanticsAction
import com.android.messaging.ui.conversation.conversationMessageSelectionRowTestTag
import com.android.messaging.ui.conversation.messages.model.message.ConversationMessageUiModel
import com.android.messaging.ui.conversation.messages.model.message.MmsDownloadUiModel
Expand Down Expand Up @@ -222,6 +224,54 @@ internal class ConversationMessageBubbleInteractionTest :
}
}

@Test
fun incomingSenderLongClick_copiesPhoneNumberWithoutSelectingMessage() {
setConversationMessageContent(
message = message(
status = ConversationMessageUiModel.Status.Incoming.Complete,
isIncoming = true,
senderDisplayName = "Nora",
senderNormalizedDestination = "+15550123",
),
)

composeTestRule
.onNodeWithText(text = "Nora")
.performSemanticsAction(SemanticsActions.OnLongClick)

composeTestRule.runOnIdle {
verify(exactly = 1) {
onPhoneNumberCopy.invoke("+15550123")
}
verify(exactly = 0) {
onMessageLongClick.invoke()
}
}
}

@Test
fun incomingMessageBodyLongClick_keepsMessageSelectionBehavior() {
setConversationMessageContent(
message = message(
status = ConversationMessageUiModel.Status.Incoming.Complete,
isIncoming = true,
senderDisplayName = "Nora",
senderNormalizedDestination = "+15550123",
),
)

longClickBubble()

composeTestRule.runOnIdle {
verify(exactly = 1) {
onMessageLongClick.invoke()
}
verify(exactly = 0) {
onPhoneNumberCopy.invoke(any())
}
}
}

@Test
fun incomingAvatarClick_forwardsAvatarClick() {
setConversationMessageContent(
Expand Down
50 changes: 50 additions & 0 deletions ...droid/messaging/ui/conversation/metadata/delegate/ConversationMetadataDelegateImplTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import app.cash.turbine.test
import com.android.messaging.data.blockedparticipants.repository.BlockedParticipantsRepository
import com.android.messaging.data.conversation.model.metadata.ConversationComposerAvailability
import com.android.messaging.data.conversation.model.metadata.ConversationMetadata
import com.android.messaging.data.conversation.model.recipient.ConversationRecipient
import com.android.messaging.data.conversation.repository.ConversationParticipantsRepository
import com.android.messaging.data.conversation.repository.ConversationsRepository
import com.android.messaging.domain.conversation.usecase.action.ConversationActionRequirementsResult
import com.android.messaging.testutil.MainDispatcherRule
Expand All @@ -15,6 +17,8 @@ import io.mockk.coVerify
import io.mockk.every
import io.mockk.mockk
import io.mockk.verify
import kotlinx.collections.immutable.ImmutableList
import kotlinx.collections.immutable.persistentListOf
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.cancel
import kotlinx.coroutines.flow.MutableStateFlow
Expand Down Expand Up @@ -130,6 +134,43 @@ class ConversationMetadataDelegateImplTest {
}
}

@Test
fun participants_areExposedAsPhoneNumberCopyTargets_andEmailIsExcluded() {
runTest(context = mainDispatcherRule.testDispatcher) {
val harness = createHarness(conversationId = "conversation-42")

try {
harness.participantsFlow.value = persistentListOf(
ConversationRecipient(
id = "participant-1",
displayName = "Alice",
destination = "+15550001",
),
ConversationRecipient(
id = "participant-2",
displayName = "Email only",
destination = "person@example.com",
),
)
harness.setPresentState(otherParticipantPhoneNumber = null)
advanceUntilIdle()

val state = harness.delegate.state.value as ConversationMetadataUiState.Present
assertEquals(
persistentListOf(
ConversationMetadataUiState.PhoneNumberCopyTarget(
displayName = "Alice",
phoneNumber = "+15550001",
),
),
state.phoneNumberCopyTargets,
)
} finally {
harness.cancel()
}
}
}

@Test
fun onDeleteConversationClick_togglesConfirmationVisibility() {
runTest(context = mainDispatcherRule.testDispatcher) {
Expand Down Expand Up @@ -241,6 +282,12 @@ class ConversationMetadataDelegateImplTest {
val dispatcher = mainDispatcherRule.testDispatcher
val scope = TestScope(dispatcher)
val conversationsRepository = mockk<ConversationsRepository>(relaxed = true)
val participantsFlow = MutableStateFlow<ImmutableList<ConversationRecipient>>(
persistentListOf(),
)
val conversationParticipantsRepository = mockk<ConversationParticipantsRepository>() {
every { getParticipants(any()) } returns participantsFlow
}
val mapper = mockk<ConversationMetadataUiStateMapper>()
val conversationIdFlow = MutableStateFlow(conversationId)
val metadataFlow = MutableStateFlow<ConversationMetadata?>(value = null)
Expand Down Expand Up @@ -275,6 +322,7 @@ class ConversationMetadataDelegateImplTest {
ConversationActionRequirementsResult.Ready
},
conversationsRepository = conversationsRepository,
conversationParticipantsRepository = conversationParticipantsRepository,
conversationMetadataUiStateMapper = mapper,
blockedParticipantsRepository = mockk<BlockedParticipantsRepository>(relaxed = true),
defaultDispatcher = dispatcher,
Expand All @@ -288,6 +336,7 @@ class ConversationMetadataDelegateImplTest {
delegate = delegate,
conversationsRepository = conversationsRepository,
metadataFlow = metadataFlow,
participantsFlow = participantsFlow,
scope = scope,
)
}
Expand All @@ -304,6 +353,7 @@ class ConversationMetadataDelegateImplTest {
val delegate: ConversationMetadataDelegateImpl,
val conversationsRepository: ConversationsRepository,
val metadataFlow: MutableStateFlow<ConversationMetadata?>,
val participantsFlow: MutableStateFlow<ImmutableList<ConversationRecipient>>,
val scope: TestScope,
) {
fun cancel() {
Expand Down
Loading