Generate QR code with vCard data by hamorillo · Pull Request #76 · Automattic/Gravatar-Android

hamorillo · 2025-07-23T15:36:42Z

Description

This PR implements the QR code generation using qrose.

Before Public Fields Section

Kapture.2025-07-23.at.17.34.37.mp4

Kapture.2025-07-29.at.14.52.52.mp4

What's included?

This PR generates a QR code containing the vCard information using the Profile data, plus the private contact information introduced by the user. It also takes into account the switches in the Private Contact Section, so if the user decides not to share their email and/or phone, those won't be included.

What's not included?

~~The public fields section is not included, but the data is being included in the QR. We need to implement the UI to show that section and take into account the switches while generating the QE~~

Edited: The public fields are now included in this PR, as they were already added. You can include those fields in the vCard by playing with the switches.

Why I've chosen qrose?

It's just kotlin and compose.ui, it doesn't have more dependencies.
It's easy to customize, for example:

Testing Steps

Launch the app and navigate to the Share section
Verify the QR code is, in fact, a vCard with the profile information
Play with the phone and email fields/switches, verifying the QR code generation is correct
Play with the public fields/switches, verifying the QR code generation is correct

Copilot

Pull Request Overview

This PR implements QR code generation with vCard data for user profiles using the qrose library. The implementation creates QR codes containing contact information that users can share, with configurable privacy settings for email and phone sharing.

Key changes:

Added QR code generation using the qrose library with vCard format
Implemented privacy controls for email and phone sharing in QR codes
Updated UI components to display generated QR codes in the share section

Reviewed Changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
`gradle/libs.versions.toml`	Added qrose library dependency for QR code generation
`homeUi/build.gradle.kts`	Integrated qrose library into the homeUi module
`ShareUiState.kt`	Added vCard data generation logic and updated privacy defaults
`ShareHeader.kt`	Integrated QR code display using qrose painter
`ShareScreen.kt`	Connected QR code data to the header component
`ShareViewModel.kt`	Added avatar URL dependency for complete profile data
`ShareUiStateTest.kt`	Added comprehensive test coverage for vCard generation logic
`ShareViewModelTest.kt`	Updated test to accommodate new avatar URL dependency
`ShareHeaderTest.kt`	Updated test with required vCard data parameter

Copilot · 2025-07-23T15:37:54Z

homeUi/src/main/kotlin/com/gravatar/app/homeUi/presentation/home/share/ShareUiState.kt

 internal data class PrivateContactInfo(
    val emailValue: String = "",
-    val isEmailShared: Boolean = false,
+    val isEmailShared: Boolean = true,


Changing the default value from false to true for isEmailShared is a breaking change that could unexpectedly expose user email addresses. This should default to false for privacy protection.

Suggested change

val isEmailShared: Boolean = true,

val isEmailShared: Boolean = false,

Is that on purpose? It's ON even with the field empty 🤔

Yes, I think that's what we agreed in the last call, and it's how the iOS app is working at the moment. However, we can improve this behaviour. We can discuss this with iOS folks.

Copilot · 2025-07-23T15:37:55Z

homeUi/src/main/kotlin/com/gravatar/app/homeUi/presentation/home/share/ShareUiState.kt

+    val isEmailShared: Boolean = true,
    val phoneValue: String = "",
-    val isPhoneShared: Boolean = false
+    val isPhoneShared: Boolean = true,


Changing the default value from false to true for isPhoneShared is a breaking change that could unexpectedly expose user phone numbers. This should default to false for privacy protection.

Suggested change

val isPhoneShared: Boolean = true,

val isPhoneShared: Boolean = false,

Copilot · 2025-07-23T15:37:55Z

homeUi/src/main/kotlin/com/gravatar/app/homeUi/presentation/home/share/ShareUiState.kt

+            vCardBuilder.append("FN:${("$firstName $lastName".trim()).ifEmpty { profile.displayName }}\n")
+            vCardBuilder.append("NICKNAME:${profile.displayName.ifEmpty { "$firstName $lastName".trim() }}\n")


[nitpick] The nested string interpolation and conditional logic is complex and hard to read. Consider extracting this to a separate variable: val fullName = "$firstName $lastName".trim().ifEmpty { profile.displayName }

Suggested change

vCardBuilder.append("FN:${("$firstName $lastName".trim()).ifEmpty { profile.displayName }}\n")

vCardBuilder.append("NICKNAME:${profile.displayName.ifEmpty { "$firstName $lastName".trim() }}\n")

val fullName = "$firstName $lastName".trim().ifEmpty { profile.displayName }

vCardBuilder.append("FN:$fullName\n")

vCardBuilder.append("NICKNAME:${profile.displayName.ifEmpty { fullName }}\n")

Copilot · 2025-07-23T15:37:56Z

homeUi/src/main/kotlin/com/gravatar/app/homeUi/presentation/home/share/ShareUiState.kt

+            vCardBuilder.append("N:$lastName;$firstName;;;\n")
+            vCardBuilder.append("FN:${("$firstName $lastName".trim()).ifEmpty { profile.displayName }}\n")
+            vCardBuilder.append("NICKNAME:${profile.displayName.ifEmpty { "$firstName $lastName".trim() }}\n")
+        }


