From d0131eb54f8397b8b2bd62dfa1fc7a5cfd5f47a9 Mon Sep 17 00:00:00 2001
From: automated-signal <37887102+automated-signal@users.noreply.github.com>
Date: Wed, 4 Dec 2024 15:50:32 -0600
Subject: [PATCH] Filter by unread: small improvements and unit tests

Co-authored-by: yash-signal <yash@signal.org>
---
 stylesheets/components/CallsTab.scss          |   5 +-
 stylesheets/components/ClearFilterButton.scss |   8 +-
 .../components/LeftPaneSearchInput.scss       |   5 +-
 stylesheets/components/NavSidebar.scss        |   2 +-
 ts/components/CallsList.tsx                   |   1 +
 ts/components/LeftPaneSearchInput.tsx         |   3 +-
 .../leftPane/LeftPaneSearchHelper_test.ts     | 100 ++++++++++++++++++
 7 files changed, 119 insertions(+), 5 deletions(-)

diff --git a/stylesheets/components/CallsTab.scss b/stylesheets/components/CallsTab.scss
index 312ea496e8ff..95384d6eb65f 100644
--- a/stylesheets/components/CallsTab.scss
+++ b/stylesheets/components/CallsTab.scss
@@ -153,12 +153,15 @@
   white-space: nowrap;
 }
 
+.CallsList__ToggleFilterByMissedWrapper {
+  margin-inline-end: 8px;
+}
+
 .CallsList__ToggleFilterByMissed {
   @include mixins.button-reset;
   & {
     flex-shrink: 0;
     padding: 4px;
-    margin-inline-end: 8px;
     border-radius: 4px;
   }
 
diff --git a/stylesheets/components/ClearFilterButton.scss b/stylesheets/components/ClearFilterButton.scss
index ba32169ab334..d88f4ffcfa1d 100644
--- a/stylesheets/components/ClearFilterButton.scss
+++ b/stylesheets/components/ClearFilterButton.scss
@@ -21,6 +21,7 @@
 
     @include mixins.dark-theme {
       background-color: color.mix(
+        // Gray 80 is the left pane background color
         variables.$color-gray-80,
         variables.$color-gray-65,
         40%
@@ -43,7 +44,12 @@
       color: variables.$color-black;
       &:hover {
         @include mixins.not-disabled {
-          background-color: variables.$color-white;
+          background-color: color.mix(
+            // gray 04 is the left pane background color
+            variables.$color-gray-04,
+            variables.$color-gray-15,
+            40%
+          );
         }
       }
     }
diff --git a/stylesheets/components/LeftPaneSearchInput.scss b/stylesheets/components/LeftPaneSearchInput.scss
index 1bb311cfb307..5f91e9b363e5 100644
--- a/stylesheets/components/LeftPaneSearchInput.scss
+++ b/stylesheets/components/LeftPaneSearchInput.scss
@@ -72,12 +72,15 @@
     }
   }
 
