VCard Detail Screen: Java/XML to Kotlin/Compose migration by m4pl · Pull Request #184 · GrapheneOS/Messaging

m4pl · 2026-07-08T01:25:08Z

Migrated the VCard Detail flow and inline vCard cards from Java + XML to Kotlin + Compose, and removed the legacy vCard Java code.

Also swapped the old MediaResourceManager/VCARD_CACHE caching for a new VCardEntryRepository on top of a Kotlin VCardParser. The old cache could show stale data for vCards backed by a contact, so those now refresh via a ContentObserver when the contact is edited.

Screenshots

Before	After

Video

Screen_recording_20260708_031103.mp4

inthewaves · 2026-07-09T08:18:21Z

+        val result = runCatching {
+            val entries = parser.parse(vCardUri)
+            cacheIfNeeded(
+                vCardUri = vCardUri,
+                entries = entries,
+                shouldCache = shouldCache,
+            )
+            entries
+        }
+
+        result.onSuccess { entries ->
+            parseHandle.deferred.complete(entries)
+        }.onFailure { throwable ->
+            parseHandle.deferred.completeExceptionally(throwable)
+        }


Need to be careful with using broad catching functions when the body is a suspend lambda/block calling suspendable (and thus possibly cancellable) functions. Kotlin coroutine cancellation is done through CancellationExceptions which should never be caught

Dropped the in-flight dedup, so there's no broad catch anymore. getEntries just parses directly and relies on the cache.

inthewaves · 2026-07-09T08:31:31Z

+        val hasFullName = fullName.isNotEmpty()
        val displayName = when {
-            fullName.isNullOrEmpty() -> {
-                bidiFormatter.unicodeWrap(participant.sendDestination.orEmpty(), LTR)
-            }
-            else -> fullName
+            hasFullName -> fullName
+            else -> participant.sendDestination.orEmpty()
        }
        val details = when {
-            fullName.isNullOrEmpty() || participant.isUnknownSender -> null
-            else -> participant.sendDestination?.let {
-                bidiFormatter.unicodeWrap(it, LTR)
-            }
+            hasFullName && !participant.isUnknownSender -> participant.sendDestination
+            else -> null


We need to retain the fullName.isNullOrEmpty() instead of fullName.isEmpty(). This currently results in an NPE when opening conversations settings for an unsaved number which would have a null fullName (should add a test for this too)

inthewaves · 2026-07-09T08:33:07Z

+        return flow {
+            emit(VCardDetailResult.Loading)
+
+            vCardEntryRepository
+                .observeEntries(
+                    vCardUri = vCardUri,
+                    refreshes = refreshes,
+                )
+                .map(::mapResult)
+                .collect(::emit)
+        }


should probably specify dispatcher via flowOn, otherwise this could flow and thus do parsing/bitmap decoding on main thread

inthewaves · 2026-07-09T08:35:41Z

        SettingsClickableItem(
            title = stringResource(R.string.mms_phone_number_pref_title),
-            summary = subscriptionSettings.displayDetail,
+            summary = subscriptionSettings.displayDetail.asLtrText(),


this should probably also be applied to SettingsMainScreen.kt:65 which also has subscription.displayDetail

inthewaves · 2026-07-09T08:37:33Z

+    private fun shouldCache(vCardUri: String): Boolean {
+        return !vCardUri.toUri().isContactVCardUri()
+    }
+
+    private fun Uri.isContactVCardUri(): Boolean {
+        val contactVCardUri = Contacts.CONTENT_VCARD_URI
+
+        return scheme == contactVCardUri.scheme &&
+            authority == contactVCardUri.authority &&
+            pathSegments.take(contactVCardUri.pathSegments.size) == contactVCardUri.pathSegments
+    }


We should probably also not cache Contacts.CONTENT_MULTI_VCARD_URI

inthewaves · 2026-07-09T08:41:35Z

+        }
+
+        return name?.takeIf(String::isNotBlank)
+    }


This seems to be doing hidden mutations in entry.consolidateFields() which isn't good practice especially if this can be called concurrently; it's probably better to do it once in e.g. VCardParser

Review lines got messed up, but it's for this:

