Skip to content

Commit af9a837

Browse files
mdecourcymdecourcy
authored
fix: duplicate LazyColumn keys for broadcast contacts (#3840)
1 parent 1c0dc48 commit af9a837

File tree

2 files changed

+116
-81
lines changed

2 files changed

+116
-81
lines changed

‎app/src/main/java/com/geeksville/mesh/ui/contact/Contacts.kt‎

Lines changed: 114 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@ import androidx.compose.foundation.layout.fillMaxSize
2424
import androidx.compose.foundation.layout.fillMaxWidth
2525
import androidx.compose.foundation.layout.padding
2626
import androidx.compose.foundation.lazy.LazyColumn
27+
import androidx.compose.foundation.lazy.LazyListScope
2728
import androidx.compose.foundation.lazy.LazyListState
28-
import androidx.compose.foundation.lazy.items
2929
import androidx.compose.foundation.lazy.rememberLazyListState
3030
import androidx.compose.foundation.selection.selectable
3131
import androidx.compose.material.icons.Icons
@@ -59,6 +59,7 @@ import androidx.compose.runtime.rememberCoroutineScope
5959
import androidx.compose.runtime.setValue
6060
import androidx.compose.ui.Alignment
6161
import androidx.compose.ui.Modifier
62+
import androidx.compose.ui.hapticfeedback.HapticFeedback
6263
import androidx.compose.ui.hapticfeedback.HapticFeedbackType
6364
import androidx.compose.ui.platform.LocalHapticFeedback
6465
import androidx.compose.ui.unit.dp
@@ -68,7 +69,6 @@ import androidx.lifecycle.compose.collectAsStateWithLifecycle
6869
import androidx.paging.LoadState
6970
import androidx.paging.compose.LazyPagingItems
7071
import androidx.paging.compose.collectAsLazyPagingItems
71-
import androidx.paging.compose.itemKey
7272
import com.geeksville.mesh.model.Contact
7373
import kotlinx.coroutines.flow.Flow
7474
import kotlinx.coroutines.flow.collectLatest
@@ -289,7 +289,7 @@ fun ContactsScreen(
289289

290290
@Suppress("LongMethod")
291291
@Composable
292-
fun MuteNotificationsDialog(
292+
private fun MuteNotificationsDialog(
293293
showDialog: Boolean,
294294
selectedContactKeys: List<String>,
295295
contactSettings: Map<String, ContactSettings>,
@@ -384,7 +384,7 @@ fun MuteNotificationsDialog(
384384
}
385385

386386
@Composable
387-
fun DeleteConfirmationDialog(
387+
private fun DeleteConfirmationDialog(
388388
showDialog: Boolean,
389389
selectedCount: Int, // Number of items to be deleted
390390
onDismiss: () -> Unit,
@@ -426,7 +426,7 @@ fun DeleteConfirmationDialog(
426426

427427
@OptIn(ExperimentalMaterial3Api::class)
428428
@Composable
429-
fun SelectionToolbar(
429+
private fun SelectionToolbar(
430430
selectedCount: Int,
431431
onCloseSelection: () -> Unit,
432432
onMuteSelected: () -> Unit,
@@ -468,116 +468,151 @@ fun SelectionToolbar(
468468
)
469469
}
470470

471-
/**
472-
* Non-paginated contact list view.
473-
*
474-
* NOTE: This is kept for ShareScreen which displays a simple contact picker. The main ContactsScreen uses
475-
* [ContactListViewPaged] for better performance.
476-
*
477-
* @see ContactListViewPaged for the paginated version
478-
*/
479471
@Composable
480-
fun ContactListView(
481-
contacts: List<Contact>,
472+
private fun ContactListViewPaged(
473+
contacts: LazyPagingItems<Contact>,
474+
channelPlaceholders: List<Contact>,
482475
selectedList: List<String>,
483476
onClick: (Contact) -> Unit,
484477
onLongClick: (Contact) -> Unit,
485478
onNodeChipClick: (Contact) -> Unit,
486479
listState: LazyListState,
480+
modifier: Modifier = Modifier,
487481
channels: AppOnlyProtos.ChannelSet? = null,
488482
) {
489483
val haptics = LocalHapticFeedback.current
490-
LazyColumn(modifier = Modifier.fillMaxSize(), state = listState) {
491-
items(contacts, key = { it.contactKey }) { contact ->
492-
val selected by remember { derivedStateOf { selectedList.contains(contact.contactKey) } }
493484

494-
ContactItem(
495-
contact = contact,
496-
selected = selected,
497-
onClick = { onClick(contact) },
498-
onLongClick = {
499-
onLongClick(contact)
500-
haptics.performHapticFeedback(HapticFeedbackType.LongPress)
501-
},
502-
channels = channels,
503-
onNodeChipClick = { onNodeChipClick(contact) },
504-
)
505-
}
485+
if (contacts.loadState.refresh is LoadState.Loading && contacts.itemCount == 0) {
486+
Box(modifier = modifier.fillMaxSize(), contentAlignment = Alignment.Center) { CircularProgressIndicator() }
487+
return
506488
}
489+
490+
val visiblePlaceholders = rememberVisiblePlaceholders(contacts, channelPlaceholders)
491+
492+
ContactListContentInternal(
493+
contacts = contacts,
494+
visiblePlaceholders = visiblePlaceholders,
495+
selectedList = selectedList,
496+
onClick = onClick,
497+
onLongClick = onLongClick,
498+
onNodeChipClick = onNodeChipClick,
499+
listState = listState,
500+
modifier = modifier,
501+
channels = channels,
502+
haptics = haptics,
503+
)
507504
}
508505

509506
@Composable
510-
fun ContactListViewPaged(
507+
private fun ContactListContentInternal(
511508
contacts: LazyPagingItems<Contact>,
512-
channelPlaceholders: List<Contact>,
509+
visiblePlaceholders: List<Contact>,
513510
selectedList: List<String>,
514511
onClick: (Contact) -> Unit,
515512
onLongClick: (Contact) -> Unit,
516513
onNodeChipClick: (Contact) -> Unit,
517514
listState: LazyListState,
515+
channels: AppOnlyProtos.ChannelSet?,
516+
haptics: HapticFeedback,
518517
modifier: Modifier = Modifier,
519-
channels: AppOnlyProtos.ChannelSet? = null,
520518
) {
521-
val haptics = LocalHapticFeedback.current
519+
LazyColumn(modifier = modifier.fillMaxSize(), state = listState) {
520+
contactListPlaceholdersItems(
521+
visiblePlaceholders = visiblePlaceholders,
522+
selectedList = selectedList,
523+
onClick = onClick,
524+
onLongClick = onLongClick,
525+
onNodeChipClick = onNodeChipClick,
526+
channels = channels,
527+
haptics = haptics,
528+
)
522529

523-
// Handle initial loading state
524-
if (contacts.loadState.refresh is LoadState.Loading && contacts.itemCount == 0) {
525-
Box(modifier = modifier.fillMaxSize(), contentAlignment = Alignment.Center) { CircularProgressIndicator() }
526-
return
530+
contactListPagedItems(
531+
contacts = contacts,
532+
selectedList = selectedList,
533+
onClick = onClick,
534+
onLongClick = onLongClick,
535+
onNodeChipClick = onNodeChipClick,
536+
channels = channels,
537+
haptics = haptics,
538+
)
539+
540+
contactListAppendLoadingItem(contacts = contacts)
527541
}
542+
}
528543

529-
val visiblePlaceholders = rememberVisiblePlaceholders(contacts, channelPlaceholders)
544+
private fun LazyListScope.contactListPlaceholdersItems(
545+
visiblePlaceholders: List<Contact>,
546+
selectedList: List<String>,
547+
onClick: (Contact) -> Unit,
548+
onLongClick: (Contact) -> Unit,
549+
onNodeChipClick: (Contact) -> Unit,
550+
channels: AppOnlyProtos.ChannelSet?,
551+
haptics: HapticFeedback,
552+
) {
553+
items(
554+
count = visiblePlaceholders.size,
555+
key = { index -> "placeholder_${visiblePlaceholders[index].contactKey}" },
556+
) { index ->
557+
val placeholder = visiblePlaceholders[index]
558+
val selected by remember { derivedStateOf { selectedList.contains(placeholder.contactKey) } }
559+
560+
ContactItem(
561+
contact = placeholder,
562+
selected = selected,
563+
onClick = { onClick(placeholder) },
564+
onLongClick = {
565+
onLongClick(placeholder)
566+
haptics.performHapticFeedback(HapticFeedbackType.LongPress)
567+
},
568+
channels = channels,
569+
onNodeChipClick = { onNodeChipClick(placeholder) },
570+
)
571+
}
572+
}
530573

531-
LazyColumn(modifier = modifier.fillMaxSize(), state = listState) {
532-
// Show channel placeholders first
533-
items(
534-
count = visiblePlaceholders.size,
535-
key = { index -> "placeholder_${visiblePlaceholders[index].contactKey}" },
536-
) { index ->
537-
val placeholder = visiblePlaceholders[index]
538-
val selected by remember { derivedStateOf { selectedList.contains(placeholder.contactKey) } }
574+
private fun LazyListScope.contactListPagedItems(
575+
contacts: LazyPagingItems<Contact>,
576+
selectedList: List<String>,
577+
onClick: (Contact) -> Unit,
578+
onLongClick: (Contact) -> Unit,
579+
onNodeChipClick: (Contact) -> Unit,
580+
channels: AppOnlyProtos.ChannelSet?,
581+
haptics: HapticFeedback,
582+
) {
583+
items(
584+
count = contacts.itemCount,
585+
key = { index ->
586+
val contact = contacts[index]
587+
contact?.let { "${it.contactKey}#$index" } ?: "contact_placeholder_$index"
588+
},
589+
) { index ->
590+
val contact = contacts[index]
591+
if (contact != null) {
592+
val selected by remember { derivedStateOf { selectedList.contains(contact.contactKey) } }
539593

540594
ContactItem(
541-
contact = placeholder,
595+
contact = contact,
542596
selected = selected,
543-
onClick = { onClick(placeholder) },
597+
onClick = { onClick(contact) },
544598
onLongClick = {
545-
onLongClick(placeholder)
599+
onLongClick(contact)
546600
haptics.performHapticFeedback(HapticFeedbackType.LongPress)
547601
},
548602
channels = channels,
549-
onNodeChipClick = { onNodeChipClick(placeholder) },
603+
onNodeChipClick = { onNodeChipClick(contact) },
550604
)
551605
}
606+
}
607+
}
552608

553-
// Show paged contacts
554-
items(count = contacts.itemCount, key = contacts.itemKey { it.contactKey }) { index ->
555-
val contact = contacts[index]
556-
if (contact != null) {
557-
val selected by remember { derivedStateOf { selectedList.contains(contact.contactKey) } }
558-
559-
ContactItem(
560-
contact = contact,
561-
selected = selected,
562-
onClick = { onClick(contact) },
563-
onLongClick = {
564-
onLongClick(contact)
565-
haptics.performHapticFeedback(HapticFeedbackType.LongPress)
566-
},
567-
channels = channels,
568-
onNodeChipClick = { onNodeChipClick(contact) },
569-
)
570-
}
571-
}
572-
573-
// Loading indicator when loading more contacts
574-
contacts.apply {
575-
when {
576-
loadState.append is LoadState.Loading -> {
577-
item(key = "append_loading") {
578-
Box(modifier = Modifier.fillMaxWidth().padding(16.dp), contentAlignment = Alignment.Center) {
579-
CircularProgressIndicator()
580-
}
609+
private fun LazyListScope.contactListAppendLoadingItem(contacts: LazyPagingItems<Contact>) {
610+
contacts.apply {
611+
when {
612+
loadState.append is LoadState.Loading -> {
613+
item(key = "append_loading") {
614+
Box(modifier = Modifier.fillMaxWidth().padding(16.dp), contentAlignment = Alignment.Center) {
615+
CircularProgressIndicator()
581616
}
582617
}
583618
}

‎app/src/main/java/com/geeksville/mesh/ui/sharing/Share.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ import androidx.compose.foundation.layout.PaddingValues
2222
import androidx.compose.foundation.layout.fillMaxWidth
2323
import androidx.compose.foundation.layout.padding
2424
import androidx.compose.foundation.lazy.LazyColumn
25-
import androidx.compose.foundation.lazy.items
25+
import androidx.compose.foundation.lazy.itemsIndexed
2626
import androidx.compose.material.icons.Icons
2727
import androidx.compose.material.icons.automirrored.filled.Send
2828
import androidx.compose.material3.Button
@@ -82,7 +82,7 @@ fun ShareScreen(contacts: List<Contact>, onConfirm: (String) -> Unit, onNavigate
8282
contentPadding = PaddingValues(6.dp),
8383
horizontalAlignment = Alignment.CenterHorizontally,
8484
) {
85-
items(contacts, key = { it.contactKey }) { contact ->
85+
itemsIndexed(contacts, key = { index, contact -> "${contact.contactKey}#$index" }) { _, contact ->
8686
val selected = contact.contactKey == selectedContact
8787
ContactItem(
8888
contact = contact,

0 commit comments

Comments
 (0)