Add new 'wallet' module. #448

davidz25 · 2024-01-17T20:49:28Z

No description provided.

wallet/src/main/java/com/android/identity_credential/wallet/MainActivity.kt

dritan-x · 2024-01-18T17:22:36Z

wallet/src/main/java/com/android/identity_credential/wallet/MainActivity.kt

+                        if (application.addSelfsignedMdl()) {
+                            navigation.popBackStack()
+                        } else {
+                            Toast.makeText(applicationContext,


alternatives to add in next iteration:

snackbar that dismisses itself after 5? or 10? seconds to user has a chance to see this message

popup modal/dialog with acknowledgement from user in the event they don't see the toast or snackbar on the bottom, tapping on "OK" dismisses it

I wouldn't worry about this. This code here is very temporary and will go away once we stand up a proper IssuingAuthority interface with a dedicated IssuingAuthoritySelfSignedMdl implementation (as well as other implementations).

dritan-x · 2024-01-18T17:25:12Z

wallet/src/main/java/com/android/identity_credential/wallet/MainActivity.kt

+            drawerState = drawerState,
+            drawerContent = {
+                ModalDrawerSheet {
+                    Text("Wallet", modifier = Modifier.padding(16.dp))


in the next iteration, we should probably put all the text strings in xml for translation/localization so this POC can be shown in various countries

Indeed. We also need to figure out how much of this is dynamic configuration downloaded from a "Wallet Server" and how much is using the usual i18n/l10n infrastructure. Cc @GarethCOliver

dritan-x · 2024-01-18T17:28:33Z

wallet/src/main/java/com/android/identity_credential/wallet/WalletApplication.kt

+import kotlin.math.ceil
+
+class WalletApplication : Application() {
+    companion object {


in the next iteration, we can define an object WalletConsts (or some other meaningful name) in its own file that is accessible globally rather than adding a companion object to the Application class (which is reserved for application-specific logic - such as instantiation of singletons, context propagation, etc..)

Yep, this is something we need to design properly, the ddoc already has a section for it!

dritan-x · 2024-01-18T17:31:31Z

wallet/src/main/java/com/android/identity_credential/wallet/WalletApplication.kt

+        credentialStore = CredentialStore(storageEngine, secureAreaRepository)
+    }
+
+    private fun createArtwork(color1: Int,


on the next iteration, we can decouple these and all provision-related code into a provisioning-specific file/class for better modularity and encapsulation of related business logic

Yep, exactly!

dritan-x

first version base foundation LGTM! I'm happy to make all proposed changes in the next iteration

suzannajiwani

other than the copyrights, just a few nits - otherwise, I'm a big fan of the overall look + flow

suzannajiwani · 2024-01-18T19:20:47Z

wallet/src/main/java/com/android/identity_credential/wallet/MainActivity.kt

@@ -0,0 +1,477 @@
+@file:OptIn(ExperimentalMaterial3Api::class, ExperimentalFoundationApi::class)


add the copyright here

I'm actually noticing now that the appholder/appverifier MainActivity classes also don't have the copyright - should we add to these types of files or leave off?

I'm actually not sure what to put in the copyright header (or if it's even advisable to have one) so I suggest we leave that out for now until I consult with OWF.

suzannajiwani · 2024-01-18T19:21:47Z

wallet/src/main/java/com/android/identity_credential/wallet/MainActivity.kt

+
+        setContent {
+            IdentityCredentialTheme {
+                // A surface container using the 'background' color from the theme


seems like an extraneous comment?

This was autogenerated!

suzannajiwani · 2024-01-18T19:31:41Z

wallet/src/main/java/com/android/identity_credential/wallet/MainActivity.kt

+                    Divider()
+                    NavigationDrawerItem(
+                        icon = { Icon(imageVector = Icons.Filled.Add, contentDescription = null) },
+                        label = { Text(text = "Add to Wallet") },


imo would be more clear if it said something along the lines of "Add new Credential to Wallet"

This is displayed in the UI in a pretty narrow drawer so I'd like to keep it short.

suzannajiwani · 2024-01-18T19:34:24Z

wallet/src/main/java/com/android/identity_credential/wallet/MainActivity.kt

+                    Image(
+                        bitmap = credentialBitmap.asImageBitmap(),
+                        contentDescription = "Artwork for $credentialName",
+                        modifier = Modifier.clickable(onClick = {
+                            navigation.navigate("CredentialInfo/$credentialId")
+                        })


the card image looks really slick on-screen! I also like the HorizontalPager

Cool, yea, I think we actually want to spend some time on getting some nice generic artwork and writing code to overlay parts of the holder's name.

suzannajiwani · 2024-01-18T19:43:12Z

wallet/src/main/java/com/android/identity_credential/wallet/WalletApplication.kt

@@ -0,0 +1,284 @@
+package com.android.identity_credential.wallet


Test: All unit tests pass Test: Manually tested all apps Signed-off-by: David Zeuthen <[email protected]>

davidz25 assigned suzannajiwani Jan 17, 2024

davidz25 force-pushed the new-wallet-app branch from eff4c02 to f9de9c7 Compare January 18, 2024 17:03

davidz25 requested a review from suzannajiwani January 18, 2024 17:04

davidz25 unassigned suzannajiwani Jan 18, 2024

dritan-x reviewed Jan 18, 2024

View reviewed changes

wallet/src/main/java/com/android/identity_credential/wallet/MainActivity.kt Show resolved Hide resolved

dritan-x reviewed Jan 18, 2024

View reviewed changes

dritan-x approved these changes Jan 18, 2024

View reviewed changes

davidz25 force-pushed the new-wallet-app branch 2 times, most recently from 2d4a4af to 4545525 Compare January 18, 2024 19:16

suzannajiwani approved these changes Jan 18, 2024

View reviewed changes

davidz25 force-pushed the new-wallet-app branch 3 times, most recently from 924c9ed to 3919847 Compare January 19, 2024 00:05

Add new 'wallet' module.

efa73cc

Test: All unit tests pass Test: Manually tested all apps Signed-off-by: David Zeuthen <[email protected]>

davidz25 force-pushed the new-wallet-app branch from 3919847 to efa73cc Compare January 19, 2024 00:12

davidz25 merged commit 33408e2 into main Jan 19, 2024
5 checks passed

davidz25 deleted the new-wallet-app branch January 19, 2024 14:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add new 'wallet' module. #448

Add new 'wallet' module. #448

davidz25 commented Jan 17, 2024

dritan-x Jan 18, 2024 •

edited

Loading

davidz25 Jan 18, 2024

dritan-x Jan 18, 2024

davidz25 Jan 18, 2024

dritan-x Jan 18, 2024 •

edited

Loading

davidz25 Jan 18, 2024

dritan-x Jan 18, 2024 •

edited

Loading

davidz25 Jan 18, 2024

dritan-x left a comment

suzannajiwani left a comment

suzannajiwani Jan 18, 2024

suzannajiwani Jan 18, 2024

davidz25 Jan 18, 2024

suzannajiwani Jan 18, 2024

davidz25 Jan 18, 2024

suzannajiwani Jan 18, 2024

davidz25 Jan 18, 2024

suzannajiwani Jan 18, 2024

davidz25 Jan 18, 2024

suzannajiwani Jan 18, 2024

		@@ -0,0 +1,477 @@
		@file:OptIn(ExperimentalMaterial3Api::class, ExperimentalFoundationApi::class)

		@@ -0,0 +1,284 @@
		package com.android.identity_credential.wallet

Add new 'wallet' module. #448

Add new 'wallet' module. #448

Conversation

davidz25 commented Jan 17, 2024

dritan-x Jan 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dritan-x Jan 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dritan-x Jan 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dritan-x left a comment

Choose a reason for hiding this comment

suzannajiwani left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dritan-x Jan 18, 2024 •

edited

Loading

dritan-x Jan 18, 2024 •

edited

Loading

dritan-x Jan 18, 2024 •

edited

Loading