[nitpick] Similar to the FN field, this logic is duplicated and complex. Consider extracting the name concatenation logic to a helper function to avoid duplication.

Suggested change

vCardBuilder.append("N:$lastName;$firstName;;;\n")

vCardBuilder.append("FN:${("$firstName $lastName".trim()).ifEmpty { profile.displayName }}\n")

vCardBuilder.append("NICKNAME:${profile.displayName.ifEmpty { "$firstName $lastName".trim() }}\n")

}

val (nField, fnField, nicknameField) = formatName(firstName, lastName, profile.displayName)

vCardBuilder.append(nField)

vCardBuilder.append(fnField)

vCardBuilder.append(nicknameField)

Copilot · 2025-07-23T15:37:56Z

homeUi/src/main/kotlin/com/gravatar/app/homeUi/presentation/home/share/ShareUiState.kt

-    val isPhoneShared: Boolean = false
+    val isPhoneShared: Boolean = true,
 )
+


The vCard generation does not escape special characters in user input. Fields like description, company, and job title should be escaped to prevent vCard format corruption if they contain newlines, semicolons, or other special characters.

Suggested change

private fun escapeVCardValue(value: String): String {

return value.replace("\n", "\\n").replace(";", "\\;").replace(",", "\\,")

}

wpmobilebot · 2025-07-23T16:09:44Z

📲 You can test the changes from this Pull Request in Gravatar Android by scanning the QR code below to install the corresponding build.

	App Name	Gravatar Android
	Build Type	release
	Commit	`fda8deb`
	Direct Download	`gravatar-app-prototype-build-pr76-fda8deb.apk`

AdamGrzybkowski

Works great! I would prefer to have the VCard generation extracted so that it can be tested in isolation, not as part of the uiState. WDYT?

homeUi/src/main/kotlin/com/gravatar/app/homeUi/presentation/home/share/ShareUiState.kt

...Ui/src/main/kotlin/com/gravatar/app/homeUi/presentation/home/share/components/ShareHeader.kt

AdamGrzybkowski · 2025-07-24T08:47:21Z

homeUi/src/main/kotlin/com/gravatar/app/homeUi/presentation/home/share/model/VCard.kt

+        if (firstName.isNotEmpty() || lastName.isNotEmpty()) {
+            contentBuilder.append("N:$lastName;$firstName;;;\n")
+                .append("FN:${("$firstName $lastName".trim()).ifEmpty { nickname }}\n")
+        }
+        val calculatedNickname = nickname.ifEmpty { "$firstName $lastName".trim() }
+
+        calculatedNickname.takeIf { it.isNotEmpty() }?.let { contentBuilder.append("NICKNAME:$it\n") }


This doesn't match the iOS:

it should be:

N:\(model.lastName);\(model.firstName); FN:\(model.displayName) NICKNAME:\(model.displayName)

🤔 I'm not sure if that is the correct approach. The specification says for the FN:

Purpose: To specify the formatted text corresponding to the name of the object the vCard represents.

I can update the nickname so that it does not use the first and last name when it's empty. But the FN looks better as it is.

@etoledom thoughts on this?

This is the logic we used with @pinarol for this one:

FN: Is a required field on the vcard spec, so we chose a profile property which we know will always have data to fill it with (displayName).
N: Will be filled with the first and last name if they exist.
NICKNAME: Is technically how the user wants to be called

We noticed by testing (on iOS contacts) that whenever N is present, FN gets replaced by N.
If N is empty and ORG is present, FN is replaced by ORG
If N and ORG are empty, FN becomes N (first and last name are split by spaces).

