A copy into a picked folder says where it went, and Show takes you there
The last acceptance criterion M16 had left open. After Copy to..., the snackbar reads 'Copied to AADst' with a Show button; after a move to a picked folder it keeps the undoable message instead, because the sources are gone and Show would lead nowhere. Two things needed care rather than a string. The action button was hard-wired to undo() - the label was a parameter beside it, which let the two disagree - so UiMessage now carries a MessageAction, and the label follows from it: Undo for an undo, Show for a jump. And a message cannot name a folder from the path alone: a volume root has no name of its own, and internal storage's is 0. The message carries the picked PickerFolder, and the screen resolves its label the way breadcrumbs do - so copying into the root reads 'Copied to Internal storage', which is the case a plain path.name would have got wrong. Four tests: the message names the destination and offers Show; Show opens it; a paste into the folder on screen still names nothing and offers nothing (its destination is the screen); a move to a picked folder offers Undo. All four driven through the view model, and both snackbar variants checked on the phone, including the volume-root one. detekt's SpreadOperator baseline entries moved with the expression, so the message building is now a small helper with a deliberate suppression and a reason, and the baseline was regenerated: three obsolete entries removed, none added.
This commit is contained in:
@@ -320,17 +320,18 @@ fun BrowserScreen(
|
||||
|
||||
|
||||
val message = state.message
|
||||
val messageText = message?.let {
|
||||
it.pluralQuantity?.let { quantity ->
|
||||
pluralStringResource(it.textRes, quantity, *it.args.toTypedArray())
|
||||
} ?: stringResource(it.textRes, *it.args.toTypedArray())
|
||||
// A message that names a folder cannot be built from the path alone: a volume root has no name of its
|
||||
// own (`0`), which is the one thing only the screen can resolve — the same rule breadcrumbs use.
|
||||
val destinationLabel = message?.destination?.let { folder ->
|
||||
folder.labelRes?.let { stringResource(it) } ?: folder.title
|
||||
}
|
||||
val messageText = message?.let { messageText(it, destinationLabel) }
|
||||
// Resolved out here because `stringResource` is a composable call and the effect is not.
|
||||
val undoLabel = message?.actionLabelRes?.let { stringResource(it) }
|
||||
val actionLabel = message?.actionLabelRes?.let { stringResource(it) }
|
||||
LaunchedEffect(message?.id) {
|
||||
if (messageText != null) {
|
||||
val outcome = snackbarHostState.showSnackbar(messageText, actionLabel = undoLabel)
|
||||
if (outcome == SnackbarResult.ActionPerformed) viewModel.undo()
|
||||
val outcome = snackbarHostState.showSnackbar(messageText, actionLabel = actionLabel)
|
||||
if (outcome == SnackbarResult.ActionPerformed) viewModel.performMessageAction()
|
||||
viewModel.consumeMessage()
|
||||
}
|
||||
}
|
||||
@@ -601,3 +602,21 @@ fun BrowserScreen(
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The text of a snackbar message.
|
||||
*
|
||||
* A message that names a folder cannot carry its label as an argument — a volume root has no name of its own
|
||||
* (internal storage's is `0`) — so the screen resolves it the way breadcrumbs are resolved and passes it as
|
||||
* the first argument.
|
||||
*
|
||||
* `SpreadOperator` is suppressed on purpose: the `vararg` resource helpers take an array, and this runs once
|
||||
* per message rather than once per row or per frame, so the copy the rule warns about is not a hot path.
|
||||
*/
|
||||
@Suppress("SpreadOperator")
|
||||
@Composable
|
||||
private fun messageText(message: UiMessage, destinationLabel: String?): String {
|
||||
val args = if (destinationLabel != null) listOf<Any>(destinationLabel) + message.args else message.args
|
||||
return message.pluralQuantity?.let { quantity ->
|
||||
pluralStringResource(message.textRes, quantity, *args.toTypedArray())
|
||||
} ?: stringResource(message.textRes, *args.toTypedArray())
|
||||
}
|
||||
|
||||
@@ -4,6 +4,7 @@ import android.net.Uri
|
||||
import androidx.activity.result.IntentSenderRequest
|
||||
import androidx.annotation.AnyRes
|
||||
import androidx.annotation.StringRes
|
||||
import de.softwareapp_hb.filemanager.R
|
||||
import de.softwareapp_hb.filemanager.core.ConflictPolicy
|
||||
import de.softwareapp_hb.filemanager.core.FileCategory
|
||||
import de.softwareapp_hb.filemanager.core.FileEntry
|
||||
@@ -71,6 +72,21 @@ data class TransferPicker(
|
||||
val loading: Boolean = false,
|
||||
)
|
||||
|
||||
/**
|
||||
* What a snackbar's action button does.
|
||||
*
|
||||
* The label follows from the action rather than being passed beside it, so the two cannot disagree: an
|
||||
* action that offers "Undo" is an undo, and one that offers "Show" is a jump.
|
||||
*/
|
||||
sealed interface MessageAction {
|
||||
|
||||
/** Take the change back — the undo of the operation that has just finished. */
|
||||
data object Undo : MessageAction
|
||||
|
||||
/** Open the folder an operation wrote to. */
|
||||
data class Show(val path: StoragePath) : MessageAction
|
||||
}
|
||||
|
||||
/** A one-shot message for a snackbar. [id] makes repeats of the same text distinguishable. */
|
||||
data class UiMessage(
|
||||
/** A `@StringRes` or `@PluralsRes` value; see [pluralQuantity]. */
|
||||
@@ -78,10 +94,25 @@ data class UiMessage(
|
||||
val args: List<Any> = emptyList(),
|
||||
/** Set when [textRes] names a `<plurals>` resource; the UI resolves it accordingly. */
|
||||
val pluralQuantity: Int? = null,
|
||||
/** When set, the snackbar offers a single action button carrying this label. */
|
||||
@StringRes val actionLabelRes: Int? = null,
|
||||
/** What the snackbar's action button does; `null` for a message with no action. */
|
||||
val action: MessageAction? = null,
|
||||
/**
|
||||
* A folder the text names, whose label only the UI can resolve: a volume root has no name of its own —
|
||||
* internal storage's is `0`. When set, its title becomes the text's first argument.
|
||||
*/
|
||||
val destination: PickerFolder? = null,
|
||||
val id: Long = System.nanoTime(),
|
||||
)
|
||||
) {
|
||||
|
||||
/** The action button's label, or `null` when there is no action. */
|
||||
@get:StringRes
|
||||
val actionLabelRes: Int?
|
||||
get() = when (action) {
|
||||
MessageAction.Undo -> R.string.browser_action_undo
|
||||
is MessageAction.Show -> R.string.browser_action_show
|
||||
null -> null
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A paste that hit existing names, waiting for the user to choose how to resolve it.
|
||||
|
||||
@@ -555,7 +555,14 @@ class BrowserViewModel(
|
||||
it.copy(conflict = ConflictPrompt(picker.paths, target, picker.cut, conflicts))
|
||||
}
|
||||
} else {
|
||||
transfer(picker.paths, target, picker.cut, ConflictPolicy.KEEP_BOTH)
|
||||
transfer(
|
||||
paths = picker.paths,
|
||||
target = target,
|
||||
cut = picker.cut,
|
||||
policy = ConflictPolicy.KEEP_BOTH,
|
||||
// Only a copy: a move's message is the undoable one, and its sources are gone.
|
||||
announce = if (picker.cut) null else picker.crumbs.lastOrNull() ?: PickerFolder(target),
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -1807,6 +1814,8 @@ class BrowserViewModel(
|
||||
target: StoragePath,
|
||||
cut: Boolean,
|
||||
policy: ConflictPolicy,
|
||||
/** Set when the destination was chosen by hand, so the message can name it and offer to show it. */
|
||||
announce: PickerFolder? = null,
|
||||
) {
|
||||
val source = storage.activeSource() ?: return
|
||||
val operations = FileOperations(source)
|
||||
@@ -1838,6 +1847,8 @@ class BrowserViewModel(
|
||||
_state.update { it.copy(undo = Undo.Relocate(moves)) }
|
||||
}
|
||||
messageUndoablePlural(R.plurals.message_moved, result.completed)
|
||||
} else if (announce != null) {
|
||||
messageCopiedTo(announce, result.completed)
|
||||
} else {
|
||||
messagePlural(R.plurals.message_copied, result.completed)
|
||||
}
|
||||
@@ -2224,6 +2235,20 @@ class BrowserViewModel(
|
||||
|
||||
// ----------------------------------------------------------------- messages
|
||||
|
||||
/**
|
||||
* Run what the current message's action button offered.
|
||||
*
|
||||
* Undo is the usual one. A **Copy to…** offers Show instead, because a copy leaves its sources where
|
||||
* they were — the interesting place is the one it wrote to, which may be several folders away.
|
||||
*/
|
||||
fun performMessageAction() {
|
||||
when (val action = _state.value.message?.action) {
|
||||
MessageAction.Undo -> undo()
|
||||
is MessageAction.Show -> openDirectory(action.path)
|
||||
null -> Unit
|
||||
}
|
||||
}
|
||||
|
||||
fun consumeMessage() {
|
||||
_state.update { it.copy(message = null) }
|
||||
}
|
||||
@@ -2246,7 +2271,7 @@ class BrowserViewModel(
|
||||
/** A message whose snackbar also offers to take the change back. */
|
||||
private fun messageUndoable(@StringRes textRes: Int, vararg args: Any) {
|
||||
postMessage(
|
||||
UiMessage(textRes, args.toList(), actionLabelRes = R.string.browser_action_undo),
|
||||
UiMessage(textRes, args.toList(), action = MessageAction.Undo),
|
||||
)
|
||||
}
|
||||
|
||||
@@ -2256,7 +2281,20 @@ class BrowserViewModel(
|
||||
textRes,
|
||||
listOf(count),
|
||||
pluralQuantity = count,
|
||||
actionLabelRes = R.string.browser_action_undo,
|
||||
action = MessageAction.Undo,
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
/** A copy into a folder the user picked: names it, and offers to go there. */
|
||||
private fun messageCopiedTo(destination: PickerFolder, count: Int) {
|
||||
postMessage(
|
||||
UiMessage(
|
||||
R.plurals.message_copied_to,
|
||||
listOf(count),
|
||||
pluralQuantity = count,
|
||||
action = MessageAction.Show(destination.path),
|
||||
destination = destination,
|
||||
),
|
||||
)
|
||||
}
|
||||
|
||||
@@ -125,6 +125,8 @@
|
||||
<string name="browser_action_select_all">Select all</string>
|
||||
<string name="browser_action_clear_selection">Clear selection</string>
|
||||
<string name="browser_action_undo">Undo</string>
|
||||
<!-- The action on a "copied to X" message: go to the folder it was copied into. -->
|
||||
<string name="browser_action_show">Show</string>
|
||||
|
||||
<!-- ===== Browser: files another app has handed over ===== -->
|
||||
<plurals name="incoming_save_here">
|
||||
@@ -238,6 +240,12 @@
|
||||
<string name="message_folder_created">Folder created</string>
|
||||
<string name="message_renamed">Renamed</string>
|
||||
<string name="message_name_in_use">That name is already in use</string>
|
||||
<!-- %1$s is the destination folder; a copy that went somewhere the user chose says where. -->
|
||||
<plurals name="message_copied_to">
|
||||
<item quantity="one">Copied to %1$s</item>
|
||||
<item quantity="other">Copied %2$d items to %1$s</item>
|
||||
</plurals>
|
||||
|
||||
<plurals name="message_copied">
|
||||
<item quantity="one">Copied %1$d item</item>
|
||||
<item quantity="other">Copied %1$d items</item>
|
||||
|
||||
@@ -353,6 +353,92 @@ class BrowserViewModelTest {
|
||||
assertTrue(recordedNotifications.isEmpty())
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a copy says where it went and offers to show it`() {
|
||||
val model = viewModel()
|
||||
model.openDirectory(downloads)
|
||||
model.toggleSelection(downloads.child("a.txt"))
|
||||
|
||||
model.startTransferTo(cut = false)
|
||||
model.pickerOpen(documents)
|
||||
model.confirmTransferTo()
|
||||
|
||||
val message = model.state.value.message
|
||||
assertNotNull(message)
|
||||
assertEquals(R.plurals.message_copied_to, message!!.textRes)
|
||||
// The count is the only argument the view model owns; the folder's label is the screen's job.
|
||||
assertEquals(listOf(1), message.args)
|
||||
assertEquals(documents, message.destination?.path)
|
||||
assertEquals(MessageAction.Show(documents), message.action)
|
||||
assertEquals(R.string.browser_action_show, message.actionLabelRes)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `show takes the user to the folder the copy went to`() {
|
||||
val model = viewModel()
|
||||
model.openDirectory(downloads)
|
||||
model.toggleSelection(downloads.child("a.txt"))
|
||||
|
||||
model.startTransferTo(cut = false)
|
||||
model.pickerOpen(documents)
|
||||
model.confirmTransferTo()
|
||||
model.performMessageAction()
|
||||
|
||||
assertEquals(documents, model.state.value.path)
|
||||
assertTrue(storage.contains(documents.child("a.txt")))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a paste into the folder on screen names no destination and offers no action`() {
|
||||
val model = viewModel()
|
||||
model.openDirectory(root)
|
||||
model.copyToClipboard(listOf(downloads.child("a.txt")), cut = false)
|
||||
|
||||
model.openDirectory(documents)
|
||||
model.paste()
|
||||
|
||||
val message = model.state.value.message
|
||||
assertNotNull(message)
|
||||
assertEquals(R.plurals.message_copied, message!!.textRes)
|
||||
assertNull(message.destination)
|
||||
assertNull(message.action)
|
||||
assertNull(message.actionLabelRes)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `the message's undo action is the undo`() {
|
||||
val model = viewModel()
|
||||
model.openDirectory(downloads)
|
||||
model.toggleSelection(downloads.child("a.txt"))
|
||||
|
||||
model.startTransferTo(cut = true)
|
||||
model.pickerOpen(documents)
|
||||
model.confirmTransferTo()
|
||||
assertFalse(storage.contains(downloads.child("a.txt")))
|
||||
|
||||
// What the snackbar's button does — the UI no longer calls `undo()` itself, so this is the path
|
||||
// that has to work for Undo to work at all.
|
||||
model.performMessageAction()
|
||||
|
||||
assertTrue(storage.contains(downloads.child("a.txt")))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a move to a chosen folder offers undo rather than show`() {
|
||||
val model = viewModel()
|
||||
model.openDirectory(downloads)
|
||||
model.toggleSelection(downloads.child("a.txt"))
|
||||
|
||||
model.startTransferTo(cut = true)
|
||||
model.pickerOpen(documents)
|
||||
model.confirmTransferTo()
|
||||
|
||||
val message = model.state.value.message
|
||||
assertNotNull(message)
|
||||
assertEquals(MessageAction.Undo, message!!.action)
|
||||
assertEquals(R.string.browser_action_undo, message.actionLabelRes)
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `navigating up goes to the parent then to the roots`() {
|
||||
val model = viewModel()
|
||||
|
||||
Reference in New Issue
Block a user