From befe424f7bff3d3d140137780ca2e639386bbce6 Mon Sep 17 00:00:00 2001
From: Robert Knight <robertknight@gmail.com>
Date: Fri, 19 Jan 2024 12:04:56 +0000
Subject: [PATCH] Make selection override other filters

When the user clicks highlight(s) in the document and makes a "selection" in the
sidebar, we want the selected threads to always be visible. Previously this was
handled by clearing other types of filter when the selection was made, in
addition to setting the selection. This however meant that after the selection
was cleared, the state was different than prior to making the selection, with
any non-selection filters being disabled.

This commit changes the behavior so that the selection, if present, temporarily
overrides other filters rather than being combined with them. This allows the
selection to later be removed to revert back to the previous state.
---
 src/sidebar/helpers/build-thread.ts           | 13 ++++++++-----
 src/sidebar/helpers/test/build-thread-test.js | 17 ++++++++++++++---
 2 files changed, 22 insertions(+), 8 deletions(-)

diff --git a/src/sidebar/helpers/build-thread.ts b/src/sidebar/helpers/build-thread.ts
index 40aa898c6c9..ac80207d8db 100644
--- a/src/sidebar/helpers/build-thread.ts
+++ b/src/sidebar/helpers/build-thread.ts
@@ -307,18 +307,15 @@ export function buildThread(
 
   let thread = threadAnnotations(annotations);
 
+  // When a selection is present, it overrides other filters.
   if (hasSelection) {
-    // Remove threads (annotations) that are not selected or
-    // are not forced-visible
     thread.children = thread.children.filter(child => {
       const isSelected = options.selected.includes(child.id);
       const isForcedVisible =
         hasForcedVisible && options.forcedVisible.includes(child.id);
       return isSelected || isForcedVisible;
     });
-  }
-
-  if (options.threadFilterFn) {
+  } else if (options.threadFilterFn) {
     // Remove threads not matching thread-level filters
     thread.children = thread.children.filter(options.threadFilterFn);
   }
@@ -329,6 +326,12 @@ export function buildThread(
   // are the top-level annotations.
   thread.visible = false;
   thread = mapThread(thread, thread => {
+    if (hasSelection) {
+      // When a selection is active, make the full conversation thread for
+      // each selected annotation visible.
+      return { ...thread, visible: true };
+    }
+
     let threadIsVisible = thread.visible;
 
     if (options.filterFn) {
diff --git a/src/sidebar/helpers/test/build-thread-test.js b/src/sidebar/helpers/test/build-thread-test.js
index fbc08e68cd5..87ed329eee3 100644
--- a/src/sidebar/helpers/test/build-thread-test.js
+++ b/src/sidebar/helpers/test/build-thread-test.js
@@ -413,16 +413,27 @@ describe('sidebar/helpers/build-thread', () => {
 
     context('when there is a selection', () => {
       it('shows only selected annotations', () => {
-        const thread = createThread(SIMPLE_FIXTURE, {
-          selected: ['1'],
-        });
+        const thread = createThread(
+          SIMPLE_FIXTURE,
+          {
+            selected: ['1'],
+
+            // Other thread and annotation-level filters should be ignored
+            // when there is a selection.
+            threadFilterFn: () => false,
+            filterFn: () => false,
+          },
+          ['visible'],
+        );
         assert.deepEqual(thread, [
           {
             annotation: SIMPLE_FIXTURE[0],
+            visible: true,
             children: [
               {
                 annotation: SIMPLE_FIXTURE[2],
                 children: [],
+                visible: true,
               },
             ],
           },