While NICKNAME is always preserved.

So, if N and ORG are empty, we will have FN = N = displayName
If N or ORG have data, FN gets replaced by either, so whatever data in FN is irrelevant.
And NICKNAME will always have the data of display name, as it is how the user wants to be recognized (or so we assume), and also, is a way to not get it replaced by N or ORG.

If FN = N (first - last name), FN becomes irrelevant in any case.

To know if this also counts for Android, we will need to know if FN is added to a separated field than N when both have different data. And a similar test when N is empty and ORG has data. Is FN replaced by ORG or is it added to a separated contact field?

Great explanation @etoledom! Appreciate it.

I've done some tests and I'm not wrong, what you've detailed also applies to Android. I observe the same behaviour so, I'll update the code to save displayName in FN.

Thanks!

I can't confirm this behavior on my device.

Now that the displayName is added as FN and NICKNAME, it's always added as a first name in my Google contacts. Even if N has data. The N is ignored.

When N is empty and ORG has data, the ORG is in the Organization field, and displayName is set as first name.

Here's an example showing the N is ignored.

Data from the profile screen Google contacts

You are right @AdamGrzybkowski. I messed up things with the code while testing. If I'm not wrong (again), if FN is present, Contact apps will use that value instead of N or NICKNAME (I've downloaded a couple of contact apps and the behaviour seems to be the same).

The priority is first FN, and if FN is not available, then N. I don't see Nickname being used in any case. 🤔

If that's correct, I would revert the last commit and set FN with the first and last name as we were doing when the discussion started. If those values are not available, I would use the display name. With this approach, I believe we should achieve acceptable results regardless of the platform scanning the QR. Wdyt?

We can adapt iOS also to play good with Android contact.

The priority is first FN, and if FN is not available, then N. I don't see Nickname being used in any case. 🤔

So it would be:
FN: fullName ? displayName ?

What to do when there's no fullName and ORG is set?

When N is empty and ORG has data, the ORG is in the Organization field, and displayName is set as first name.

Is this ok?

cc @pinarol

It seems that FN is parsed incorrectly on some Android devices, because the space character is treated as a delimiter(which, by the way, doesn’t comply with the spec). This becomes especially problematic when names include multiple first or last names, or prefixes.

Interestingly, when we pass an empty string to FN, the N field gets used and is parsed correctly.

That puts us in a tricky spot: leaving a “required” field empty seems to fix the issue, but it also looks like we may not have much of a choice.

We don't need to do any changes on the ORG field in the vCard template I think.

So it becomes:

FN: N: First/Last name > Display name (prioritize First/Last name over Display name) NICKNAME: Display name ORG: SomeCompany ... ...

# Conflicts: # homeUi/src/main/kotlin/com/gravatar/app/homeUi/presentation/home/share/ShareScreen.kt # homeUi/src/main/kotlin/com/gravatar/app/homeUi/presentation/home/share/ShareUiState.kt

With those classes, we kept the VCard generation isolated and easy to test.

hamorillo · 2025-07-29T12:55:29Z

@etoledom I've removed the FN and updated the logic of the vCard generation. Could you give it a try, and if everything looks good, a 👍 ? 🙏

pinarol · 2025-07-29T14:23:43Z

Hey, just to give a bit of context, we didn’t completely remove the FN field, we just left it empty since it’s required. I’m not totally sure this is the best way to handle it, but I do feel like keeping the vCard format consistent across platforms is the correct approach.

hamorillo · 2025-07-29T14:50:28Z

When the standard says The property MUST be present in the vCard object. I wouldn't say an empty value in FN is accepted. I think neither option is meeting the standard.

I'm not sure what the best approach is, so I've added the empty FN to maintain consistency with iOS.

FN: N: First/Last name > Display name NICKNAME: Display name

etoledom

Working great as expected 🎉

Tested reading the generated QR code in both iOS and Xiaomi

hamorillo requested a review from Copilot July 23, 2025 15:36

Copilot AI reviewed Jul 23, 2025

View reviewed changes

hamorillo force-pushed the hamorillo/GRA-579 branch from 6505ef7 to fda8deb Compare July 23, 2025 15:51

hamorillo marked this pull request as ready for review July 23, 2025 16:11

hamorillo requested a review from AdamGrzybkowski July 23, 2025 16:11

Base automatically changed from hamorillo/GRA-576 to trunk July 23, 2025 16:19

