Fix contact search list flickering on query change.

2026-07-17 11:12:22 +01:00 · 2026-06-10 11:57:23 -03:00
parent 9e3ee16e65
commit 4cdd1f70ac
6 changed files with 162 additions and 30 deletions
@@ -35,6 +35,7 @@ public class ContactRepository {

  public static final String ID_COLUMN           = "id";
  public static final String NAME_COLUMN         = "name";
+  public static final String SORT_NAME_COLUMN    = "sort_name";
         static final String NUMBER_COLUMN       = "number";
         static final String NUMBER_TYPE_COLUMN  = "number_type";
         static final String LABEL_COLUMN        = "label";
@@ -55,6 +56,11 @@ public class ContactRepository {
      return Util.getFirstNonEmpty(system, profile);
    }));

+    // The key the results are actually ordered by (nickname/system/profile/username, lowercased). Letter
+    // headers must derive from this rather than NAME_COLUMN, which omits nickname/username and can begin
+    // with a different letter than the row's sort position.
+    add(new Pair<>(SORT_NAME_COLUMN, cursor -> CursorUtil.requireString(cursor, RecipientTable.SORT_NAME)));
+
    add(new Pair<>(NUMBER_COLUMN, cursor -> {
      String phone = CursorUtil.requireString(cursor, RecipientTable.E164);
      String email = CursorUtil.requireString(cursor, RecipientTable.EMAIL);
@@ -18,7 +18,6 @@ import org.thoughtcrime.securesms.database.model.ThreadWithRecipient
 import org.thoughtcrime.securesms.keyvalue.StorySend
 import org.thoughtcrime.securesms.phonenumbers.NumberUtil
 import org.thoughtcrime.securesms.recipients.Recipient
-import org.thoughtcrime.securesms.recipients.RecipientId
 import org.thoughtcrime.securesms.search.MessageResult
 import org.thoughtcrime.securesms.search.MessageSearchResult
 import org.thoughtcrime.securesms.search.SearchRepository
@@ -240,13 +239,33 @@ class ContactSearchPagedDataSource(
    return 0
  }

-  private fun getNonGroupHeaderLetterMap(section: ContactSearchConfiguration.Section.Individuals, query: String?): Map<RecipientId, String> {
-    return contactSearchPagedDataSourceRepository.querySignalContactLetterHeaders(
-      query = query,
-      includeSelfMode = section.includeSelfMode,
-      includePush = true,
-      includeSms = false
-    )
+  /**
+   * Returns the letter header to display above the recipient at the cursor's current row, or null if
+   * none should be shown. A header is shown only when this row begins a new letter group, determined by
+   * comparing its letter to the immediately preceding row in display order. Peeking that single adjacent
+   * row means a letter group split across pages still yields exactly one header, anchored to the first
+   * row of the group, without re-scanning the whole contact set.
+   *
+   * The cursor is restored to its original position before returning so iteration is unaffected.
+   */
+  private fun getHeaderLetterForCurrentRow(cursor: Cursor): String? {
+    val position = cursor.position
+    val currentLetter = letterForCurrentRow(cursor) ?: return null
+
+    if (position <= 0) {
+      return currentLetter
+    }
+
+    cursor.moveToPosition(position - 1)
+    val previousLetter = letterForCurrentRow(cursor)
+    cursor.moveToPosition(position)
+
+    return if (previousLetter != currentLetter) currentLetter else null
+  }
+
+  private fun letterForCurrentRow(cursor: Cursor): String? {
+    val sortName = cursor.getString(cursor.getColumnIndexOrThrow(ContactRepository.SORT_NAME_COLUMN))
+    return sortName?.takeIf { it.isNotEmpty() }?.first()?.uppercaseChar()?.toString()
  }

  private fun getStoriesSearchIterator(query: String?): ContactSearchIterator<Cursor> {
@@ -379,12 +398,6 @@ class ContactSearchPagedDataSource(
  }

  private fun getNonGroupContactsData(section: ContactSearchConfiguration.Section.Individuals, query: String?, startIndex: Int, endIndex: Int): List<ContactSearchData> {
-    val headerMap: Map<RecipientId, String> = if (section.includeLetterHeaders) {
-      getNonGroupHeaderLetterMap(section, query)
-    } else {
-      emptyMap()
-    }
-
    return getNonGroupSearchIterator(section, query).use { records ->
      readContactData(
        records = records,
@@ -394,7 +407,8 @@ class ContactSearchPagedDataSource(
        endIndex = endIndex,
        recordMapper = {
          val recipient = contactSearchPagedDataSourceRepository.getRecipientFromSearchCursor(it)
-          ContactSearchData.KnownRecipient(section.sectionKey, recipient, headerLetter = headerMap[recipient.id])
+          val headerLetter = if (section.includeLetterHeaders) getHeaderLetterForCurrentRow(it) else null
+          ContactSearchData.KnownRecipient(section.sectionKey, recipient, headerLetter = headerLetter)
        }
      )
    }
@@ -43,10 +43,6 @@ open class ContactSearchPagedDataSourceRepository(
    return contactRepository.querySignalContacts(contactsSearchQuery)
  }

-  open fun querySignalContactLetterHeaders(query: String?, includeSelfMode: RecipientTable.IncludeSelfMode, includePush: Boolean, includeSms: Boolean): Map<RecipientId, String> {
-    return SignalDatabase.recipients.querySignalContactLetterHeaders(query ?: "", includeSelfMode, includePush, includeSms)
-  }
-
  open fun queryGroupMemberContacts(query: String?): Cursor? {
    return contactRepository.queryGroupMemberContacts(query ?: "")
  }
@@ -20,7 +20,6 @@ import kotlinx.coroutines.flow.SharedFlow
 import kotlinx.coroutines.flow.SharingStarted
 import kotlinx.coroutines.flow.StateFlow
 import kotlinx.coroutines.flow.combine
-import kotlinx.coroutines.flow.debounce
 import kotlinx.coroutines.flow.drop
 import kotlinx.coroutines.flow.flatMapLatest
 import kotlinx.coroutines.flow.flowOf
@@ -98,7 +97,7 @@ class ContactSearchViewModel(

  init {
    viewModelScope.launch {
-      rawQuery.drop(1).debounce(300).collect { query ->
+      rawQuery.drop(1).collect { query ->
        savedStateHandle[QUERY] = query
        internalConfigurationState.update { it.copy(query = query) }
      }
@@ -148,15 +147,17 @@ class ContactSearchViewModel(
  }

  suspend fun setConfiguration(contactSearchConfiguration: ContactSearchConfiguration) {
-    val pagedDataSource = ContactSearchPagedDataSource(
-      contactSearchConfiguration,
-      arbitraryRepository = arbitraryRepository,
-      searchRepository = searchRepository,
-      contactSearchPagedDataSourceRepository = contactSearchPagedDataSourceRepository
-    )
-    val size = withContext(Dispatchers.IO) { pagedDataSource.size() }
+    val (pagedDataSource, size) = withContext(Dispatchers.IO) {
+      val source = ContactSearchPagedDataSource(
+        contactSearchConfiguration,
+        arbitraryRepository = arbitraryRepository,
+        searchRepository = searchRepository,
+        contactSearchPagedDataSourceRepository = contactSearchPagedDataSourceRepository
+      )
+      source to source.size()
+    }
    internalTotalCount.value = size
-    pagedData.value = PagedData.createForStateFlow(pagedDataSource, pagingConfig)
+    pagedData.value = PagedData.createForStateFlow(pagedDataSource, pagingConfig, data.value)
  }

  fun setQuery(query: String?) {
@@ -20,6 +20,7 @@ import org.signal.paging.PagedDataSource
 import org.thoughtcrime.securesms.database.RecipientTable
 import org.thoughtcrime.securesms.database.SignalDatabase
 import org.thoughtcrime.securesms.keyvalue.StorySend
+import org.thoughtcrime.securesms.profiles.ProfileName
 import org.thoughtcrime.securesms.recipients.RecipientId
 import org.thoughtcrime.securesms.testutil.RecipientTestRule
 import java.util.UUID
@@ -67,6 +68,109 @@ class ContactSearchPagedDataSourceTest_letterHeaders {
    )
  }

+  @Test
+  fun `each letter group yields exactly one header anchored to its first contact`() {
+    val aaron = recipients.createRecipient("Aaron")
+    val bella = recipients.createRecipient("Bella")
+    val carol = recipients.createRecipient("Carol")
+    val carrie = recipients.createRecipient("Carrie")
+    val casey = recipients.createRecipient("Casey")
+    val dana = recipients.createRecipient("Dana")
+
+    val dataSource = buildIndividualsDataSource()
+    val size = dataSource.size()
+    val rows = dataSource.load(0, size, size, PagedDataSource.CancellationSignal { false })
+
+    // Single-contact groups always carry their own header.
+    assertEquals("A", rows.headerFor(aaron))
+    assertEquals("B", rows.headerFor(bella))
+    assertEquals("D", rows.headerFor(dana))
+
+    // The three-contact C group must contribute exactly one "C" header (on whichever sorts first),
+    // not one per contact.
+    val cHeaders = listOf(carol, carrie, casey).mapNotNull { rows.headerFor(it) }
+    assertEquals("exactly one C header across the C group", listOf("C"), cHeaders)
+  }
+
+  @Test
+  fun `a letter group split across two page loads still yields a single header on the first page`() {
+    val aaron = recipients.createRecipient("Aaron")
+    val bella = recipients.createRecipient("Bella")
+    val carol = recipients.createRecipient("Carol")
+    val carrie = recipients.createRecipient("Carrie")
+    val casey = recipients.createRecipient("Casey")
+    val dana = recipients.createRecipient("Dana")
+
+    val dataSource = buildIndividualsDataSource()
+    val size = dataSource.size()
+
+    // Sorted order is [Aaron(A), Bella(B), C, C, C, Dana(D)], so loading in two pages of three splits
+    // the C group: one C ends the first page, two C's begin the second.
+    val firstPage = dataSource.load(0, 3, size, PagedDataSource.CancellationSignal { false })
+    val secondPage = dataSource.load(3, 3, size, PagedDataSource.CancellationSignal { false })
+
+    val cContacts = listOf(carol, carrie, casey)
+    val firstPageCHeaders = cContacts.mapNotNull { firstPage.knownRecipient(it)?.headerLetter }
+    val secondPageCHeaders = cContacts.mapNotNull { secondPage.knownRecipient(it)?.headerLetter }
+
+    // The single C header lands on the first contact of the run (first page); the C's that begin the
+    // second page must NOT repeat it.
+    assertEquals("C header belongs to the first contact of the run, on the first page", listOf("C"), firstPageCHeaders)
+    assertEquals("the second page continues the C group and must not repeat the header", emptyList<String>(), secondPageCHeaders)
+    assertEquals("D", secondPage.headerFor(dana))
+
+    // Sanity: the C group genuinely straddled the page boundary.
+    assertEquals("one C contact on the first page", 1, cContacts.count { firstPage.knownRecipient(it) != null })
+    assertEquals("two C contacts on the second page", 2, cContacts.count { secondPage.knownRecipient(it) != null })
+  }
+
+  @Test
+  fun `letter header reflects the sort name, not the display name, when a nickname diverges from the profile name`() {
+    recipients.createRecipient("Bella")
+    val mikeSortsAsCharlie = recipients.createRecipient("Mike")
+    SignalDatabase.recipients.setNicknameAndNote(mikeSortsAsCharlie, ProfileName.fromParts("Charlie", null), "")
+    recipients.createRecipient("Dan")
+
+    val dataSource = buildIndividualsDataSource()
+    val size = dataSource.size()
+    val rows = dataSource.load(0, size, size, PagedDataSource.CancellationSignal { false })
+
+    // This contact's nickname ("Charlie") sorts it among the C's even though its profile name is "Mike".
+    // The header must follow the sort key (C), never the display name (M).
+    assertEquals("header follows the sort key, not the display name", "C", rows.headerFor(mikeSortsAsCharlie))
+
+    // End to end: headers march B, C, D in order, with no stray "M" leaking out of the display name.
+    val headers = rows.filterIsInstance<ContactSearchData.KnownRecipient>().mapNotNull { it.headerLetter }
+    assertEquals(listOf("B", "C", "D"), headers)
+  }
+
+  private fun buildIndividualsDataSource(): ContactSearchPagedDataSource {
+    return ContactSearchPagedDataSource(
+      contactConfiguration = ContactSearchConfiguration.build {
+        addSection(
+          ContactSearchConfiguration.Section.Individuals(
+            includeHeader = false,
+            includeSelfMode = RecipientTable.IncludeSelfMode.Exclude,
+            includeLetterHeaders = true,
+            transportType = ContactSearchConfiguration.TransportType.ALL
+          )
+        )
+      },
+      contactSearchPagedDataSourceRepository = object : ContactSearchPagedDataSourceRepository(ApplicationProvider.getApplicationContext()) {
+        override fun getLatestStorySends(activeStoryCutoffDuration: Long): List<StorySend> = emptyList()
+      }
+    )
+  }
+
+  private fun List<ContactSearchData>.knownRecipient(id: RecipientId): ContactSearchData.KnownRecipient? {
+    return filterIsInstance<ContactSearchData.KnownRecipient>().firstOrNull { it.recipient.id == id }
+  }
+
+  private fun List<ContactSearchData>.headerFor(id: RecipientId): String? {
+    val match = knownRecipient(id) ?: error("Recipient $id not present in $this")
+    return match.headerLetter
+  }
+
  private fun insertUnregisteredSystemContact(name: String): RecipientId {
    val rowId = SignalDatabase.recipients.writableDatabase.insertOrThrow(
      RecipientTable.TABLE_NAME,
@@ -40,7 +40,18 @@ public class PagedData<Key> {

  @AnyThread
  public static <Key, Data> StateFlowPagedData<Key, Data> createForStateFlow(@NonNull PagedDataSource<Key, Data> dataSource, @NonNull PagingConfig config) {
-    MutableStateFlow<List<Data>> stateFlow  = kotlinx.coroutines.flow.StateFlowKt.MutableStateFlow(java.util.Collections.emptyList());
+    return createForStateFlow(dataSource, config, java.util.Collections.emptyList());
+  }
+
+  /**
+   * Same as {@link #createForStateFlow(PagedDataSource, PagingConfig)}, but seeds the backing
+   * {@link StateFlow} with {@code initialValue} instead of an empty list. Useful when rebuilding a
+   * source (e.g. on a query change) and you want the previously-loaded items to remain visible until
+   * the new source's first page arrives, rather than briefly collapsing the list to empty.
+   */
+  @AnyThread
+  public static <Key, Data> StateFlowPagedData<Key, Data> createForStateFlow(@NonNull PagedDataSource<Key, Data> dataSource, @NonNull PagingConfig config, @NonNull List<Data> initialValue) {
+    MutableStateFlow<List<Data>> stateFlow  = kotlinx.coroutines.flow.StateFlowKt.MutableStateFlow(initialValue);
    PagingController<Key>        controller = new BufferedPagingController<>(dataSource, config, stateFlow::setValue);

    return new StateFlowPagedData<>(stateFlow, controller);