Blocked Participants Screen: Java/XML to Kotlin/Compose migration#121
Blocked Participants Screen: Java/XML to Kotlin/Compose migration#121m4pl wants to merge 19 commits into
Conversation
34bab44 to
a876359
Compare
7e32488 to
26e8fe4
Compare
| .clip( | ||
| RoundedCornerShape( | ||
| topStart = CardCornerRadius, | ||
| topEnd = CardCornerRadius, | ||
| ), | ||
| ), |
There was a problem hiding this comment.
Let's use shapes based on the the MaterialTheme shapes for this if possible
| 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 { |
There was a problem hiding this comment.
We should use a Box as a parent like the conversation settings PR for cleaner code
| 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, | ||
| ) | ||
| } |
There was a problem hiding this comment.
I think we just have a phone number and no display name, we should vertically center the text
| 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() | ||
| } |
There was a problem hiding this comment.
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.
| val updated = if (participantId in current.selectedParticipantIds) { | ||
| current.selectedParticipantIds - participantId | ||
| } else { | ||
| current.selectedParticipantIds + participantId | ||
| } | ||
| current.copy(selectedParticipantIds = updated.toPersistentSet()) |
There was a problem hiding this comment.
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
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
| 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, | ||
| ) |
There was a problem hiding this comment.
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
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
Video
Screen_recording_20260522_171007.webm