PM-43235: bug: Remove edit button for disabled Send types (#7374)

This commit is contained in:
David Perez
2026-09-10 13:51:32 +00:00
committed by GitHub
parent 051b2acd0e
commit e02593a5a5
9 changed files with 219 additions and 75 deletions
@@ -23,6 +23,7 @@ import com.bitwarden.ui.platform.resource.BitwardenString
import com.x8bit.bitwarden.ui.platform.components.listitem.BitwardenGroupItem
import com.x8bit.bitwarden.ui.tools.feature.send.handlers.SendHandlers
import com.x8bit.bitwarden.ui.tools.feature.send.model.UpgradedToPremiumCardData
import com.x8bit.bitwarden.ui.vault.feature.itemlisting.model.ListingItemOverflowAction
private const val SEND_TYPES_COUNT: Int = 2
@@ -108,10 +109,13 @@ fun SendContent(
.standardHorizontalMargin(),
)
}
item {
Spacer(modifier = Modifier.height(height = 16.dp))
}
}
item {
Spacer(modifier = Modifier.height(16.dp))
BitwardenListHeaderText(
label = stringResource(id = BitwardenString.all_sends),
supportingLabel = state.sendItems.size.toString(),
@@ -131,16 +135,34 @@ fun SendContent(
trailingLabelIcons = it.iconList,
showMoreOptions = !policyDisablesSend,
onClick = { sendHandlers.onSendClick(it) },
onViewClick = { sendHandlers.onViewSendClick(it) },
onCopyClick = { sendHandlers.onCopySendClick(it) },
onEditClick = { sendHandlers.onEditSendClick(it) },
onShareClick = { sendHandlers.onShareSendClick(it) },
onDeleteClick = { sendHandlers.onDeleteSendClick(it) },
onRemovePasswordClick = if (it.hasPassword) {
{ sendHandlers.onRemovePasswordClick(it) }
} else {
null
onOverflowAction = { action ->
when (action) {
is ListingItemOverflowAction.SendAction.CopyUrlClick -> {
sendHandlers.onCopySendClick(it)
}
is ListingItemOverflowAction.SendAction.DeleteClick -> {
sendHandlers.onDeleteSendClick(it)
}
is ListingItemOverflowAction.SendAction.EditClick -> {
sendHandlers.onEditSendClick(it)
}
is ListingItemOverflowAction.SendAction.RemovePasswordClick -> {
sendHandlers.onRemovePasswordClick(it)
}
is ListingItemOverflowAction.SendAction.ShareUrlClick -> {
sendHandlers.onShareSendClick(it)
}
is ListingItemOverflowAction.SendAction.ViewClick -> {
sendHandlers.onViewSendClick(it)
}
}
},
overflowOptions = it.overflowItems,
cardStyle = state
.sendItems
.toListItemCardStyle(index = index, dividerPadding = 56.dp),
@@ -7,17 +7,15 @@ import androidx.compose.runtime.saveable.rememberSaveable
import androidx.compose.runtime.setValue
import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.painter.Painter
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.tooling.preview.Preview
import com.bitwarden.core.util.persistentListOfNotNull
import com.bitwarden.ui.platform.components.dialog.BitwardenTwoButtonDialog
import com.bitwarden.ui.platform.components.icon.model.IconData
import com.bitwarden.ui.platform.components.model.CardStyle
import com.bitwarden.ui.platform.resource.BitwardenDrawable
import com.bitwarden.ui.platform.resource.BitwardenString
import com.bitwarden.ui.platform.theme.BitwardenTheme
import com.x8bit.bitwarden.ui.platform.components.listitem.BitwardenListItem
import com.x8bit.bitwarden.ui.platform.components.listitem.SelectionItemData
import com.x8bit.bitwarden.ui.vault.feature.itemlisting.model.ListingItemOverflowAction
import kotlinx.collections.immutable.ImmutableList
import kotlinx.collections.immutable.persistentListOf
import kotlinx.collections.immutable.toPersistentList
@@ -26,8 +24,9 @@ import kotlinx.collections.immutable.toPersistentList
* A Composable function that displays a row send item.
*
* @param label The primary text label to display for the item.
* @param supportingLabel An secondary text label to display beneath the label.
* @param supportingLabel A secondary text label to display beneath the label.
* @param startIcon The [Painter] object used to draw the icon at the start of the item.
* @param isDisabled Whether the item is disabled or not.
* @param showMoreOptions Whether to show the button for the overflow options.
* @param onClick The lambda to be invoked when the item is clicked.
* @param onViewClick The lambda to be invoked when the view option is clicked from the menu.
@@ -50,54 +49,33 @@ fun SendListItem(
trailingLabelIcons: ImmutableList<IconData>,
showMoreOptions: Boolean,
onClick: () -> Unit,
onViewClick: () -> Unit,
onEditClick: () -> Unit,
onCopyClick: () -> Unit,
onShareClick: () -> Unit,
onDeleteClick: () -> Unit,
onRemovePasswordClick: (() -> Unit)?,
onOverflowAction: (ListingItemOverflowAction.SendAction) -> Unit,
overflowOptions: ImmutableList<ListingItemOverflowAction.SendAction>,
cardStyle: CardStyle,
modifier: Modifier = Modifier,
) {
var shouldShowDeleteConfirmationDialog by rememberSaveable { mutableStateOf(false) }
var speedBumpAction: ListingItemOverflowAction.SendAction? by rememberSaveable {
mutableStateOf(null)
}
BitwardenListItem(
label = label,
supportingLabel = supportingLabel,
startIcon = startIcon,
trailingLabelIcons = trailingLabelIcons,
onClick = onClick,
selectionDataList = persistentListOfNotNull(
SelectionItemData(
text = stringResource(id = BitwardenString.copy_link),
onClick = onCopyClick,
),
SelectionItemData(
text = stringResource(id = BitwardenString.share_link),
contentDescription = stringResource(
id = BitwardenString.external_link_format,
formatArgs = arrayOf(stringResource(id = BitwardenString.share_link)),
),
onClick = onShareClick,
),
SelectionItemData(
text = stringResource(id = BitwardenString.view),
onClick = onViewClick,
),
SelectionItemData(
text = stringResource(id = BitwardenString.edit),
onClick = onEditClick,
),
onRemovePasswordClick?.let {
selectionDataList = overflowOptions
.map { action ->
SelectionItemData(
text = stringResource(id = BitwardenString.remove_password),
onClick = it,
text = action.title(),
onClick = {
action
.speedBump
?.let { speedBumpAction = action }
?: onOverflowAction(action)
},
contentDescription = action.contentDescription(),
)
},
SelectionItemData(
text = stringResource(id = BitwardenString.delete),
onClick = { shouldShowDeleteConfirmationDialog = true },
),
)
}
// Only show options if allowed
.filter { showMoreOptions }
.toPersistentList(),
@@ -105,19 +83,25 @@ fun SendListItem(
cardStyle = cardStyle,
modifier = modifier,
)
if (shouldShowDeleteConfirmationDialog) {
BitwardenTwoButtonDialog(
title = stringResource(id = BitwardenString.delete),
message = stringResource(id = BitwardenString.are_you_sure_delete_send),
confirmButtonText = stringResource(id = BitwardenString.yes),
dismissButtonText = stringResource(id = BitwardenString.cancel),
onConfirmClick = {
shouldShowDeleteConfirmationDialog = false
onDeleteClick()
},
onDismissClick = { shouldShowDeleteConfirmationDialog = false },
onDismissRequest = { shouldShowDeleteConfirmationDialog = false },
)
speedBumpAction?.let { action ->
action
.speedBump
?.let { speedBump ->
BitwardenTwoButtonDialog(
twoButtonDialogData = speedBump,
onConfirmClick = {
speedBumpAction = null
onOverflowAction(action)
},
onDismissClick = { speedBumpAction = null },
onDismissRequest = { speedBumpAction = null },
)
}
?: run {
// If we somehow get here and there is no speed bump, then we should keep on going.
speedBumpAction = null
onOverflowAction(action)
}
}
}
@@ -131,13 +115,9 @@ private fun SendListItem_preview() {
startIcon = IconData.Local(BitwardenDrawable.ic_file_text),
trailingLabelIcons = persistentListOf(),
showMoreOptions = true,
overflowOptions = persistentListOf(),
onOverflowAction = { },
onClick = {},
onCopyClick = {},
onViewClick = {},
onEditClick = {},
onShareClick = {},
onDeleteClick = {},
onRemovePasswordClick = null,
cardStyle = CardStyle.Full,
)
}
@@ -34,6 +34,7 @@ import com.x8bit.bitwarden.ui.tools.feature.send.model.SendItemType
import com.x8bit.bitwarden.ui.tools.feature.send.util.toSendItemType
import com.x8bit.bitwarden.ui.tools.feature.send.util.toViewState
import com.x8bit.bitwarden.ui.vault.feature.item.VaultItemScreen
import com.x8bit.bitwarden.ui.vault.feature.itemlisting.model.ListingItemOverflowAction
import dagger.hilt.android.lifecycle.HiltViewModel
import kotlinx.collections.immutable.ImmutableList
import kotlinx.coroutines.delay
@@ -66,7 +67,8 @@ class SendViewModel @Inject constructor(
) : BaseViewModel<SendState, SendEvent, SendAction>(
// We load the state from the savedStateHandle for testing purposes.
initialState = savedStateHandle[KEY_STATE]
?: policyManager.getEffectiveSendPolicy().let { effectiveSendPolicy ->
?: run {
val effectiveSendPolicy = policyManager.getEffectiveSendPolicy()
SendState(
viewState = SendState.ViewState.Loading,
dialogState = null,
@@ -565,7 +567,7 @@ data class SendState(
override val shouldDisplayFab: Boolean get() = true
/**
* Represents the an individual send item to be displayed.
* Represents an individual send item to be displayed.
*/
@Parcelize
data class SendItem(
@@ -576,6 +578,7 @@ data class SendState(
val iconList: ImmutableList<IconData>,
val shareUrl: String,
val hasPassword: Boolean,
val overflowItems: ImmutableList<ListingItemOverflowAction.SendAction>,
) : Parcelable {
/**
* Indicates the type of send this, a text or file.
@@ -46,6 +46,7 @@ private fun List<SendView>.toSendContent(
iconList = sendView.toLabelIcons(),
shareUrl = sendView.toSendUrl(baseWebSendUrl),
hasPassword = sendView.hasPassword,
overflowItems = sendView.toOverflowActions(baseWebSendUrl),
)
},
)
@@ -47,7 +47,8 @@ fun SendView.toOverflowActions(
sendUrl = toSendUrl(baseWebSendUrl = baseWebSendUrl),
),
ListingItemOverflowAction.SendAction.ViewClick(sendId = sendId, sendType = type),
ListingItemOverflowAction.SendAction.EditClick(sendId = sendId, sendType = type),
ListingItemOverflowAction.SendAction.EditClick(sendId = sendId, sendType = type)
.takeUnless { this.disabled },
ListingItemOverflowAction.SendAction.RemovePasswordClick(sendId = sendId).takeIf {
hasPassword
},
@@ -100,7 +100,7 @@ sealed class ListingItemOverflowAction : Parcelable {
data class DeleteClick(val sendId: String) : SendAction() {
override val title: Text get() = BitwardenString.delete.asText()
override val contentDescription: Text get() = title
override val speedBump: BitwardenTwoButtonDialogData?
override val speedBump: BitwardenTwoButtonDialogData
get() = BitwardenTwoButtonDialogData(
title = BitwardenString.delete.asText(),
message = BitwardenString.are_you_sure_delete_send.asText(),
@@ -296,7 +296,7 @@ sealed class ListingItemOverflowAction : Parcelable {
override val title: Text get() = BitwardenString.archive_verb.asText()
override val requiresPasswordReprompt: Boolean get() = true
override val contentDescription: Text get() = title
override val speedBump: BitwardenTwoButtonDialogData?
override val speedBump: BitwardenTwoButtonDialogData
get() = BitwardenTwoButtonDialogData(
title = BitwardenString.archive_item.asText(),
message = BitwardenString.once_archived_this_item_will_be_excluded.asText(),
@@ -23,6 +23,7 @@ import androidx.compose.ui.test.performScrollTo
import androidx.compose.ui.test.performScrollToNode
import androidx.core.net.toUri
import com.bitwarden.core.data.repository.util.bufferedMutableSharedFlow
import com.bitwarden.send.SendType
import com.bitwarden.ui.platform.components.snackbar.model.BitwardenSnackbarData
import com.bitwarden.ui.platform.manager.IntentManager
import com.bitwarden.ui.util.asText
@@ -34,11 +35,13 @@ import com.x8bit.bitwarden.ui.tools.feature.send.addedit.AddEditSendRoute
import com.x8bit.bitwarden.ui.tools.feature.send.addedit.ModeType
import com.x8bit.bitwarden.ui.tools.feature.send.model.SendItemType
import com.x8bit.bitwarden.ui.tools.feature.send.viewsend.ViewSendRoute
import com.x8bit.bitwarden.ui.vault.feature.itemlisting.model.ListingItemOverflowAction
import io.mockk.every
import io.mockk.just
import io.mockk.mockk
import io.mockk.runs
import io.mockk.verify
import kotlinx.collections.immutable.ImmutableList
import kotlinx.collections.immutable.persistentListOf
import kotlinx.coroutines.flow.MutableStateFlow
import kotlinx.coroutines.flow.update
@@ -621,6 +624,69 @@ class SendScreenTest : BitwardenComposeTest() {
composeTestRule.assertNoDialogExists()
}
@Suppress("MaxLineLength")
@Test
fun `on send item overflow dialog only the overflow items present on the send should be displayed`() {
mutableStateFlow.update {
it.copy(
viewState = SendState.ViewState.Content(
textTypeCount = 0,
fileTypeCount = 1,
sendItems = listOf(
DEFAULT_SEND_ITEM.copy(
// A disabled send has no edit action available.
overflowItems = persistentListOf(
ListingItemOverflowAction.SendAction.ViewClick(
sendId = "mockId-1",
sendType = SendType.FILE,
),
ListingItemOverflowAction.SendAction.DeleteClick(
sendId = "mockId-1",
),
),
),
DEFAULT_SEND_ITEM.copy(id = "mockId-2", name = "mockName-2"),
),
),
)
}
composeTestRule.assertNoDialogExists()
// We scroll to the last item but click the first one to avoid clicking the FAB by mistake
composeTestRule
.onNodeWithText("mockName-2")
.performScrollTo()
.assertIsDisplayed()
composeTestRule
.onNodeWithText("mockName-1")
.onChildren()
.filterToOne(hasContentDescription("More options"))
.assertIsDisplayed()
.performClick()
composeTestRule
.onNodeWithText("View")
.assert(hasAnyAncestor(isDialog()))
.assertIsDisplayed()
composeTestRule
.onNodeWithText("Delete")
.assert(hasAnyAncestor(isDialog()))
.assertIsDisplayed()
composeTestRule
.onNodeWithText("Edit")
.assertDoesNotExist()
composeTestRule
.onNodeWithText("Copy link")
.assertDoesNotExist()
composeTestRule
.onNodeWithText("Share link")
.assertDoesNotExist()
composeTestRule
.onNodeWithText("Remove password")
.assertDoesNotExist()
}
@Test
fun `on send item overflow dialog copy click should send CopyClick`() {
mutableStateFlow.update {
@@ -1135,6 +1201,26 @@ private val DEFAULT_STATE: SendState = SendState(
isPremiumUser = false,
)
private val DEFAULT_SEND_OVERFLOW_ITEMS: ImmutableList<ListingItemOverflowAction.SendAction> =
persistentListOf(
ListingItemOverflowAction.SendAction.CopyUrlClick(
sendUrl = "www.test.com/#/send/mockAccessId-1/mockKey-1",
),
ListingItemOverflowAction.SendAction.ShareUrlClick(
sendUrl = "www.test.com/#/send/mockAccessId-1/mockKey-1",
),
ListingItemOverflowAction.SendAction.ViewClick(
sendId = "mockId-1",
sendType = SendType.FILE,
),
ListingItemOverflowAction.SendAction.EditClick(
sendId = "mockId-1",
sendType = SendType.FILE,
),
ListingItemOverflowAction.SendAction.RemovePasswordClick(sendId = "mockId-1"),
ListingItemOverflowAction.SendAction.DeleteClick(sendId = "mockId-1"),
)
private val DEFAULT_SEND_ITEM: SendState.ViewState.Content.SendItem =
SendState.ViewState.Content.SendItem(
id = "mockId-1",
@@ -1144,6 +1230,7 @@ private val DEFAULT_SEND_ITEM: SendState.ViewState.Content.SendItem =
iconList = persistentListOf(),
shareUrl = "www.test.com/#/send/mockAccessId-1/mockKey-1",
hasPassword = true,
overflowItems = DEFAULT_SEND_OVERFLOW_ITEMS,
)
private val DEFAULT_CONTENT_VIEW_STATE: SendState.ViewState.Content = SendState.ViewState.Content(
@@ -1159,6 +1246,7 @@ private val DEFAULT_CONTENT_VIEW_STATE: SendState.ViewState.Content = SendState.
iconList = persistentListOf(),
shareUrl = "www.test.com/#/send/mockAccessId-1/mockKey-1",
hasPassword = true,
overflowItems = DEFAULT_SEND_OVERFLOW_ITEMS,
),
),
)
@@ -7,6 +7,7 @@ import com.x8bit.bitwarden.data.vault.datasource.sdk.model.createMockSendView
import com.x8bit.bitwarden.data.vault.repository.model.SendData
import com.x8bit.bitwarden.ui.tools.feature.send.SendState
import com.x8bit.bitwarden.ui.tools.feature.send.model.SendStatusIcon
import com.x8bit.bitwarden.ui.vault.feature.itemlisting.model.ListingItemOverflowAction
import io.mockk.every
import io.mockk.mockkStatic
import io.mockk.unmockkStatic
@@ -31,6 +32,7 @@ class SendDataExtensionsTest {
fun setup() {
mockkStatic(
SendView::toLabelIcons,
SendView::toOverflowActions,
SendView::toSendUrl,
)
}
@@ -39,6 +41,7 @@ class SendDataExtensionsTest {
fun tearDown() {
unmockkStatic(
SendView::toLabelIcons,
SendView::toOverflowActions,
SendView::toSendUrl,
)
}
@@ -67,6 +70,12 @@ class SendDataExtensionsTest {
every { fileSendView.toSendUrl(DEFAULT_BASE_URL) } returns textSendViewUrl1
every { textSendView.toLabelIcons(any()) } returns DEFAULT_SEND_STATUS_ICONS
every { fileSendView.toLabelIcons(any()) } returns DEFAULT_SEND_STATUS_ICONS
every {
textSendView.toOverflowActions(DEFAULT_BASE_URL)
} returns TEXT_SEND_OVERFLOW_ACTIONS
every {
fileSendView.toOverflowActions(DEFAULT_BASE_URL)
} returns FILE_SEND_OVERFLOW_ACTIONS
val result = sendData.toViewState(DEFAULT_BASE_URL, fixedClock)
@@ -83,6 +92,7 @@ class SendDataExtensionsTest {
iconList = DEFAULT_SEND_STATUS_ICONS,
shareUrl = "www.test.com/#/send/mockAccessId-1/mockKey-1",
hasPassword = true,
overflowItems = FILE_SEND_OVERFLOW_ACTIONS,
),
SendState.ViewState.Content.SendItem(
id = "mockId-2",
@@ -92,6 +102,7 @@ class SendDataExtensionsTest {
iconList = DEFAULT_SEND_STATUS_ICONS,
shareUrl = "www.test.com/#/send/mockAccessId-2/mockKey-2",
hasPassword = true,
overflowItems = TEXT_SEND_OVERFLOW_ACTIONS,
),
),
),
@@ -102,6 +113,24 @@ class SendDataExtensionsTest {
private const val DEFAULT_BASE_URL: String = "www.test.com/"
private val FILE_SEND_OVERFLOW_ACTIONS: ImmutableList<ListingItemOverflowAction.SendAction> =
persistentListOf(
ListingItemOverflowAction.SendAction.ViewClick(
sendId = "mockId-1",
sendType = SendType.FILE,
),
ListingItemOverflowAction.SendAction.DeleteClick(sendId = "mockId-1"),
)
private val TEXT_SEND_OVERFLOW_ACTIONS: ImmutableList<ListingItemOverflowAction.SendAction> =
persistentListOf(
ListingItemOverflowAction.SendAction.ViewClick(
sendId = "mockId-2",
sendType = SendType.TEXT,
),
ListingItemOverflowAction.SendAction.DeleteClick(sendId = "mockId-2"),
)
private val DEFAULT_SEND_STATUS_ICONS: ImmutableList<IconData> = persistentListOf(
IconData.Local(
iconRes = SendStatusIcon.DISABLED.iconRes,
@@ -101,6 +101,26 @@ class SendViewExtensionsTest {
)
}
@Suppress("MaxLineLength")
@Test
fun `toOverflowActions should return overflow options without Edit when the send is disabled`() {
val baseWebSendUrl = "www.test.com"
val sendView = createMockSendView(
number = 1,
// Make sure the send is disabled to remove the edit action
disabled = true,
)
val result = sendView.toOverflowActions(baseWebSendUrl = baseWebSendUrl)
assertEquals(
ALL_SEND_OVERFLOW_OPTIONS.filter {
it !is ListingItemOverflowAction.SendAction.EditClick
},
result,
)
}
@Test
fun `toOverflowActions should return no overflow options when the id is null`() {
val baseWebSendUrl = "www.test.com"