Sort groups you've left to the bottom of search

Co-authored-by: Scott Nonnenberg <scott@signal.org>
signalapp · May 31, 2022 · 3cc89eb · 3cc89eb
1 parent 110e596
commit 3cc89eb
Show file tree

Hide file tree

Showing 5 changed files with 18 additions and 128 deletions.
diff --git a/...onents/conversation/conversation-details/AddGroupMembersModal/ChooseGroupMembersModal.tsx b/...onents/conversation/conversation-details/AddGroupMembersModal/ChooseGroupMembersModal.tsx
@@ -20,7 +20,7 @@ import { useRestoreFocus } from '../../../../hooks/useRestoreFocus';
 import { missingCaseError } from '../../../../util/missingCaseError';
 import type { LookupConversationWithoutUuidActionsType } from '../../../../util/lookupConversationWithoutUuid';
 import { parseAndFormatPhoneNumber } from '../../../../util/libphonenumberInstance';
-import { filterAndSortConversationsByTitle } from '../../../../util/filterAndSortConversations';
+import { filterAndSortConversationsByRecent } from '../../../../util/filterAndSortConversations';
 import type { ConversationType } from '../../../../state/ducks/conversations';
 import type { PreferredBadgeSelectorType } from '../../../../state/selectors/badges';
 import type {
@@ -114,13 +114,13 @@ export const ChooseGroupMembersModal: FunctionComponent<PropsType> = ({
   const canContinue = Boolean(selectedContacts.length);
 
   const [filteredContacts, setFilteredContacts] = useState(
-    filterAndSortConversationsByTitle(candidateContacts, '', regionCode)
+    filterAndSortConversationsByRecent(candidateContacts, '', regionCode)
   );
   const normalizedSearchTerm = searchTerm.trim();
   useEffect(() => {
     const timeout = setTimeout(() => {
       setFilteredContacts(
-        filterAndSortConversationsByTitle(
+        filterAndSortConversationsByRecent(
           candidateContacts,
           normalizedSearchTerm,
           regionCode

diff --git a/ts/state/selectors/conversations.ts b/ts/state/selectors/conversations.ts
@@ -31,7 +31,7 @@ import type { PropsDataType as TimelinePropsType } from '../../components/conver
 import type { TimelineItemType } from '../../components/conversation/TimelineItem';
 import { assert } from '../../util/assert';
 import { isConversationUnregistered } from '../../util/isConversationUnregistered';
-import { filterAndSortConversationsByTitle } from '../../util/filterAndSortConversations';
+import { filterAndSortConversationsByRecent } from '../../util/filterAndSortConversations';
 import type { ContactNameColorType } from '../../types/Colors';
 import { ContactNameColors } from '../../types/Colors';
 import type { AvatarDataType } from '../../types/Avatar';
@@ -523,7 +523,7 @@ export const getFilteredComposeContacts = createSelector(
     contacts: Array<ConversationType>,
     regionCode: string | undefined
   ): Array<ConversationType> => {
-    return filterAndSortConversationsByTitle(contacts, searchTerm, regionCode);
+    return filterAndSortConversationsByRecent(contacts, searchTerm, regionCode);
   }
 );
 
@@ -536,15 +536,15 @@ export const getFilteredComposeGroups = createSelector(
     groups: Array<ConversationType>,
     regionCode: string | undefined
   ): Array<ConversationType> => {
-    return filterAndSortConversationsByTitle(groups, searchTerm, regionCode);
+    return filterAndSortConversationsByRecent(groups, searchTerm, regionCode);
   }
 );
 
 export const getFilteredCandidateContactsForNewGroup = createSelector(
   getCandidateContactsForNewGroup,
   getNormalizedComposerConversationSearchTerm,
   getRegionCode,
-  filterAndSortConversationsByTitle
+  filterAndSortConversationsByRecent
 );
 
 const getGroupCreationComposerState = createSelector(

diff --git a/ts/test-both/state/selectors/conversations_test.ts b/ts/test-both/state/selectors/conversations_test.ts
@@ -965,10 +965,10 @@ describe('both/state/selectors/conversations', () => {
 
       const ids = result.map(contact => contact.id);
       assert.deepEqual(ids, [
+        'our-conversation-id',
         'convo-1',
         'convo-5',
         'convo-6',
-        'our-conversation-id',
       ]);
     });
 

diff --git a/ts/test-both/util/filterAndSortConversations_test.ts b/ts/test-both/util/filterAndSortConversations_test.ts
@@ -4,90 +4,7 @@
 import { assert } from 'chai';
 import { getDefaultConversation } from '../helpers/getDefaultConversation';
 
-import {
-  filterAndSortConversationsByTitle,
-  filterAndSortConversationsByRecent,
-} from '../../util/filterAndSortConversations';
-
-describe('filterAndSortConversationsByTitle', () => {
-  const conversations = [
-    getDefaultConversation({
-      title: '+16505551234',
-      e164: '+16505551234',
-      name: undefined,
-      profileName: undefined,
-    }),
-    getDefaultConversation({
-      name: 'Carlos Santana',
-      title: 'Carlos Santana',
-      e164: '+16505559876',
-      username: 'thisismyusername',
-    }),
-    getDefaultConversation({
-      name: 'Aaron Aardvark',
-      title: 'Aaron Aardvark',
-    }),
-    getDefaultConversation({
-      name: 'Belinda Beetle',
-      title: 'Belinda Beetle',
-    }),
-    getDefaultConversation({
-      name: 'Belinda Zephyr',
-      title: 'Belinda Zephyr',
-    }),
-  ];
-
-  it('without a search term, sorts conversations by title (but puts no-name contacts at the bottom)', () => {
-    const titles = filterAndSortConversationsByTitle(
-      conversations,
-      '',
-      'US'
-    ).map(contact => contact.title);
-    assert.deepEqual(titles, [
-      'Aaron Aardvark',
-      'Belinda Beetle',
-      'Belinda Zephyr',
-      'Carlos Santana',
-      '+16505551234',
-    ]);
-  });
-
-  it('can search for contacts by title', () => {
-    const titles = filterAndSortConversationsByTitle(
-      conversations,
-      'belind',
-      'US'
-    ).map(contact => contact.title);
-    assert.sameMembers(titles, ['Belinda Beetle', 'Belinda Zephyr']);
-  });
-
-  it('can search for contacts by phone number (and puts no-name contacts at the bottom)', () => {
-    const titles = filterAndSortConversationsByTitle(
-      conversations,
-      '650555',
-      'US'
-    ).map(contact => contact.title);
-    assert.sameMembers(titles, ['Carlos Santana', '+16505551234']);
-  });
-
-  it('can search for contacts by formatted phone number (and puts no-name contacts at the bottom)', () => {
-    const titles = filterAndSortConversationsByTitle(
-      conversations,
-      '(650)555 12-34',
-      'US'
-    ).map(contact => contact.title);
-    assert.sameMembers(titles, ['+16505551234']);
-  });
-
-  it('can search for contacts by username', () => {
-    const titles = filterAndSortConversationsByTitle(
-      conversations,
-      'thisis',
-      'US'
-    ).map(contact => contact.title);
-    assert.sameMembers(titles, ['Carlos Santana']);
-  });
-});
+import { filterAndSortConversationsByRecent } from '../../util/filterAndSortConversations';
 
 describe('filterAndSortConversationsByRecent', () => {
   const conversations = [

diff --git a/ts/util/filterAndSortConversations.ts b/ts/util/filterAndSortConversations.ts
@@ -9,6 +9,7 @@ import { WEEK } from './durations';
 
 // Fuse.js scores have order of 0.01
 const ACTIVE_AT_SCORE_FACTOR = (1 / WEEK) * 0.01;
+const LEFT_GROUP_PENALTY = 1;
 
 const FUSE_OPTIONS: Fuse.IFuseOptions<ConversationType> = {
   // A small-but-nonzero threshold lets us match parts of E164s better, and makes the
@@ -42,8 +43,6 @@ const FUSE_OPTIONS: Fuse.IFuseOptions<ConversationType> = {
   ],
 };
 
-const collator = new Intl.Collator();
-
 const cachedIndices = new WeakMap<
   ReadonlyArray<ConversationType>,
   Fuse<ConversationType>
@@ -119,15 +118,19 @@ export function filterAndSortConversationsByRecent(
     return searchConversations(conversations, searchTerm, regionCode)
       .slice()
       .sort((a, b) => {
-        const { activeAt: aActiveAt = 0 } = a.item;
-        const { activeAt: bActiveAt = 0 } = b.item;
+        const { activeAt: aActiveAt = 0, left: aLeft = false } = a.item;
+        const { activeAt: bActiveAt = 0, left: bLeft = false } = b.item;
 
         // See: https://fusejs.io/api/options.html#includescore
         // 0 score is a perfect match, 1 - complete mismatch
         const aScore =
-          (now - aActiveAt) * ACTIVE_AT_SCORE_FACTOR + (a.score ?? 0);
+          (now - aActiveAt) * ACTIVE_AT_SCORE_FACTOR +
+          (a.score ?? 0) +
+          (aLeft ? LEFT_GROUP_PENALTY : 0);
         const bScore =
-          (now - bActiveAt) * ACTIVE_AT_SCORE_FACTOR + (b.score ?? 0);
+          (now - bActiveAt) * ACTIVE_AT_SCORE_FACTOR +
+          (b.score ?? 0) +
+          (bLeft ? LEFT_GROUP_PENALTY : 0);
 
         return aScore - bScore;
       })
@@ -142,33 +145,3 @@ export function filterAndSortConversationsByRecent(
     return a.activeAt && !b.activeAt ? -1 : 1;
   });
 }
-
-export function filterAndSortConversationsByTitle(
-  conversations: ReadonlyArray<ConversationType>,
-  searchTerm: string,
-  regionCode: string | undefined
-): Array<ConversationType> {
-  if (searchTerm.length) {
-    return searchConversations(conversations, searchTerm, regionCode)
-      .slice()
-      .sort((a, b) => {
-        return (a.score ?? 0) - (b.score ?? 0);
-      })
-      .map(result => result.item);
-  }
-
-  return conversations.concat().sort((a, b) => {
-    const aHasName = hasName(a);
-    const bHasName = hasName(b);
-
-    if (aHasName === bHasName) {
-      return collator.compare(a.title, b.title);
-    }
-
-    return aHasName && !bHasName ? -1 : 1;
-  });
-}
-
-function hasName(contact: Readonly<ConversationType>): boolean {
-  return Boolean(contact.name || contact.profileName);
-}