Improve search logic and UI (#703)

* Don't list the current directory in search results

* Scroll to top result when search term is changed

* Match relative path in StrictDomain filter mode

* Improve and document DirectoryStructure null handling

Signed-off-by: Harsh Shandilya <me@msfjarvis.dev>
This commit is contained in:
Fabian Henneke 2020-04-14 21:27:51 +02:00 committed by GitHub
parent ec8bcae8fa
commit 75a70543b3
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 94 additions and 26 deletions

View file

@ -124,6 +124,13 @@ class PasswordFragment : Fragment() {
// and not on folder navigations since the latter leads to too many removal animations.
(recyclerView.itemAnimator as OnOffItemAnimator).isEnabled = result.isFiltered
recyclerAdapter.submitList(result.passwordItems) {
if (result.isFiltered) {
// When the result is filtered, we always scroll to the top since that is where
// the best fuzzy match appears.
recyclerView.scrollToPosition(0)
} else {
// When the result is not filtered and there is a saved scroll position for it,
// we try to restore it.
recyclerViewStateToRestore?.let {
recyclerView.layoutManager!!.onRestoreInstanceState(it)
}
@ -131,6 +138,7 @@ class PasswordFragment : Fragment() {
}
}
}
}
override fun onDestroyView() {
_binding = null

View file

@ -39,6 +39,7 @@ import kotlinx.coroutines.FlowPreview
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.asFlow
import kotlinx.coroutines.flow.distinctUntilChanged
import kotlinx.coroutines.flow.drop
import kotlinx.coroutines.flow.emptyFlow
import kotlinx.coroutines.flow.filter
import kotlinx.coroutines.flow.map
@ -94,11 +95,11 @@ private fun PasswordItem.Companion.makeComparator(
PasswordRepository.PasswordSortOrder.FILE_FIRST -> compareByDescending { it.type }
}
.then(compareBy(nullsLast(CaseInsensitiveComparator)) {
directoryStructure.getIdentifierFor(
it.file
)
directoryStructure.getIdentifierFor(it.file)
})
.then(compareBy(CaseInsensitiveComparator) {
directoryStructure.getUsernameFor(it.file)
})
.then(compareBy(CaseInsensitiveComparator) { directoryStructure.getUsernameFor(it.file) })
}
val PasswordItem.stableId: String
@ -219,7 +220,8 @@ class SearchableRepositoryViewModel(application: Application) : AndroidViewModel
FilterMode.StrictDomain -> {
check(searchAction.listMode == ListMode.FilesOnly) { "Searches with StrictDomain search mode can only list files" }
prefilteredResultFlow
.filter { file ->
.filter { absoluteFile ->
val file = absoluteFile.relativeTo(root)
val toMatch =
directoryStructure.getIdentifierFor(file) ?: return@filter false
// In strict domain mode, we match
@ -268,6 +270,8 @@ class SearchableRepositoryViewModel(application: Application) : AndroidViewModel
return dir
.walkTopDown().onEnter { file -> shouldTake(file) }
.asFlow()
// Skip the root directory
.drop(1)
.map {
yield()
it

View file

@ -19,32 +19,72 @@ enum class DirectoryStructure(val value: String) {
FileBased("file"),
DirectoryBased("directory");
fun getUsernameFor(file: File) = when (this) {
/**
* Returns the username associated to [file], following the convention of the current
* [DirectoryStructure].
*
* Examples:
* - work/example.org/john@doe.org.gpg --> john@doe.org (FileBased)
* - work/example.org/john@doe.org/password.gpg --> john@doe.org (DirectoryBased)
* - Temporary PIN.gpg --> Temporary PIN (DirectoryBased, fallback)
*/
fun getUsernameFor(file: File): String = when (this) {
FileBased -> file.nameWithoutExtension
DirectoryBased -> file.parentFile?.name ?: file.nameWithoutExtension
}
fun getIdentifierFor(file: File) = when (this) {
FileBased -> file.parentFile.name
DirectoryBased -> file.parentFile.parentFile?.name
/**
* Returns the origin identifier associated to [file], following the convention of the current
* [DirectoryStructure].
*
* At least one of [DirectoryStructure.getIdentifierFor] and
* [DirectoryStructure.getAccountPartFor] will always return a non-null result.
*
* Examples:
* - work/example.org/john@doe.org.gpg --> example.org (FileBased)
* - example.org.gpg --> example.org (FileBased, fallback)
* - work/example.org/john@doe.org/password.gpg --> example.org (DirectoryBased)
* - Temporary PIN.gpg --> null (DirectoryBased)
*/
fun getIdentifierFor(file: File): String? = when (this) {
FileBased -> file.parentFile?.name ?: file.nameWithoutExtension
DirectoryBased -> file.parentFile?.parent
}
/**
* Returns the path components of [file] until right before the component that contains the
* origin identifier according to the current [DirectoryStructure].
*
* At least one of [DirectoryStructure.getIdentifierFor] and
* [DirectoryStructure.getAccountPartFor] will always return a non-null result.
*
* Examples:
* - /work/example.org/john@doe.org --> /work (FileBased)
* - /work/example.org/john@doe.org/password --> /work (DirectoryBased)
* - work/example.org/john@doe.org.gpg --> work (FileBased)
* - example.org/john@doe.org.gpg --> null (FileBased)
* - john@doe.org.gpg --> null (FileBased)
* - work/example.org/john@doe.org/password.gpg --> work (DirectoryBased)
* - example.org/john@doe.org/password.gpg --> null (DirectoryBased)
*/
fun getPathToIdentifierFor(file: File) = when (this) {
FileBased -> file.parentFile.parent
DirectoryBased -> file.parentFile.parentFile?.parent
fun getPathToIdentifierFor(file: File): String? = when (this) {
FileBased -> file.parentFile?.parent
DirectoryBased -> file.parentFile?.parentFile?.parent
}
fun getAccountPartFor(file: File) = when (this) {
FileBased -> file.nameWithoutExtension
DirectoryBased -> "${file.parentFile.name}/${file.nameWithoutExtension}"
/**
* Returns the path component of [file] following the origin identifier according to the current
* [DirectoryStructure] (without file extension).
*
* Examples:
* - work/example.org/john@doe.org.gpg --> john@doe.org (FileBased)
* - example.org.gpg --> null (FileBased, fallback)
* - work/example.org/john@doe.org/password.gpg --> john@doe.org/password (DirectoryBased)
* - Temporary PIN.gpg --> Temporary PIN (DirectoryBased, fallback)
*/
fun getAccountPartFor(file: File): String? = when (this) {
FileBased -> file.nameWithoutExtension.takeIf { file.parentFile != null }
DirectoryBased -> file.parentFile?.let { parentFile ->
"${parentFile.name}/${file.nameWithoutExtension}"
} ?: file.nameWithoutExtension
}
@RequiresApi(Build.VERSION_CODES.O)

View file

@ -11,6 +11,7 @@ import android.content.Intent
import android.content.IntentSender
import android.os.Build
import android.os.Bundle
import android.view.View
import android.view.autofill.AutofillManager
import android.widget.TextView
import androidx.activity.viewModels
@ -119,13 +120,26 @@ class AutofillFilterView : AppCompatActivity() {
::PasswordViewHolder) { item ->
val file = item.file.relativeTo(item.rootDir)
val pathToIdentifier = directoryStructure.getPathToIdentifierFor(file)
val identifier = directoryStructure.getIdentifierFor(file) ?: "INVALID"
val identifier = directoryStructure.getIdentifierFor(file)
val accountPart = directoryStructure.getAccountPartFor(file)
title.text = buildSpannedString {
pathToIdentifier?.let { append("$it/") }
check(identifier != null || accountPart != null) { "At least one of identifier and accountPart should always be non-null" }
title.text = if (identifier != null) {
buildSpannedString {
if (pathToIdentifier != null)
append("$pathToIdentifier/")
bold { underline { append(identifier) } }
}
subtitle.text = accountPart
} else {
accountPart
}
subtitle.apply {
if (identifier != null && accountPart != null) {
text = accountPart
visibility = View.VISIBLE
} else {
visibility = View.GONE
}
}
}.onItemClicked { _, item ->
decryptAndFill(item)
}
@ -154,7 +168,9 @@ class AutofillFilterView : AppCompatActivity() {
}
model.searchResult.observe(this) { result ->
val list = result.passwordItems
recyclerAdapter.submitList(list)
recyclerAdapter.submitList(list) {
binding.rvPassword.scrollToPosition(0)
}
// Switch RecyclerView out for a "no results" message if the new list is empty and
// the message is not yet shown (and vice versa).
if ((list.isEmpty() && binding.rvPasswordSwitcher.nextView.id == binding.rvPasswordEmpty.id) ||