Skip to content

Commit 5c288b5

Browse files
authored
Improve credential grouping and sorting (#2117)
<!-- Note: This checklist is a reminder of our shared engineering expectations. The items in Bold are required If your PR involves UI changes: 1. Upload screenshots or screencasts that illustrate the changes before / after 2. Add them under the UI changes section (feel free to add more columns if needed) 3. Make sure these changes are tested in API 23 and API 26 If your PR does not involve UI changes, you can remove the **UI changes** section --> Task/Issue URL: https://app.asana.com/0/0/1202698381206771/f ### Description Improves the grouping and sorting of credentials in the management list view, taking into consideration accented characters, non standard-English letters, numbers etc... and applies a natural sorting to them. ### Steps to test this PR - [x] Generate half a dozen different logins on any site - [x] Edit one to have a title starting with a number; verify it gets its own numerical grouping - [x] Edit one to have a title starting with a character (e.g, `$`); verify it gets put into the `#` placeholder - [x] Edit one to have a title starting with an accented character (e.g, `ç`); verify it gets put alongside its base letter (e.g., `c`) - [x] Edit one to have a title starting with a non-english letter (e.g, `あ`); verify it gets its own grouping Repeat the above with adding the cases above into the domain instead of title
1 parent 1c0880f commit 5c288b5

File tree

12 files changed

+380
-290
lines changed

12 files changed

+380
-290
lines changed

autofill/autofill-impl/src/main/java/com/duckduckgo/autofill/ui/credential/management/AutofillCharacterValidator.kt

Lines changed: 0 additions & 34 deletions
This file was deleted.

autofill/autofill-impl/src/main/java/com/duckduckgo/autofill/ui/credential/management/AutofillManagementRecyclerAdapter.kt

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import com.duckduckgo.app.browser.favicon.FaviconManager
2626
import com.duckduckgo.autofill.domain.app.LoginCredentials
2727
import com.duckduckgo.autofill.impl.databinding.ItemRowAutofillCredentialsManagementScreenBinding
2828
import com.duckduckgo.autofill.impl.databinding.ItemRowAutofillCredentialsManagementScreenHeaderBinding
29+
import com.duckduckgo.autofill.ui.credential.management.sorting.CredentialGrouper
2930
import kotlinx.coroutines.launch
3031

3132
class AutofillManagementRecyclerAdapter(
@@ -109,7 +110,7 @@ class AutofillManagementRecyclerAdapter(
109110

110111
sealed class ListItem {
111112
data class Credential(val credentials: LoginCredentials) : ListItem()
112-
data class GroupHeading(val initial: Char) : ListItem()
113+
data class GroupHeading(val initial: String) : ListItem()
113114
}
114115

115116
companion object {

autofill/autofill-impl/src/main/java/com/duckduckgo/autofill/ui/credential/management/CredentialGrouper.kt

Lines changed: 0 additions & 49 deletions
This file was deleted.

autofill/autofill-impl/src/main/java/com/duckduckgo/autofill/ui/credential/management/CredentialListSorter.kt

Lines changed: 0 additions & 75 deletions
This file was deleted.
Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
/*
2+
* Copyright (c) 2022 DuckDuckGo
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package com.duckduckgo.autofill.ui.credential.management.sorting
18+
19+
import com.duckduckgo.autofill.domain.app.LoginCredentials
20+
import com.duckduckgo.autofill.ui.credential.management.AutofillManagementRecyclerAdapter.ListItem
21+
import java.util.*
22+
import javax.inject.Inject
23+
24+
class CredentialGrouper @Inject constructor(
25+
private val initialExtractor: InitialExtractor,
26+
private val sorter: CredentialListSorter
27+
) {
28+
29+
fun group(unsortedCredentials: List<LoginCredentials>): List<ListItem> {
30+
val unsortedGroups = buildGroups(unsortedCredentials)
31+
val sortedGroups = sortGroups(unsortedGroups)
32+
return buildFlatList(sortedGroups)
33+
}
34+
35+
private fun buildFlatList(sortedGroups: SortedMap<String, MutableList<LoginCredentials>>): MutableList<ListItem> {
36+
val flattenedList = mutableListOf<ListItem>()
37+
38+
sortedGroups.forEach { (key, value) ->
39+
flattenedList.add(ListItem.GroupHeading(key))
40+
41+
sorter.sort(value).forEach {
42+
flattenedList.add(ListItem.Credential(it))
43+
}
44+
}
45+
46+
return flattenedList
47+
}
48+
49+
private fun sortGroups(groups: MutableMap<String, MutableList<LoginCredentials>>): SortedMap<String, MutableList<LoginCredentials>> {
50+
return groups.toSortedMap(sorter.comparator())
51+
}
52+
53+
private fun buildGroups(unsortedCredentials: List<LoginCredentials>): MutableMap<String, MutableList<LoginCredentials>> {
54+
val groups = mutableMapOf<String, MutableList<LoginCredentials>>()
55+
unsortedCredentials.forEach { credential ->
56+
val initial = initialExtractor.extractInitial(credential)
57+
val list = groups.getOrPut(initial) { mutableListOf() }
58+
list.add(credential)
59+
}
60+
return groups
61+
}
62+
63+
}
Lines changed: 32 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -14,45 +14,61 @@
1414
* limitations under the License.
1515
*/
1616

17-
package com.duckduckgo.autofill.ui.credential.management
17+
package com.duckduckgo.autofill.ui.credential.management.sorting
1818

1919
import com.duckduckgo.autofill.AutofillDomainFormatter
2020
import com.duckduckgo.autofill.domain.app.LoginCredentials
2121
import com.duckduckgo.di.scopes.FragmentScope
2222
import com.squareup.anvil.annotations.ContributesBinding
23+
import timber.log.Timber
24+
import java.lang.Character.*
25+
import java.text.Normalizer
2326
import javax.inject.Inject
2427

2528
interface InitialExtractor {
26-
fun extractInitial(loginCredentials: LoginCredentials): Char
27-
fun extractInitialFromTitle(loginCredentials: LoginCredentials): Char?
28-
fun extractInitialFromDomain(loginCredentials: LoginCredentials): Char?
29+
fun extractInitial(loginCredentials: LoginCredentials): String
30+
fun extractInitialFromTitle(loginCredentials: LoginCredentials): String?
31+
fun extractInitialFromDomain(loginCredentials: LoginCredentials): String?
2932
}
3033

3134
@ContributesBinding(FragmentScope::class)
3235
class CredentialInitialExtractor @Inject constructor(
33-
private val domainFormatter: AutofillDomainFormatter,
34-
private val characterValidator: AutofillCharacterValidator
36+
private val domainFormatter: AutofillDomainFormatter
3537
) : InitialExtractor {
3638

37-
override fun extractInitial(loginCredentials: LoginCredentials): Char {
38-
val initial = extractInitialFromTitle(loginCredentials) ?: extractInitialFromDomain(loginCredentials)
39+
override fun extractInitial(loginCredentials: LoginCredentials): String {
40+
val rawInitial =
41+
extractInitialFromTitle(loginCredentials) ?: extractInitialFromDomain(loginCredentials) ?: return INITIAL_CHAR_FOR_NON_LETTERS
3942

40-
if (initial == null || !characterValidator.isLetter(initial)) {
41-
return INITIAL_CHAR_FOR_NON_LETTERS
43+
return when (val type = getCharacterType(rawInitial)) {
44+
LOWERCASE_LETTER,
45+
UPPERCASE_LETTER,
46+
OTHER_LETTER,
47+
DECIMAL_DIGIT_NUMBER
48+
-> {
49+
Normalizer.normalize(rawInitial, Normalizer.Form.NFKD).firstOrNull()?.toString() ?: INITIAL_CHAR_FOR_NON_LETTERS
50+
}
51+
else -> {
52+
Timber.v("Rejecting type %d for %s", type, rawInitial)
53+
INITIAL_CHAR_FOR_NON_LETTERS
54+
}
4255
}
56+
}
4357

44-
return initial.uppercaseChar()
58+
private fun getCharacterType(rawInitial: String): Byte {
59+
val initial = rawInitial.firstOrNull() ?: return UNASSIGNED
60+
return getType(initial).toByte()
4561
}
4662

47-
override fun extractInitialFromTitle(loginCredentials: LoginCredentials): Char? {
48-
return loginCredentials.domainTitle?.firstOrNull()?.uppercaseChar()
63+
override fun extractInitialFromTitle(loginCredentials: LoginCredentials): String? {
64+
return loginCredentials.domainTitle?.firstOrNull()?.uppercaseChar()?.toString()
4965
}
5066

51-
override fun extractInitialFromDomain(loginCredentials: LoginCredentials): Char? {
52-
return domainFormatter.extractDomain(loginCredentials.domain)?.firstOrNull()?.uppercaseChar()
67+
override fun extractInitialFromDomain(loginCredentials: LoginCredentials): String? {
68+
return domainFormatter.extractDomain(loginCredentials.domain)?.firstOrNull()?.uppercaseChar()?.toString()
5369
}
5470

5571
companion object {
56-
const val INITIAL_CHAR_FOR_NON_LETTERS = '#'
72+
const val INITIAL_CHAR_FOR_NON_LETTERS = "#"
5773
}
5874
}

0 commit comments

Comments
 (0)