override fun displayName(entry: CustomVCardEntry): String? { val name = entry.displayName ?: run { entry.consolidateFields() // <-- entry.displayName } return name?.takeIf(String::isNotBlank) }

Already handled by CustomVCardEntryConstructor.onEntryEnded() before onEntryCreated(), so I removed it here. displayName is now just a read.

inthewaves · 2026-07-09T08:43:08Z

+    Column(
+        modifier = Modifier
+            .fillMaxWidth()
+            .combinedClickable(
+                onClick = onClick,
+                onLongClick = onLongClick,
+            )
+            .padding(
+                horizontal = RowHorizontalPadding,
+                vertical = RowVerticalPadding,
+            ),
+        verticalArrangement = Arrangement.spacedBy(2.dp),


Note that the VCardDetailViewModel can drop clicks for VCardFieldAction.None, but semantically it will still show a click action for e.g. TalkBack

inthewaves · 2026-07-09T08:55:35Z

+    val avatarImage = remember(avatarPhoto) {
+        avatarPhoto
+            ?.asReadOnlyByteBuffer()
+            ?.let(ParticipantAvatarImage::Bytes)
+    }


Repeated code and could be refactored

inthewaves · 2026-07-09T08:55:55Z

+    val avatarImage = remember(avatarPhoto) {
+        avatarPhoto
+            ?.asReadOnlyByteBuffer()
+            ?.let(ParticipantAvatarImage::Bytes)
+    }


Repeated code and could be refactored

inthewaves · 2026-07-09T08:56:11Z

+            val avatarImage = remember(attachment.vCardUiModel.avatarPhoto) {
+                attachment.vCardUiModel.avatarPhoto
+                    ?.asReadOnlyByteBuffer()
+                    ?.let(ParticipantAvatarImage::Bytes)
+            }


Repeated code and could be refactored

inthewaves · 2026-07-09T09:02:43Z

+            var currentConversationId: String? = null
+
            conversationIdFlow.collectLatest { conversationId ->
-                _state.value = ConversationMessagesUiState.Loading
+                if (conversationId != currentConversationId) {
+                    currentConversationId = conversationId
+                    _state.value = ConversationMessagesUiState.Loading
+                }


I don't think currentConversationId and the equality check is needed. conversationIdFlow is a StateFlow, which never emits consecutive equal values

_state is also a StateFlow, so setting Loading unconditionally is also a no-op when the state is already Loading, for the same reason

inthewaves · 2026-07-09T09:06:11Z

+        itemsIndexed(
+            items = contacts,
+            key = { index, _ -> index },
+        ) { _, contact ->


This key = { index, _ -> index } is redundant since itemsIndexed documentation already says:

If null is passed the position in the list will represent the key.

inthewaves · 2026-07-09T09:07:35Z

+        return VCardFieldUiModel(
+            value = field.value,
+            displayValue = field.value,


It seems like value is always equal to displayValue

inthewaves · 2026-07-09T09:09:35Z

+    private fun contactChangeEvents(): Flow<Unit> {
+        return callbackFlow {
+            val observer = object : ContentObserver(null) {
+                override fun onChange(selfChange: Boolean) {
+                    trySend(Unit)
+                }
+            }
+
+            contentResolver.registerContentObserver(Contacts.CONTENT_URI, true, observer)
+            trySend(Unit)


Note that this trySend(Unit) will trigger a full parse after first observer registration and can cause double parsing on open; see #184 (comment)

inthewaves · 2026-07-09T09:10:43Z

    override fun onScreenForegrounded(cancelNotification: Boolean) {
+        conversationComposerAttachmentsDelegate.refresh()
+        conversationMessagesDelegate.refresh()


Note that this will trigger a vCards parse . This will do uncached contact vCards parsing twice on first open, since the repository's initial observer registration does its own trySend(Unit) from the initial refreshEvents

e.g. put a log inside of VCardEntryRepositoryImpl.parseShared before the val result = runCatching call, and 2 log lines will be printed

inthewaves · 2026-07-11T03:53:06Z

LGTM, just needs to fix conflicts with main branch

m4pl force-pushed the vcard-compose branch from ecda16c to 5cbc15c Compare July 8, 2026 18:15

m4pl requested review from RankoR and inthewaves July 8, 2026 20:22

m4pl marked this pull request as ready for review July 8, 2026 20:23

inthewaves reviewed Jul 9, 2026

View reviewed changes

inthewaves suggested changes Jul 9, 2026

View reviewed changes

m4pl marked this pull request as draft July 9, 2026 22:34

m4pl force-pushed the vcard-compose branch 3 times, most recently from f65b4a7 to 40e2935 Compare July 10, 2026 07:45

m4pl added 15 commits July 10, 2026 16:14

Extract shared contentSurfaceShape into common components

5ca61ef

Add VCard detail data layer

0a0710b

Add VCard detail screen ViewModel

53adb2b

Migrate VCardDetailActivity to Compose

13e89ce

Remove legacy VCard detail views

a1b21b6

Replace legacy vCard loader with Kotlin parser

71ecaf6

Refresh vCard attachments on contact updates

760f507

Move LTR text formatting to UI layer

b422782

Delete unused xml resources

54c036b

Refine vCard metadata refresh handling

a4af743

Unify vCard avatars and destination normalization

7c1b35d

Simplify vCard entry caching and refresh

d9f7cb7

Parse vCard details off the main thread

8b136e1

Fix crash opening conversation settings for unsaved number

04b1015

Clean up vCard UI and models

8e001f1

m4pl force-pushed the vcard-compose branch from 81be96d to 8e001f1 Compare July 10, 2026 14:14

m4pl marked this pull request as ready for review July 10, 2026 14:16

Uh oh!

Conversation

m4pl commented Jul 8, 2026

Screenshots

Video

Uh oh!

inthewaves Jul 9, 2026 • 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!

inthewaves Jul 9, 2026 • 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!

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!

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!

Choose a reason for hiding this comment

Uh oh!

inthewaves Jul 9, 2026 • 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!

inthewaves Jul 9, 2026 • 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!

inthewaves commented Jul 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

inthewaves Jul 9, 2026 •

edited

Loading

inthewaves Jul 9, 2026 •

edited

Loading

inthewaves Jul 9, 2026 •

edited

Loading

inthewaves Jul 9, 2026 •

edited

Loading