From 75a70543b3def38d3e89fce489c2c99c1bb45d5f Mon Sep 17 00:00:00 2001 From: Fabian Henneke Date: Tue, 14 Apr 2020 21:27:51 +0200 Subject: 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 --- .../java/com/zeapo/pwdstore/PasswordFragment.kt | 14 ++++- .../pwdstore/SearchableRepositoryViewModel.kt | 14 +++-- .../pwdstore/autofill/oreo/AutofillPreferences.kt | 64 ++++++++++++++++++---- .../autofill/oreo/ui/AutofillFilterActivity.kt | 28 ++++++++-- 4 files changed, 94 insertions(+), 26 deletions(-) (limited to 'app/src/main/java') diff --git a/app/src/main/java/com/zeapo/pwdstore/PasswordFragment.kt b/app/src/main/java/com/zeapo/pwdstore/PasswordFragment.kt index f5d9b3ab..e4551f1c 100644 --- a/app/src/main/java/com/zeapo/pwdstore/PasswordFragment.kt +++ b/app/src/main/java/com/zeapo/pwdstore/PasswordFragment.kt @@ -124,10 +124,18 @@ 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) { - recyclerViewStateToRestore?.let { - recyclerView.layoutManager!!.onRestoreInstanceState(it) + 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) + } + recyclerViewStateToRestore = null } - recyclerViewStateToRestore = null } } } diff --git a/app/src/main/java/com/zeapo/pwdstore/SearchableRepositoryViewModel.kt b/app/src/main/java/com/zeapo/pwdstore/SearchableRepositoryViewModel.kt index 3e0dddd6..992a5f79 100644 --- a/app/src/main/java/com/zeapo/pwdstore/SearchableRepositoryViewModel.kt +++ b/app/src/main/java/com/zeapo/pwdstore/SearchableRepositoryViewModel.kt @@ -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 diff --git a/app/src/main/java/com/zeapo/pwdstore/autofill/oreo/AutofillPreferences.kt b/app/src/main/java/com/zeapo/pwdstore/autofill/oreo/AutofillPreferences.kt index bedb551d..39e1dbb5 100644 --- a/app/src/main/java/com/zeapo/pwdstore/autofill/oreo/AutofillPreferences.kt +++ b/app/src/main/java/com/zeapo/pwdstore/autofill/oreo/AutofillPreferences.kt @@ -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) diff --git a/app/src/main/java/com/zeapo/pwdstore/autofill/oreo/ui/AutofillFilterActivity.kt b/app/src/main/java/com/zeapo/pwdstore/autofill/oreo/ui/AutofillFilterActivity.kt index aee7f1a9..0901f8d9 100644 --- a/app/src/main/java/com/zeapo/pwdstore/autofill/oreo/ui/AutofillFilterActivity.kt +++ b/app/src/main/java/com/zeapo/pwdstore/autofill/oreo/ui/AutofillFilterActivity.kt @@ -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/") } - bold { underline { append(identifier) } } + 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) } } + } + } else { + accountPart + } + subtitle.apply { + if (identifier != null && accountPart != null) { + text = accountPart + visibility = View.VISIBLE + } else { + visibility = View.GONE + } } - subtitle.text = accountPart }.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) || -- cgit v1.2.3