Skip to content

Blocked Participants Screen: Java/XML to Kotlin/Compose migration#121

Open
m4pl wants to merge 19 commits into
GrapheneOS:mainfrom
m4pl:blocked-participants-compose
Open

Blocked Participants Screen: Java/XML to Kotlin/Compose migration#121
m4pl wants to merge 19 commits into
GrapheneOS:mainfrom
m4pl:blocked-participants-compose

Conversation

@m4pl
Copy link
Copy Markdown
Contributor

@m4pl m4pl commented May 22, 2026

Migrated the Blocked Participants screen from Java + XML to Kotlin + Compose. Partially reused code from the Conversation Settings migration PR to avoid conflicts and code duplication.

Reworked the data layer: the list is now built from a single SQL query that joins blocked participants with existing 1-on-1 conversations. Only blocked participants who currently have a direct chat are shown - after deleting a blocked chat the row disappears, but the participant stays blocked.

Added multi-select mode in the top bar (counter + clear selection + batched delete) and covered the screen with Compose UI tests.

Out of scope (task): the chat screen will surface a "this chat is blocked" indicator so the delete/block flow becomes more transparent to the user.

Screenshots

Before After
Screenshot_20260522_170604 Screenshot_20260522_170534

Video

Screen_recording_20260522_171007.webm

@m4pl m4pl force-pushed the blocked-participants-compose branch from 34bab44 to a876359 Compare May 24, 2026 13:07
@m4pl m4pl requested review from RankoR and inthewaves May 25, 2026 07:52
@m4pl m4pl marked this pull request as ready for review May 25, 2026 07:52
@m4pl m4pl force-pushed the blocked-participants-compose branch from 7e32488 to 26e8fe4 Compare May 28, 2026 09:08
Comment on lines +203 to +208
.clip(
RoundedCornerShape(
topStart = CardCornerRadius,
topEnd = CardCornerRadius,
),
),
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use shapes based on the the MaterialTheme shapes for this if possible

Comment on lines +11 to +22
internal class AnchorRelativePositionProvider(
private val anchorBoundsPx: IntRect,
private val gapPx: Int,
private val transformOriginState: MutableState<TransformOrigin>,
) : PopupPositionProvider {

override fun calculatePosition(
anchorBounds: IntRect,
windowSize: IntSize,
layoutDirection: LayoutDirection,
popupContentSize: IntSize,
): IntOffset {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use a Box as a parent like the conversation settings PR for cleaner code

Comment on lines +216 to +232
Column(modifier = modifier) {
Text(
text = displayName,
style = MaterialTheme.typography.bodyLarge,
color = MaterialTheme.colorScheme.onSurface,
maxLines = 1,
overflow = TextOverflow.Ellipsis,
)

Text(
text = details.orEmpty(),
style = MaterialTheme.typography.bodySmall,
color = MaterialTheme.colorScheme.onSurfaceVariant,
maxLines = 1,
overflow = TextOverflow.Ellipsis,
)
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we just have a phone number and no display name, we should vertically center the text

Comment on lines +105 to +115
private fun ImmutableSet<String>.retainKnown(
participants: List<BlockedParticipantUiState>,
): ImmutableSet<String> {
if (isEmpty()) return this

val knownIds = participants.mapTo(HashSet(participants.size)) {
it.participantId
}

return filter { it in knownIds }.toPersistentSet()
}
Copy link
Copy Markdown
Member

@inthewaves inthewaves May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

filter { it in knownIds } creates an intermediate List

PersistentSet has an APIs for this part like retainAll (renamed to retainingAll in upcoming kotlinx immutable collections 0.5.0): https://kotlin.github.io/kotlinx.collections.immutable/kotlinx-collections-immutable/kotlinx.collections.immutable/-persistent-set/retaining-all.html. This API doesn't exist on ImmutableSet

It might be worth changing selectedParticipantIds to use PersistentSet instead of ImmutableSet so that we can use retainAll directly.

Comment on lines +64 to +69
val updated = if (participantId in current.selectedParticipantIds) {
current.selectedParticipantIds - participantId
} else {
current.selectedParticipantIds + participantId
}
current.copy(selectedParticipantIds = updated.toPersistentSet())
Copy link
Copy Markdown
Member

@inthewaves inthewaves May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ImmutableSet has no ImmutableSet.minus or ImmutableSet.plus operators, so this will use Kotlin's standard Set.minus and Set.plus, hence why val updated can only be typed as a Set. Set.minus and Set.plus are generic so they will do $O(n)$ copies every time

For example, from the Kotlin standard library:

public operator fun <T> Set<T>.minus(element: T): Set<T> {
      val result = LinkedHashSet<T>(mapCapacity(size))
      var removed = false
      return this.filterTo(result) {
          if (!removed && it == element) {
              removed = true
              false
          } else {
              true
          }
      }
  }

It seems it would be better overall to use PersistentSet as the type instead of ImmutableSet in the state data class. If selectedParticipantIds was typed to be a PersistentSet, then these operations would use PersistentSet (copy-on-write) operations for adding and removing elements which looks more efficient:

PersistentCollection.plus and PersistentCollection.minus do exist: https://github.com/Kotlin/kotlinx.collections.immutable/blob/master/core/commonMain/src/extensions.kt#L71

removing calls remove (removing seems to be a new API renaming): https://github.com/Kotlin/kotlinx.collections.immutable/blob/master/core/commonMain/src/ImmutableCollection.kt#L91

PersistentOrderedSet's remove, for example, looks efficient (or at least more specialized than creating a brand-new LinkedHashSet) and reuses PersistentHashMap structure: https://github.com/Kotlin/kotlinx.collections.immutable/blob/master/core/commonMain/src/implementations/persistentOrderedSet/PersistentOrderedSet.kt#L72

Comment on lines +106 to +127
modifier = Modifier
.combinedClickable(
onClick = onClick,
onLongClick = onLongClick
)
.padding(
horizontal = ItemHorizontalPadding,
vertical = ItemVerticalPadding,
),
verticalAlignment = Alignment.CenterVertically,
) {
BlockedParticipantAvatarWithQuickActions(
participant = participant,
fallbackIcon = fallbackIcon,
isSelected = isSelected,
onAvatarClick = onAvatarClick,
quickActionsVisible = quickActionsVisible,
onDismissQuickActions = onDismissQuickActions,
onMessageClick = onMessageClick,
onCallClick = onCallClick,
onContactClick = onContactClick,
)
Copy link
Copy Markdown
Member

@inthewaves inthewaves May 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When in multi-select mode after a long press, for an unselected contact, tapping their avatar will bring up quick actions instead of selecting that contact/number. For a selected contact, tapping their avatar will do nothing. I think quick actions should be disabled when in multi-select mode

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants