From c4ee0b415c8bea1e61e4e1e3be5fb65464cfeb65 Mon Sep 17 00:00:00 2001 From: Qinmei Du Date: Thu, 22 Dec 2022 03:12:01 +0000 Subject: [PATCH] Fix the sorting for more options in create flow before: Provider1[sorted entries inside this provider], Provider2[sorted entries inside this provider]... but no sorting within those providers, so the result is entries inside one provider is always group together and the order of the providers are just defined by the order of the test data providers list after: sorted entries by lastUsed from all providers screencast: https://drive.google.com/file/d/18fxaGGOfKYDzvShq8bBEdAFGhTRdjS1C/view?usp=sharing&resourcekey=0-JbBhLEEkFhoZaniIay-Y_w Test: deployed locally Bug: 261060321 Change-Id: I936f5d9e8bc2f28ac0b97d36fa21e6567ce7b301 --- .../CredentialManagerRepo.kt | 11 ++-- .../credentialmanager/DataConverter.kt | 14 +++-- .../createflow/CreateCredentialComponents.kt | 63 ++++++++++--------- .../createflow/CreateCredentialViewModel.kt | 9 +-- .../createflow/CreateModel.kt | 2 +- 5 files changed, 52 insertions(+), 47 deletions(-) diff --git a/packages/CredentialManager/src/com/android/credentialmanager/CredentialManagerRepo.kt b/packages/CredentialManager/src/com/android/credentialmanager/CredentialManagerRepo.kt index 9b8443dd7c5bf..d516d210b3c05 100644 --- a/packages/CredentialManager/src/com/android/credentialmanager/CredentialManagerRepo.kt +++ b/packages/CredentialManager/src/com/android/credentialmanager/CredentialManagerRepo.kt @@ -140,9 +140,6 @@ class CredentialManagerRepo( val providerEnabledList = CreateFlowUtils.toEnabledProviderList( // Handle runtime cast error providerEnabledList as List, context) - providerEnabledList.forEach{providerInfo -> providerInfo.createOptions = - providerInfo.createOptions.sortedWith(compareBy { it.lastUsedTimeMillis }).reversed() - } return providerEnabledList } @@ -180,9 +177,9 @@ class CredentialManagerRepo( .setSaveEntries( listOf( newCreateEntry("key1", "subkey-1", "elisa.beckett@gmail.com", - 20, 7, 27, 10000), + 20, 7, 27, 10L), newCreateEntry("key1", "subkey-2", "elisa.work@google.com", - 20, 7, 27, 11000), + 20, 7, 27, 12L), ) ) .setRemoteEntry( @@ -194,9 +191,9 @@ class CredentialManagerRepo( .setSaveEntries( listOf( newCreateEntry("key1", "subkey-3", "elisa.beckett@dashlane.com", - 20, 7, 27, 30000), + 20, 7, 27, 11L), newCreateEntry("key1", "subkey-4", "elisa.work@dashlane.com", - 20, 7, 27, 31000), + 20, 7, 27, 14L), ) ) .build(), diff --git a/packages/CredentialManager/src/com/android/credentialmanager/DataConverter.kt b/packages/CredentialManager/src/com/android/credentialmanager/DataConverter.kt index 93f566ce783b3..e667ebd283ea1 100644 --- a/packages/CredentialManager/src/com/android/credentialmanager/DataConverter.kt +++ b/packages/CredentialManager/src/com/android/credentialmanager/DataConverter.kt @@ -304,20 +304,23 @@ class CreateFlowUtils { isOnPasskeyIntroStateAlready: Boolean, isPasskeyFirstUse: Boolean, ): CreateCredentialUiState { - var createOptionSize = 0 var lastSeenProviderWithNonEmptyCreateOptions: EnabledProviderInfo? = null var remoteEntry: RemoteInfo? = null var defaultProvider: EnabledProviderInfo? = null + var createOptionsPairs: + MutableList> = mutableListOf() enabledProviders.forEach { enabledProvider -> if (defaultProviderId != null) { - if (enabledProvider.name == defaultProviderId) { + if (enabledProvider.id == defaultProviderId) { defaultProvider = enabledProvider } } if (enabledProvider.createOptions.isNotEmpty()) { - createOptionSize += enabledProvider.createOptions.size lastSeenProviderWithNonEmptyCreateOptions = enabledProvider + enabledProvider.createOptions.forEach { + createOptionsPairs.add(Pair(it, enabledProvider)) + } } if (enabledProvider.remoteEntry != null) { remoteEntry = enabledProvider.remoteEntry!! @@ -327,16 +330,17 @@ class CreateFlowUtils { enabledProviders = enabledProviders, disabledProviders = disabledProviders, toCreateScreenState( - /*createOptionSize=*/createOptionSize, + /*createOptionSize=*/createOptionsPairs.size, /*isOnPasskeyIntroStateAlready=*/isOnPasskeyIntroStateAlready, /*requestDisplayInfo=*/requestDisplayInfo, /*defaultProvider=*/defaultProvider, /*remoteEntry=*/remoteEntry, /*isPasskeyFirstUse=*/isPasskeyFirstUse), requestDisplayInfo, + createOptionsPairs.sortedWith(compareByDescending{ it.first.lastUsedTimeMillis }), defaultProvider != null, toActiveEntry( /*defaultProvider=*/defaultProvider, - /*createOptionSize=*/createOptionSize, + /*createOptionSize=*/createOptionsPairs.size, /*lastSeenProviderWithNonEmptyCreateOptions=*/lastSeenProviderWithNonEmptyCreateOptions, /*remoteEntry=*/remoteEntry), ) diff --git a/packages/CredentialManager/src/com/android/credentialmanager/createflow/CreateCredentialComponents.kt b/packages/CredentialManager/src/com/android/credentialmanager/createflow/CreateCredentialComponents.kt index 3d2361332b0a6..46e1b60ae856a 100644 --- a/packages/CredentialManager/src/com/android/credentialmanager/createflow/CreateCredentialComponents.kt +++ b/packages/CredentialManager/src/com/android/credentialmanager/createflow/CreateCredentialComponents.kt @@ -76,6 +76,7 @@ fun CreateCredentialScreen( requestDisplayInfo = uiState.requestDisplayInfo, enabledProviderList = uiState.enabledProviders, disabledProviderList = uiState.disabledProviders, + sortedCreateOptionsPairs = uiState.sortedCreateOptionsPairs, onOptionSelected = viewModel::onEntrySelectedFromFirstUseScreen, onDisabledPasswordManagerSelected = viewModel::onDisabledPasswordManagerSelected, @@ -94,6 +95,7 @@ fun CreateCredentialScreen( requestDisplayInfo = uiState.requestDisplayInfo, enabledProviderList = uiState.enabledProviders, disabledProviderList = uiState.disabledProviders, + sortedCreateOptionsPairs = uiState.sortedCreateOptionsPairs, hasDefaultProvider = uiState.hasDefaultProvider, isFromProviderSelection = uiState.isFromProviderSelection!!, onBackProviderSelectionButtonSelected = @@ -246,6 +248,7 @@ fun ProviderSelectionCard( requestDisplayInfo: RequestDisplayInfo, enabledProviderList: List, disabledProviderList: List?, + sortedCreateOptionsPairs: List>, onOptionSelected: (ActiveEntry) -> Unit, onDisabledPasswordManagerSelected: () -> Unit, onMoreOptionsSelected: () -> Unit, @@ -295,22 +298,21 @@ fun ProviderSelectionCard( LazyColumn( verticalArrangement = Arrangement.spacedBy(2.dp) ) { - enabledProviderList.forEach { enabledProviderInfo -> - enabledProviderInfo.createOptions.forEach { createOptionInfo -> - item { - MoreOptionsInfoRow( - requestDisplayInfo = requestDisplayInfo, - providerInfo = enabledProviderInfo, - createOptionInfo = createOptionInfo, - onOptionSelected = { - onOptionSelected( - ActiveEntry( - enabledProviderInfo, - createOptionInfo - ) + sortedCreateOptionsPairs.forEach { entry -> + item { + MoreOptionsInfoRow( + requestDisplayInfo = requestDisplayInfo, + providerInfo = entry.second, + createOptionInfo = entry.first, + onOptionSelected = { + onOptionSelected( + ActiveEntry( + entry.second, + entry.first ) - }) - } + ) + } + ) } } item { @@ -355,6 +357,7 @@ fun MoreOptionsSelectionCard( requestDisplayInfo: RequestDisplayInfo, enabledProviderList: List, disabledProviderList: List?, + sortedCreateOptionsPairs: List>, hasDefaultProvider: Boolean, isFromProviderSelection: Boolean, onBackProviderSelectionButtonSelected: () -> Unit, @@ -411,23 +414,23 @@ fun MoreOptionsSelectionCard( LazyColumn( verticalArrangement = Arrangement.spacedBy(2.dp) ) { + // Only in the flows with default provider(not first time use) we can show the + // createOptions here, or they will be shown on ProviderSelectionCard if (hasDefaultProvider) { - enabledProviderList.forEach { enabledProviderInfo -> - enabledProviderInfo.createOptions.forEach { createOptionInfo -> - item { - MoreOptionsInfoRow( - requestDisplayInfo = requestDisplayInfo, - providerInfo = enabledProviderInfo, - createOptionInfo = createOptionInfo, - onOptionSelected = { - onOptionSelected( - ActiveEntry( - enabledProviderInfo, - createOptionInfo - ) + sortedCreateOptionsPairs.forEach { entry -> + item { + MoreOptionsInfoRow( + requestDisplayInfo = requestDisplayInfo, + providerInfo = entry.second, + createOptionInfo = entry.first, + onOptionSelected = { + onOptionSelected( + ActiveEntry( + entry.second, + entry.first ) - }) - } + ) + }) } } item { diff --git a/packages/CredentialManager/src/com/android/credentialmanager/createflow/CreateCredentialViewModel.kt b/packages/CredentialManager/src/com/android/credentialmanager/createflow/CreateCredentialViewModel.kt index 7b9e113e37d8a..55e14a9ccb6ea 100644 --- a/packages/CredentialManager/src/com/android/credentialmanager/createflow/CreateCredentialViewModel.kt +++ b/packages/CredentialManager/src/com/android/credentialmanager/createflow/CreateCredentialViewModel.kt @@ -39,6 +39,7 @@ data class CreateCredentialUiState( val disabledProviders: List? = null, val currentScreenState: CreateScreenState, val requestDisplayInfo: RequestDisplayInfo, + val sortedCreateOptionsPairs: List>, // Should not change with the real time update of default provider, only determine whether we're // showing provider selection page at the beginning val hasDefaultProvider: Boolean, @@ -89,9 +90,9 @@ class CreateCredentialViewModel( UserConfigRepo.getInstance().setIsPasskeyFirstUse(false) } - fun getProviderInfoByName(providerName: String): EnabledProviderInfo { + fun getProviderInfoByName(providerId: String): EnabledProviderInfo { return uiState.enabledProviders.single { - it.name == providerName + it.id == providerId } } @@ -133,7 +134,7 @@ class CreateCredentialViewModel( currentScreenState = CreateScreenState.CREATION_OPTION_SELECTION, activeEntry = activeEntry ) - val providerId = uiState.activeEntry?.activeProvider?.name + val providerId = uiState.activeEntry?.activeProvider?.id onDefaultChanged(providerId) } @@ -150,7 +151,7 @@ class CreateCredentialViewModel( uiState = uiState.copy( currentScreenState = CreateScreenState.CREATION_OPTION_SELECTION, ) - val providerId = uiState.activeEntry?.activeProvider?.name + val providerId = uiState.activeEntry?.activeProvider?.id onDefaultChanged(providerId) } diff --git a/packages/CredentialManager/src/com/android/credentialmanager/createflow/CreateModel.kt b/packages/CredentialManager/src/com/android/credentialmanager/createflow/CreateModel.kt index 58db36c777936..1035c46969273 100644 --- a/packages/CredentialManager/src/com/android/credentialmanager/createflow/CreateModel.kt +++ b/packages/CredentialManager/src/com/android/credentialmanager/createflow/CreateModel.kt @@ -22,7 +22,7 @@ import android.graphics.drawable.Drawable open class ProviderInfo( val icon: Drawable, - val name: String, + val id: String, val displayName: String, )