diff --git a/CHANGELOG.md b/CHANGELOG.md index 4434c68..29d5aff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserContent.kt b/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserContent.kt index 558dd10..c3b1ebc 100644 --- a/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserContent.kt +++ b/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserContent.kt @@ -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 diff --git a/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserScreen.kt b/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserScreen.kt index 6d52f6a..72a9984 100644 --- a/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserScreen.kt +++ b/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserScreen.kt @@ -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) { diff --git a/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserTopBar.kt b/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserTopBar.kt index ef821fd..52d64bc 100644 --- a/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserTopBar.kt +++ b/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserTopBar.kt @@ -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 diff --git a/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserViewModel.kt b/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserViewModel.kt index 49b23b4..8a6d0e4 100644 --- a/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserViewModel.kt +++ b/app/src/main/java/de/softwareapp_hb/filemanager/ui/browser/BrowserViewModel.kt @@ -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()) } diff --git a/app/src/test/java/de/softwareapp_hb/filemanager/ui/browser/BrowserViewModelTest.kt b/app/src/test/java/de/softwareapp_hb/filemanager/ui/browser/BrowserViewModelTest.kt index 1952bc2..c1197fb 100644 --- a/app/src/test/java/de/softwareapp_hb/filemanager/ui/browser/BrowserViewModelTest.kt +++ b/app/src/test/java/de/softwareapp_hb/filemanager/ui/browser/BrowserViewModelTest.kt @@ -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,