Search from the volume list, and type into the field without a second tap
Two papercuts in the search added in the previous commits, both found by using it. BrowserContent decided between the volume list and a folder listing on path == null alone, so on the "Storage" screen the roots were drawn over the answer: a search everywhere from there was computed and never shown. The switch now also asks whether a search is running - its results are full paths, so nothing has to be open first - and the volume list is drawn only when there is neither a folder nor a search. Opening the field on that screen also switches the scope to everywhere, since "this folder" has no meaning with no folder open and the field's hint would otherwise invite a query that could never match. The view model makes the same fallback on its own, so a query typed from the volume list is answered everywhere whether or not the scope was switched. Tapping the search action now focuses the field and brings the keyboard with it, waiting for the frame the field appears in, because focusing a node that is not attached yet does nothing. Asking the user to tap the box they just opened was a tap of pure ceremony. The wide layout no longer focuses the first root while a search is running, which would have moved the detail pane under the results. 289 JVM tests, lint clean, release APK builds. No device pass: /dev/kvm is still absent from this environment, so the emulator cannot start - the focus change in particular is verified by compile and by reading the API contract, not by watching a keyboard appear.
This commit is contained in:
@@ -12,6 +12,20 @@ upload needs a **higher `versionCode`** than every previous upload.
|
||||
|
||||
### Fixed
|
||||
|
||||
- **Searching needed a volume to be open first, and needed a second tap to be typed into.** Two
|
||||
papercuts in the feature added above, both found by using it. `BrowserContent` decided between
|
||||
the volume list and a folder listing on `path == null` alone, so on the "Storage" screen the
|
||||
roots were drawn *over* the answer: a search everywhere from there was computed and never
|
||||
shown. The switch now also asks whether a search is running — its results are full paths, so
|
||||
nothing has to be open first — and the volume list is drawn only when there is neither a folder
|
||||
nor a search. Opening the field there also switches the scope to everywhere, because "this
|
||||
folder" has no meaning with no folder open and the field's own hint would otherwise invite a
|
||||
query that could never match; the view model makes the same fallback on its own, so a query
|
||||
typed from the volume list is answered everywhere whether or not the scope was switched. And
|
||||
tapping the search action now focuses the field and brings the keyboard up with it: it waited
|
||||
for the frame the field appears in, because focusing a node that is not attached yet does
|
||||
nothing, and asking the user to tap the box they just opened was one tap of pure ceremony.
|
||||
|
||||
- **The progress notification never appeared on Android 13 or newer, and five permissions were
|
||||
declared that the app never used.** `StorageAccessPolicy.missingPermissions` returned an empty
|
||||
list from API 33 on the reasoning that "all files access" is a special permission rather than
|
||||
|
||||
@@ -78,12 +78,17 @@ fun BrowserContent(
|
||||
|
||||
// Landing on "Storage" with a tablet in landscape should already show what is
|
||||
// inside the device's main volume, rather than an empty second pane.
|
||||
LaunchedEffect(wide, state.isRootView, state.roots) {
|
||||
if (wide && state.isRootView && state.focus == null) {
|
||||
LaunchedEffect(wide, state.isRootView, state.roots, state.searchRoot) {
|
||||
if (wide && state.isRootView && state.searchRoot == null && state.focus == null) {
|
||||
state.roots.firstOrNull()?.let(onFocusRoot)
|
||||
}
|
||||
}
|
||||
|
||||
// A search is answered from wherever the user is, including the volume list: its results
|
||||
// are full paths, so nothing has to be open first. That is what decides this switch —
|
||||
// and it is why searching works before a volume has been picked.
|
||||
val showDirectory = !state.isRootView || state.searchRoot != null
|
||||
|
||||
when {
|
||||
state.needsTreeGrant -> StorageAccessPrompt(
|
||||
folderMode = true,
|
||||
@@ -91,6 +96,44 @@ fun BrowserContent(
|
||||
onPickFolder = onPickFolder,
|
||||
)
|
||||
|
||||
showDirectory && wide -> Row(modifier = Modifier.fillMaxSize()) {
|
||||
DirectoryView(
|
||||
state = state,
|
||||
crumbLabel = crumbLabel,
|
||||
wide = true,
|
||||
listState = listState,
|
||||
gridState = gridState,
|
||||
onOpenEntry = onOpenEntry,
|
||||
onFocusEntry = onFocusEntry,
|
||||
onToggleSelection = onToggleSelection,
|
||||
onNavigate = onNavigate,
|
||||
onGrantAllFiles = onGrantAllFiles,
|
||||
onRebuildIndex = onRebuildIndex,
|
||||
modifier = Modifier.width(MasterPaneWidth),
|
||||
)
|
||||
VerticalDivider()
|
||||
DetailPane(
|
||||
focus = state.focus,
|
||||
crumbLabel = crumbLabel,
|
||||
onOpenEntry = onOpenEntry,
|
||||
modifier = Modifier.weight(1f),
|
||||
)
|
||||
}
|
||||
|
||||
showDirectory -> DirectoryView(
|
||||
state = state,
|
||||
crumbLabel = crumbLabel,
|
||||
wide = false,
|
||||
listState = listState,
|
||||
gridState = gridState,
|
||||
onOpenEntry = onOpenEntry,
|
||||
onFocusEntry = onFocusEntry,
|
||||
onToggleSelection = onToggleSelection,
|
||||
onNavigate = onNavigate,
|
||||
onGrantAllFiles = onGrantAllFiles,
|
||||
onRebuildIndex = onRebuildIndex,
|
||||
)
|
||||
|
||||
state.isRootView && wide -> Row(modifier = Modifier.fillMaxSize()) {
|
||||
LazyColumn(modifier = Modifier.width(MasterPaneWidth), state = listState) {
|
||||
items(items = state.roots, key = { it.path.value }) { root ->
|
||||
@@ -116,44 +159,6 @@ fun BrowserContent(
|
||||
RootRow(item = root, onClick = { onOpenRoot(root.path) })
|
||||
}
|
||||
}
|
||||
|
||||
wide -> Row(modifier = Modifier.fillMaxSize()) {
|
||||
DirectoryView(
|
||||
state = state,
|
||||
crumbLabel = crumbLabel,
|
||||
wide = true,
|
||||
listState = listState,
|
||||
gridState = gridState,
|
||||
onOpenEntry = onOpenEntry,
|
||||
onFocusEntry = onFocusEntry,
|
||||
onToggleSelection = onToggleSelection,
|
||||
onNavigate = onNavigate,
|
||||
onGrantAllFiles = onGrantAllFiles,
|
||||
onRebuildIndex = onRebuildIndex,
|
||||
modifier = Modifier.width(MasterPaneWidth),
|
||||
)
|
||||
VerticalDivider()
|
||||
DetailPane(
|
||||
focus = state.focus,
|
||||
crumbLabel = crumbLabel,
|
||||
onOpenEntry = onOpenEntry,
|
||||
modifier = Modifier.weight(1f),
|
||||
)
|
||||
}
|
||||
|
||||
else -> DirectoryView(
|
||||
state = state,
|
||||
crumbLabel = crumbLabel,
|
||||
wide = false,
|
||||
listState = listState,
|
||||
gridState = gridState,
|
||||
onOpenEntry = onOpenEntry,
|
||||
onFocusEntry = onFocusEntry,
|
||||
onToggleSelection = onToggleSelection,
|
||||
onNavigate = onNavigate,
|
||||
onGrantAllFiles = onGrantAllFiles,
|
||||
onRebuildIndex = onRebuildIndex,
|
||||
)
|
||||
}
|
||||
|
||||
// One bar at a time: while the index is being built, the row above this one says what
|
||||
|
||||
@@ -340,7 +340,15 @@ fun BrowserScreen(
|
||||
canPaste = state.clipboard?.isEmpty == false,
|
||||
onUp = viewModel::navigateUp,
|
||||
onQuery = viewModel::setQuery,
|
||||
onSearchOpen = { searchOpen = it },
|
||||
onSearchOpen = { open ->
|
||||
searchOpen = open
|
||||
// With no folder open there is nothing for "this folder" to mean, so the
|
||||
// scope is switched as the field opens: its hint then says what the search
|
||||
// will actually do, instead of inviting a query that could never match.
|
||||
if (open && state.isRootView && state.searchScope != SearchScope.EVERYWHERE) {
|
||||
viewModel.setSearchScope(SearchScope.EVERYWHERE)
|
||||
}
|
||||
},
|
||||
onSearchScope = {
|
||||
viewModel.setSearchScope(
|
||||
if (state.searchScope == SearchScope.EVERYWHERE) {
|
||||
|
||||
@@ -23,11 +23,15 @@ import androidx.compose.material3.OutlinedTextField
|
||||
import androidx.compose.material3.Text
|
||||
import androidx.compose.material3.TopAppBar
|
||||
import androidx.compose.runtime.Composable
|
||||
import androidx.compose.runtime.LaunchedEffect
|
||||
import androidx.compose.runtime.getValue
|
||||
import androidx.compose.runtime.mutableStateOf
|
||||
import androidx.compose.runtime.remember
|
||||
import androidx.compose.runtime.setValue
|
||||
import androidx.compose.runtime.withFrameNanos
|
||||
import androidx.compose.ui.Modifier
|
||||
import androidx.compose.ui.focus.FocusRequester
|
||||
import androidx.compose.ui.focus.focusRequester
|
||||
import androidx.compose.ui.res.stringResource
|
||||
import androidx.compose.ui.text.style.TextOverflow
|
||||
import androidx.compose.ui.unit.dp
|
||||
@@ -54,6 +58,18 @@ fun BrowserTopBar(
|
||||
onTrash: () -> Unit,
|
||||
) {
|
||||
var menuOpen by remember { mutableStateOf(false) }
|
||||
val searchField = remember { FocusRequester() }
|
||||
|
||||
// The keyboard comes with the field. Tapping the search action opens a field the user asked
|
||||
// to type into, and making them tap it a second time to get a keyboard is a tap of pure
|
||||
// ceremony — but focusing a node that is not attached yet does nothing, so this waits for
|
||||
// the frame the field appears in.
|
||||
LaunchedEffect(searchOpen) {
|
||||
if (searchOpen) {
|
||||
withFrameNanos { }
|
||||
runCatching { searchField.requestFocus() }
|
||||
}
|
||||
}
|
||||
|
||||
TopAppBar(
|
||||
// The field takes the bar while it is open, which is what search in an app bar
|
||||
@@ -91,7 +107,9 @@ fun BrowserTopBar(
|
||||
)
|
||||
},
|
||||
singleLine = true,
|
||||
modifier = Modifier.width(SEARCH_FIELD_WIDTH),
|
||||
modifier = Modifier
|
||||
.width(SEARCH_FIELD_WIDTH)
|
||||
.focusRequester(searchField),
|
||||
)
|
||||
// Which folder tree a search answers for is a property of the search, not of
|
||||
// the folder being browsed, so it lives with the field rather than in the
|
||||
|
||||
@@ -306,17 +306,17 @@ class BrowserViewModel(
|
||||
return
|
||||
}
|
||||
|
||||
if (_state.value.searchScope == SearchScope.EVERYWHERE) {
|
||||
// A search started from the volume list has no folder to look in, and "this folder" with
|
||||
// nothing open is not an answer — so it is answered everywhere instead of finding
|
||||
// nothing. The field's own hint says which of the two is happening.
|
||||
val noFolderOpen = _state.value.path == null
|
||||
if (_state.value.searchScope == SearchScope.EVERYWHERE || noFolderOpen) {
|
||||
searchEverywhere(query, source)
|
||||
return
|
||||
}
|
||||
|
||||
val root = _state.value.path
|
||||
if (root == null) {
|
||||
_state.update { it.copy(searchRoot = null, searching = false) }
|
||||
recompute()
|
||||
return
|
||||
}
|
||||
// There is a folder here: the case with none was answered above.
|
||||
val root = _state.value.path ?: return
|
||||
|
||||
_state.update { it.copy(searchRoot = root, searching = true, entries = emptyList()) }
|
||||
|
||||
|
||||
@@ -634,6 +634,30 @@ class BrowserViewModelTest {
|
||||
assertEquals(listOf(".secret.txt"), model.state.value.entries.map { it.name })
|
||||
}
|
||||
|
||||
@Test
|
||||
fun `a search from the volume list is answered everywhere, with no folder open`() {
|
||||
val index = InMemorySearchIndex()
|
||||
storage.addFile(root.child("holiday.png"), "x")
|
||||
runBlocking {
|
||||
index.beginVolume(root.value)
|
||||
index.add(listOf(indexed(root.child("holiday.png"), "holiday.png")))
|
||||
// Current, so this is about the scope and nothing else.
|
||||
index.endVolume(root.value, 1000L, full = true, hiddenIncluded = false)
|
||||
}
|
||||
|
||||
// No folder is opened: the app is still on the volume list, so there is no path.
|
||||
val model = viewModel(searchIndex = index, clock = { 1000L })
|
||||
model.setQuery("holiday")
|
||||
|
||||
assertEquals(listOf("holiday.png"), model.state.value.entries.map { it.name })
|
||||
assertTrue("the volume list is still what is underneath", model.state.value.isRootView)
|
||||
assertEquals(
|
||||
"answered everywhere, because there is no folder to look in",
|
||||
StoragePath.Root,
|
||||
model.state.value.searchRoot,
|
||||
)
|
||||
}
|
||||
|
||||
private fun indexed(path: StoragePath, name: String) = IndexedFile(
|
||||
volume = root.value,
|
||||
path = path,
|
||||
|
||||
Reference in New Issue
Block a user