From 4c5985426d896ca3538ddee2da529e09feaf4797 Mon Sep 17 00:00:00 2001 From: Qinmei Du Date: Thu, 8 Dec 2022 02:21:58 +0000 Subject: [PATCH] Update get flow about the icons and chip UI 1. Update the sorting of crendentialEntry to sort passkey first 2. Update the padding of the entry 3. Make the icons apply dynamatic color 4. Make the credentialEntry icons fall back to unknown sign-in icons 5. Make the icons of action chip use provider icon 6. Add the lock icon inside the authentication chip 7. Make password type display 12 dots screencast: https://drive.google.com/file/d/1hM2M0yNMGKjpipvoIZg0oeCRBDN9UQ5d/view?usp=sharing&resourcekey=0-AVdG9BISCHteQqtv6a-NYw Test: deployed locally Bug: 253157237 Change-Id: Ibe5556076cae3b64ab1cc1514cf14986e84182b0 --- .../res/drawable/ic_face.xml | 30 -------- .../res/drawable/ic_manage_accounts.xml | 30 -------- .../CredentialManagerRepo.kt | 8 +-- .../credentialmanager/DataConverter.kt | 11 +-- .../getflow/GetCredentialComponents.kt | 68 ++++++++++++------- .../getflow/GetCredentialViewModel.kt | 20 +++--- .../jetpack/provider/ActionUi.kt | 9 +-- 7 files changed, 64 insertions(+), 112 deletions(-) delete mode 100644 packages/CredentialManager/res/drawable/ic_face.xml delete mode 100644 packages/CredentialManager/res/drawable/ic_manage_accounts.xml diff --git a/packages/CredentialManager/res/drawable/ic_face.xml b/packages/CredentialManager/res/drawable/ic_face.xml deleted file mode 100644 index 16fe14495e9d1..0000000000000 --- a/packages/CredentialManager/res/drawable/ic_face.xml +++ /dev/null @@ -1,30 +0,0 @@ - - - - - - - \ No newline at end of file diff --git a/packages/CredentialManager/res/drawable/ic_manage_accounts.xml b/packages/CredentialManager/res/drawable/ic_manage_accounts.xml deleted file mode 100644 index adad2f105d55b..0000000000000 --- a/packages/CredentialManager/res/drawable/ic_manage_accounts.xml +++ /dev/null @@ -1,30 +0,0 @@ - - - - - - - \ No newline at end of file diff --git a/packages/CredentialManager/src/com/android/credentialmanager/CredentialManagerRepo.kt b/packages/CredentialManager/src/com/android/credentialmanager/CredentialManagerRepo.kt index 94aa8e81ff609..4faf00c43d34d 100644 --- a/packages/CredentialManager/src/com/android/credentialmanager/CredentialManagerRepo.kt +++ b/packages/CredentialManager/src/com/android/credentialmanager/CredentialManagerRepo.kt @@ -63,7 +63,7 @@ class CredentialManagerRepo( requestInfo = intent.extras?.getParcelable( RequestInfo.EXTRA_REQUEST_INFO, RequestInfo::class.java - ) ?: testCreatePasskeyRequestInfo() + ) ?: testGetRequestInfo() providerEnabledList = when (requestInfo.type) { RequestInfo.TYPE_CREATE -> @@ -245,12 +245,10 @@ class CredentialManagerRepo( listOf( newActionEntry( "key3", "subkey-1", TYPE_PASSWORD_CREDENTIAL, - Icon.createWithResource(context, R.drawable.ic_manage_accounts), "Open Google Password Manager", "elisa.beckett@gmail.com" ), newActionEntry( "key3", "subkey-2", TYPE_PASSWORD_CREDENTIAL, - Icon.createWithResource(context, R.drawable.ic_manage_accounts), "Open Google Password Manager", "beckett-family@gmail.com" ), ) @@ -275,7 +273,6 @@ class CredentialManagerRepo( listOf( newActionEntry( "key3", "subkey-1", TYPE_PASSWORD_CREDENTIAL, - Icon.createWithResource(context, R.drawable.ic_face), "Open Enpass" ), ) @@ -287,7 +284,6 @@ class CredentialManagerRepo( key: String, subkey: String, credentialType: String, - icon: Icon, text: String, subtext: String? = null, ): Entry { @@ -295,7 +291,7 @@ class CredentialManagerRepo( Entry.CREDENTIAL_MANAGER_ENTRY_URI, SliceSpec(credentialType, 1) ).addText( text, null, listOf(Entry.HINT_ACTION_TITLE) - ).addIcon(icon, null, listOf(Entry.HINT_ACTION_ICON)) + ) if (subtext != null) { slice.addText(subtext, null, listOf(Entry.HINT_ACTION_SUBTEXT)) } diff --git a/packages/CredentialManager/src/com/android/credentialmanager/DataConverter.kt b/packages/CredentialManager/src/com/android/credentialmanager/DataConverter.kt index f514b02f8abe2..56fbf6632dc50 100644 --- a/packages/CredentialManager/src/com/android/credentialmanager/DataConverter.kt +++ b/packages/CredentialManager/src/com/android/credentialmanager/DataConverter.kt @@ -67,7 +67,8 @@ class GetFlowUtils { .getPackageInfo(packageName!!, PackageManager.PackageInfoFlags.of(0)) val providerDisplayName = pkgInfo.applicationInfo.loadLabel(packageManager).toString() - // TODO: decide what to do when failed to load a provider icon + // TODO: get the provider icon from the service + // and decide what to do when failed to load a provider icon val providerIcon = pkgInfo.applicationInfo.loadIcon(packageManager)!! ProviderInfo( id = it.providerFlattenedComponentName, @@ -83,7 +84,7 @@ class GetFlowUtils { it.authenticationEntry), remoteEntry = getRemoteEntry(it.providerFlattenedComponentName, it.remoteEntry), actionEntryList = getActionEntryList( - it.providerFlattenedComponentName, it.actionChips, context), + it.providerFlattenedComponentName, it.actionChips, providerIcon), ) } } @@ -119,7 +120,7 @@ class GetFlowUtils { displayName = credentialEntryUi.userDisplayName?.toString(), // TODO: proper fallback icon = credentialEntryUi.entryIcon?.loadDrawable(context) - ?: context.getDrawable(R.drawable.ic_passkey)!!, + ?: context.getDrawable(R.drawable.ic_other_sign_in)!!, lastUsedTimeMillis = credentialEntryUi.lastUsedTimeMillis, ) } @@ -164,7 +165,7 @@ class GetFlowUtils { private fun getActionEntryList( providerId: String, actionEntries: List, - context: Context, + providerIcon: Drawable, ): List { return actionEntries.map { val actionEntryUi = ActionUi.fromSlice(it.slice) @@ -177,7 +178,7 @@ class GetFlowUtils { fillInIntent = it.frameworkExtrasIntent, title = actionEntryUi.text.toString(), // TODO: gracefully fail - icon = actionEntryUi.icon.loadDrawable(context)!!, + icon = providerIcon, subTitle = actionEntryUi.subtext?.toString(), ) } diff --git a/packages/CredentialManager/src/com/android/credentialmanager/getflow/GetCredentialComponents.kt b/packages/CredentialManager/src/com/android/credentialmanager/getflow/GetCredentialComponents.kt index 7a015a841fae7..619f5a37d45b8 100644 --- a/packages/CredentialManager/src/com/android/credentialmanager/getflow/GetCredentialComponents.kt +++ b/packages/CredentialManager/src/com/android/credentialmanager/getflow/GetCredentialComponents.kt @@ -16,6 +16,7 @@ package com.android.credentialmanager.getflow +import android.credentials.Credential import android.text.TextUtils import androidx.activity.compose.ManagedActivityResultLauncher import androidx.activity.result.ActivityResult @@ -35,6 +36,7 @@ import androidx.compose.foundation.lazy.items import androidx.compose.material.icons.Icons import androidx.compose.material.icons.filled.ArrowBack import androidx.compose.material.icons.filled.Close +import androidx.compose.material.icons.outlined.Lock import androidx.compose.material3.Divider import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.material3.Icon @@ -189,7 +191,7 @@ fun PrimarySelectionCard( color = Color.Transparent ) Row( - horizontalArrangement = Arrangement.Start, + horizontalArrangement = Arrangement.SpaceBetween, modifier = Modifier.fillMaxWidth().padding(horizontal = 24.dp) ) { CancelButton(stringResource(R.string.get_dialog_button_label_no_thanks), onCancel) @@ -423,11 +425,12 @@ fun CredentialEntryRow( Entry( onClick = { onEntrySelected(credentialEntryInfo) }, icon = { - Image( + Icon( modifier = Modifier.padding(start = 10.dp).size(32.dp), bitmap = credentialEntryInfo.icon.toBitmap().asImageBitmap(), // TODO: add description. - contentDescription = "" + contentDescription = "", + tint = LocalAndroidColorScheme.current.colorAccentPrimaryVariant, ) }, label = { @@ -436,19 +439,24 @@ fun CredentialEntryRow( TextOnSurfaceVariant( text = credentialEntryInfo.userName, style = MaterialTheme.typography.titleLarge, - modifier = Modifier.padding(top = 16.dp) + modifier = Modifier.padding(top = 16.dp, start = 5.dp) ) TextSecondary( - text = - if (TextUtils.isEmpty(credentialEntryInfo.displayName)) - credentialEntryInfo.credentialTypeDisplayName - else - credentialEntryInfo.credentialTypeDisplayName + - stringResource( - R.string.get_dialog_sign_in_type_username_separator) + - credentialEntryInfo.displayName, + text = if ( + credentialEntryInfo.credentialType == Credential.TYPE_PASSWORD_CREDENTIAL) { + "••••••••••••" + } else { + if (TextUtils.isEmpty(credentialEntryInfo.displayName)) + credentialEntryInfo.credentialTypeDisplayName + else + credentialEntryInfo.credentialTypeDisplayName + + stringResource( + R.string.get_dialog_sign_in_type_username_separator + ) + + credentialEntryInfo.displayName + }, style = MaterialTheme.typography.bodyMedium, - modifier = Modifier.padding(bottom = 16.dp) + modifier = Modifier.padding(bottom = 16.dp, start = 5.dp) ) } } @@ -472,17 +480,27 @@ fun AuthenticationEntryRow( ) }, label = { - Column() { - // TODO: fix the text values. - TextOnSurfaceVariant( - text = authenticationEntryInfo.title, - style = MaterialTheme.typography.titleLarge, - modifier = Modifier.padding(top = 16.dp) - ) - TextSecondary( - text = stringResource(R.string.locked_credential_entry_label_subtext), - style = MaterialTheme.typography.bodyMedium, - modifier = Modifier.padding(bottom = 16.dp) + Row( + horizontalArrangement = Arrangement.SpaceBetween, + modifier = Modifier.fillMaxWidth().padding(horizontal = 5.dp), + ) { + Column() { + // TODO: fix the text values. + TextOnSurfaceVariant( + text = authenticationEntryInfo.title, + style = MaterialTheme.typography.titleLarge, + modifier = Modifier.padding(top = 16.dp) + ) + TextSecondary( + text = stringResource(R.string.locked_credential_entry_label_subtext), + style = MaterialTheme.typography.bodyMedium, + modifier = Modifier.padding(bottom = 16.dp) + ) + } + Icon( + Icons.Outlined.Lock, + null, + Modifier.align(alignment = Alignment.CenterVertically).padding(end = 10.dp), ) } } @@ -509,11 +527,13 @@ fun ActionEntryRow( TextOnSurfaceVariant( text = actionEntryInfo.title, style = MaterialTheme.typography.titleLarge, + modifier = Modifier.padding(start = 5.dp), ) if (actionEntryInfo.subTitle != null) { TextSecondary( text = actionEntryInfo.subTitle, style = MaterialTheme.typography.bodyMedium, + modifier = Modifier.padding(start = 5.dp), ) } } diff --git a/packages/CredentialManager/src/com/android/credentialmanager/getflow/GetCredentialViewModel.kt b/packages/CredentialManager/src/com/android/credentialmanager/getflow/GetCredentialViewModel.kt index fcf49db6935ad..c182397b8aeca 100644 --- a/packages/CredentialManager/src/com/android/credentialmanager/getflow/GetCredentialViewModel.kt +++ b/packages/CredentialManager/src/com/android/credentialmanager/getflow/GetCredentialViewModel.kt @@ -221,7 +221,16 @@ private fun toGetScreenState( internal class CredentialEntryInfoComparator : Comparator { override fun compare(p0: CredentialEntryInfo, p1: CredentialEntryInfo): Int { - // First order by last used timestamp + // First prefer passkey type for its security benefits + if (p0.credentialType != p1.credentialType) { + if (PublicKeyCredential.TYPE_PUBLIC_KEY_CREDENTIAL == p0.credentialType) { + return -1 + } else if (PublicKeyCredential.TYPE_PUBLIC_KEY_CREDENTIAL == p1.credentialType) { + return 1 + } + } + + // Then order by last used timestamp if (p0.lastUsedTimeMillis != null && p1.lastUsedTimeMillis != null) { if (p0.lastUsedTimeMillis < p1.lastUsedTimeMillis) { return 1 @@ -233,15 +242,6 @@ internal class CredentialEntryInfoComparator : Comparator { } else if (p1.lastUsedTimeMillis != null && p1.lastUsedTimeMillis > 0) { return 1 } - - // Then prefer passkey type for its security benefits - if (p0.credentialType != p1.credentialType) { - if (PublicKeyCredential.TYPE_PUBLIC_KEY_CREDENTIAL == p0.credentialType) { - return -1 - } else if (PublicKeyCredential.TYPE_PUBLIC_KEY_CREDENTIAL == p1.credentialType) { - return 1 - } - } return 0 } } \ No newline at end of file diff --git a/packages/CredentialManager/src/com/android/credentialmanager/jetpack/provider/ActionUi.kt b/packages/CredentialManager/src/com/android/credentialmanager/jetpack/provider/ActionUi.kt index 1e639fe6bd552..19c5c2dfa4fee 100644 --- a/packages/CredentialManager/src/com/android/credentialmanager/jetpack/provider/ActionUi.kt +++ b/packages/CredentialManager/src/com/android/credentialmanager/jetpack/provider/ActionUi.kt @@ -18,7 +18,6 @@ package com.android.credentialmanager.jetpack.provider import android.app.slice.Slice import android.credentials.ui.Entry -import android.graphics.drawable.Icon /** * UI representation for a credential entry used during the get credential flow. @@ -26,28 +25,24 @@ import android.graphics.drawable.Icon * TODO: move to jetpack. */ class ActionUi( - val icon: Icon, val text: CharSequence, val subtext: CharSequence?, ) { companion object { fun fromSlice(slice: Slice): ActionUi { - var icon: Icon? = null var text: CharSequence? = null var subtext: CharSequence? = null val items = slice.items items.forEach { - if (it.hasHint(Entry.HINT_ACTION_ICON)) { - icon = it.icon - } else if (it.hasHint(Entry.HINT_ACTION_TITLE)) { + if (it.hasHint(Entry.HINT_ACTION_TITLE)) { text = it.text } else if (it.hasHint(Entry.HINT_ACTION_SUBTEXT)) { subtext = it.text } } // TODO: fail NPE more elegantly. - return ActionUi(icon!!, text!!, subtext) + return ActionUi(text!!, subtext) } } }