AdamGrzybkowski reviewed Jul 23, 2025

View reviewed changes

...Ui/src/main/kotlin/com/gravatar/app/homeUi/presentation/home/share/components/ShareHeader.kt Outdated Show resolved Hide resolved

hamorillo force-pushed the hamorillo/GRA-579 branch from 750a3f5 to a16ae6b Compare July 24, 2025 06:21

AdamGrzybkowski reviewed Jul 24, 2025

View reviewed changes

hamorillo force-pushed the hamorillo/GRA-579 branch 2 times, most recently from 8d20527 to 0365912 Compare July 29, 2025 09:49

hamorillo added 4 commits July 29, 2025 12:56

Generate QR code with vCard data

4e3fa43

# Conflicts: # homeUi/src/main/kotlin/com/gravatar/app/homeUi/presentation/home/share/ShareScreen.kt # homeUi/src/main/kotlin/com/gravatar/app/homeUi/presentation/home/share/ShareUiState.kt

Round QR code corners

d13c25b

Implemented VCard and VCard.Builder classes

0b5ded7

With those classes, we kept the VCard generation isolated and easy to test.

Escape vCard fields

006bbc3

hamorillo force-pushed the hamorillo/GRA-579 branch 2 times, most recently from b205c0a to e1a8824 Compare July 29, 2025 12:51

hamorillo force-pushed the hamorillo/GRA-579 branch from e1a8824 to 2b8446b Compare July 29, 2025 14:50

Set FN empty and apply N logic

e82d6d9

FN: N: First/Last name > Display name NICKNAME: Display name

hamorillo force-pushed the hamorillo/GRA-579 branch from 2b8446b to e82d6d9 Compare July 29, 2025 14:51

etoledom approved these changes Jul 29, 2025

View reviewed changes

hamorillo merged commit 85bc86a into trunk Jul 29, 2025
10 checks passed

hamorillo deleted the hamorillo/GRA-579 branch July 29, 2025 20:06

	val isEmailShared: Boolean = true,
	val isEmailShared: Boolean = false,

	val isPhoneShared: Boolean = true,
	val isPhoneShared: Boolean = false,

		vCardBuilder.append("FN:${("$firstName $lastName".trim()).ifEmpty { profile.displayName }}\n")
		vCardBuilder.append("NICKNAME:${profile.displayName.ifEmpty { "$firstName $lastName".trim() }}\n")

-            vCardBuilder.append("FN:${("$firstName $lastName".trim()).ifEmpty { profile.displayName }}\n")
-            vCardBuilder.append("NICKNAME:${profile.displayName.ifEmpty { "$firstName $lastName".trim() }}\n")
+            val fullName = "$firstName $lastName".trim().ifEmpty { profile.displayName }
+            vCardBuilder.append("FN:$fullName\n")
+            vCardBuilder.append("NICKNAME:${profile.displayName.ifEmpty { fullName }}\n")

-            vCardBuilder.append("N:$lastName;$firstName;;;\n")
-            vCardBuilder.append("FN:${("$firstName $lastName".trim()).ifEmpty { profile.displayName }}\n")
-            vCardBuilder.append("NICKNAME:${profile.displayName.ifEmpty { "$firstName $lastName".trim() }}\n")
-        }
+            val (nField, fnField, nicknameField) = formatName(firstName, lastName, profile.displayName)
+            vCardBuilder.append(nField)
+            vCardBuilder.append(fnField)
+            vCardBuilder.append(nicknameField)

+private fun escapeVCardValue(value: String): String {
+    return value.replace("\n", "\\n").replace(";", "\\;").replace(",", "\\,")
+}

Conversation

hamorillo commented Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing Steps

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

wpmobilebot commented Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AdamGrzybkowski left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

etoledom Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AdamGrzybkowski Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

etoledom Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pinarol Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hamorillo commented Jul 29, 2025

Uh oh!

pinarol commented Jul 29, 2025

Uh oh!

hamorillo commented Jul 29, 2025

Uh oh!

etoledom left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

hamorillo commented Jul 23, 2025 •

edited

Loading

wpmobilebot commented Jul 23, 2025 •

edited

Loading

etoledom Jul 24, 2025 •

edited

Loading

AdamGrzybkowski Jul 24, 2025 •

edited

Loading

etoledom Jul 25, 2025 •

edited

Loading

pinarol Jul 25, 2025 •

edited

Loading