+  &__FilterButtonWrapper {
+    margin-inline-end: 8px;
+  }
+
   &__FilterButton {
     @include mixins.button-reset;
     & {
       flex-shrink: 0;
       padding: 4px;
-      margin-inline-end: 8px;
       border-radius: 4px;
     }
 
diff --git a/stylesheets/components/NavSidebar.scss b/stylesheets/components/NavSidebar.scss
index e6e7f073aec1..3a93f1399456 100644
--- a/stylesheets/components/NavSidebar.scss
+++ b/stylesheets/components/NavSidebar.scss
@@ -176,7 +176,7 @@
 
 .NavSidebar__HeaderActions {
   display: flex;
-  gap: 20px;
+  gap: 8px;
   margin-block: -4px;
   align-items: center;
   justify-content: center;
diff --git a/ts/components/CallsList.tsx b/ts/components/CallsList.tsx
index c5f83247a9ac..0a4a5a67b9e0 100644
--- a/ts/components/CallsList.tsx
+++ b/ts/components/CallsList.tsx
@@ -1029,6 +1029,7 @@ export function CallsList({
           content={i18n('icu:CallsList__ToggleFilterByMissedLabel')}
           theme={Theme.Dark}
           delay={600}
+          wrapperClassName="CallsList__ToggleFilterByMissedWrapper"
         >
           <button
             className={classNames('CallsList__ToggleFilterByMissed', {
diff --git a/ts/components/LeftPaneSearchInput.tsx b/ts/components/LeftPaneSearchInput.tsx
index e361f2bb15ec..1cb3e9a13ef1 100644
--- a/ts/components/LeftPaneSearchInput.tsx
+++ b/ts/components/LeftPaneSearchInput.tsx
@@ -201,7 +201,8 @@ export function LeftPaneSearchInput({
           direction={TooltipPlacement.Bottom}
           content={i18n('icu:filterByUnreadButtonLabel')}
           theme={Theme.Dark}
-          delay={600}
+          delay={2000}
+          wrapperClassName="LeftPaneSearchInput__FilterButtonWrapper"
         >
           <button
             className={classNames('LeftPaneSearchInput__FilterButton', {
diff --git a/ts/test-node/components/leftPane/LeftPaneSearchHelper_test.ts b/ts/test-node/components/leftPane/LeftPaneSearchHelper_test.ts
index 792ffc361735..5c9f4dd34b01 100644
--- a/ts/test-node/components/leftPane/LeftPaneSearchHelper_test.ts
+++ b/ts/test-node/components/leftPane/LeftPaneSearchHelper_test.ts
@@ -106,6 +106,28 @@ describe('LeftPaneSearchHelper', () => {
 
       assert.strictEqual(helper.getRowCount(), 5);
     });
+
+    it('adds a row for the clear unread filter button when filterByUnread is true', () => {
+      const helper = new LeftPaneSearchHelper({
+        ...baseSearchHelperArgs,
+        filterByUnread: true,
+        conversationResults: {
+          isLoading: false,
+          results: [getDefaultConversation(), getDefaultConversation()],
+        },
+        contactResults: {
+          isLoading: false,
+          results: [],
+        },
+        messageResults: {
+          isLoading: false,
+          results: [],
+        },
+      });
+
+      // 2 conversations + 1 header + 1 clear filter row = 4
+      assert.strictEqual(helper.getRowCount(), 4);
+    });
   });
 
   describe('getRow', () => {
@@ -257,6 +279,53 @@ describe('LeftPaneSearchHelper', () => {
         messageId: messages[1].id,
       });
     });
+
+    it('returns the correct row for filter clear button with filterByUnread=true', () => {
+      const helper = new LeftPaneSearchHelper({
+        ...baseSearchHelperArgs,
+        filterByUnread: true,
+        conversationResults: {
+          isLoading: false,
+          results: [getDefaultConversation(), getDefaultConversation()],
+        },
+        contactResults: {
+          isLoading: false,
+          results: [],
+        },
+        messageResults: {
+          isLoading: false,
+          results: [],
+        },
+      });
+
+      // Row 0: Conversations header
+      // Row 1: First conversation
+      // Row 2: Second conversation
+      // Row 3: Clear filter button
+      assert.deepEqual(helper.getRow(3), {
+        type: RowType.ClearFilterButton,
+        isOnNoResultsPage: false,
+      });
+
+      // Out of bounds
+      assert.isUndefined(helper.getRow(4));
+    });
+
+    it('shows unread header when filterByUnread=true', () => {
+      const helper = new LeftPaneSearchHelper({
+        ...baseSearchHelperArgs,
+        filterByUnread: true,
+        conversationResults: {
+          isLoading: false,
+          results: [getDefaultConversation()],
+        },
+      });
+
+      assert.deepEqual(
+        _testHeaderText(helper.getRow(0)),
+        'icu:conversationsUnreadHeader'
+      );
+    });
   });
 
   it('omits messages when there are no message results', () => {
@@ -530,5 +599,36 @@ describe('LeftPaneSearchHelper', () => {
         helper.getConversationAndMessageAtIndex(-100)?.messageId
       );
     });
+
+    it('handles accurate row indexing when filterByUnread is enabled', () => {
+      const conversations = [
+        getDefaultConversation(),
+        getDefaultConversation(),
+      ];
+
+      const helper = new LeftPaneSearchHelper({
+        ...baseSearchHelperArgs,
+        filterByUnread: true,
+        conversationResults: {
+          isLoading: false,
+          results: conversations,
+        },
+        contactResults: { isLoading: false, results: [] },
+        messageResults: { isLoading: false, results: [] },
+      });
+
+      // Verify conversation row indexing
+      assert.strictEqual(
+        helper.getConversationAndMessageAtIndex(0)?.conversationId,
+        conversations[0].id
+      );
+      assert.strictEqual(
+        helper.getConversationAndMessageAtIndex(1)?.conversationId,
+        conversations[1].id
+      );
+
+      // Verify clear filter row is skipped (index 2 doesn't map to a conversation)
+      assert.isUndefined(helper.getConversationAndMessageAtIndex(2));
+    });
   });
 });