Remove `filterState`; add `focusState`

Refactor the computation of "filter state", that
is, all of the store state that impacts what
constitutes applied filters on annotations.

src/sidebar/components/filter-status.js

 import { createElement } from 'preact';
+import { useMemo } from 'preact/hooks';
 import propTypes from 'prop-types';
 
 import { countVisible } from '../util/thread';
@@ -9,10 +10,19 @@ import useRootThread from './hooks/use-root-thread';
 import useStore from '../store/use-store';
 
 /**
- * @typedef {import('../store/modules/filters').FilterState} FilterState
  * @typedef {import('../util/build-thread').Thread} Thread
  */
 
+/**
+ * @typedef FilterState
+ * @prop {string|null} filterQuery
+ * @prop {boolean} focusActive
+ * @prop {boolean} focusConfigured
+ * @prop {string|null} focusDisplayName
+ * @prop {number} forcedVisibleCount
+ * @prop {number} selectedCount
+ */
+
 /**
  * @typedef FilterStatusPanelProps
  * @prop {object} actionButton -
@@ -267,7 +277,26 @@ FocusFilterStatus.propTypes = {
  */
 export default function FilterStatus() {
   const rootThread = useRootThread();
-  const filterState = useStore(store => store.filterState());
+
+  const focusState = useStore(store => store.focusState());
+  const forcedVisibleCount = useStore(
+    store => store.forcedVisibleAnnotations().length
+  );
+  const filterQuery = useStore(store => store.filterQuery());
+  const selectedCount = useStore(store => store.selectedAnnotations().length);
+
+  // Build a memoized state object with filter and selection details
+  // This will be used by the FilterStatus subcomponents
+  const filterState = useMemo(() => {
+    return {
+      filterQuery,
+      focusActive: focusState.active,
+      focusConfigured: focusState.configured,
+      focusDisplayName: focusState.displayName,
+      forcedVisibleCount,
+      selectedCount,
+    };
+  }, [focusState, forcedVisibleCount, filterQuery, selectedCount]);
 
   if (filterState.selectedCount > 0) {
     return (

src/sidebar/components/test/filter-status-test.js

@@ -16,6 +16,14 @@ function getFilterState() {
   };
 }
 
+function getFocusState() {
+  return {
+    active: false,
+    configured: false,
+    focusDisplayName: '',
+  };
+}
+
 describe('FilterStatus', () => {
   let fakeStore;
   let fakeUseRootThread;
@@ -33,7 +41,11 @@ describe('FilterStatus', () => {
       annotationCount: sinon.stub(),
       clearSelection: sinon.stub(),
       directLinkedAnnotationId: sinon.stub(),
+      filterQuery: sinon.stub().returns(null),
       filterState: sinon.stub().returns(getFilterState()),
+      focusState: sinon.stub().returns(getFocusState()),
+      forcedVisibleAnnotations: sinon.stub().returns([]),
+      selectedAnnotations: sinon.stub().returns([]),
       toggleFocusMode: sinon.stub(),
     };
 
@@ -77,11 +89,8 @@ describe('FilterStatus', () => {
   });
 
   context('(State 2): filtered by query', () => {
-    let filterState;
-
     beforeEach(() => {
-      filterState = { ...getFilterState(), filterQuery: 'foobar' };
-      fakeStore.filterState.returns(filterState);
+      fakeStore.filterQuery.returns('foobar');
       fakeThreadUtil.countVisible.returns(1);
     });
 
@@ -105,15 +114,9 @@ describe('FilterStatus', () => {
   });
 
   context('(State 3): filtered by query with force-expanded threads', () => {
-    let filterState;
-
     beforeEach(() => {
-      filterState = {
-        ...getFilterState(),
-        filterQuery: 'foobar',
-        forcedVisibleCount: 3,
-      };
-      fakeStore.filterState.returns(filterState);
+      fakeStore.filterQuery.returns('foobar');
+      fakeStore.forcedVisibleAnnotations.returns([1, 2, 3]);
       fakeThreadUtil.countVisible.returns(5);
     });
 
@@ -130,14 +133,8 @@ describe('FilterStatus', () => {
   });
 
   context('(State 4): selected annotations', () => {
-    let filterState;
-
     beforeEach(() => {
-      filterState = {
-        ...getFilterState(),
-        selectedCount: 1,
-      };
-      fakeStore.filterState.returns(filterState);
+      fakeStore.selectedAnnotations.returns([1]);
     });
 
     it('should show the count of annotations', () => {
@@ -145,8 +142,7 @@ describe('FilterStatus', () => {
     });
 
     it('should pluralize annotations when necessary', () => {
-      filterState.selectedCount = 4;
-      fakeStore.filterState.returns(filterState);
+      fakeStore.selectedAnnotations.returns([1, 2, 3, 4]);
 
       assertFilterText(createComponent(), 'Showing 4 annotations');
     });
@@ -198,16 +194,12 @@ describe('FilterStatus', () => {
   });
 
   context('(State 5): user-focus mode active', () => {
-    let filterState;
-
     beforeEach(() => {
-      filterState = {
-        ...getFilterState(),
-        focusActive: true,
-        focusConfigured: true,
-        focusDisplayName: 'Ebenezer Studentolog',
-      };
-      fakeStore.filterState.returns(filterState);
+      fakeStore.focusState.returns({
+        active: true,
+        configured: true,
+        displayName: 'Ebenezer Studentolog',
+      });
       fakeThreadUtil.countVisible.returns(1);
     });
 
@@ -244,17 +236,13 @@ describe('FilterStatus', () => {
   });
 
   context('(State 6): user-focus mode active, filtered by query', () => {
-    let filterState;
-
     beforeEach(() => {
-      filterState = {
-        ...getFilterState(),
-        focusActive: true,
-        focusConfigured: true,
-        focusDisplayName: 'Ebenezer Studentolog',
-        filterQuery: 'biscuits',
-      };
-      fakeStore.filterState.returns(filterState);
+      fakeStore.focusState.returns({
+        active: true,
+        configured: true,
+        displayName: 'Ebenezer Studentolog',
+      });
+      fakeStore.filterQuery.returns('biscuits');
       fakeThreadUtil.countVisible.returns(1);
     });
 
@@ -289,18 +277,14 @@ describe('FilterStatus', () => {
   context(
     '(State 7): user-focus mode active, filtered by query, force-expanded threads',
     () => {
-      let filterState;
-
       beforeEach(() => {
-        filterState = {
-          ...getFilterState(),
-          focusActive: true,
-          focusConfigured: true,
-          focusDisplayName: 'Ebenezer Studentolog',
-          filterQuery: 'biscuits',
-          forcedVisibleCount: 2,
-        };
-        fakeStore.filterState.returns(filterState);
+        fakeStore.focusState.returns({
+          active: true,
+          configured: true,
+          displayName: 'Ebenezer Studentolog',
+        });
+        fakeStore.filterQuery.returns('biscuits');
+        fakeStore.forcedVisibleAnnotations.returns([1, 2]);
         fakeThreadUtil.countVisible.returns(3);
       });
 
@@ -318,17 +302,13 @@ describe('FilterStatus', () => {
   );
 
   context('(State 8): user-focus mode active, selected annotations', () => {
-    let filterState;
-
     beforeEach(() => {
-      filterState = {
-        ...getFilterState(),
-        focusActive: true,
-        focusConfigured: true,
-        focusDisplayName: 'Ebenezer Studentolog',
-        selectedCount: 2,
-      };
-      fakeStore.filterState.returns(filterState);
+      fakeStore.focusState.returns({
+        active: true,
+        configured: true,
+        displayName: 'Ebenezer Studentolog',
+      });
+      fakeStore.selectedAnnotations.returns([1, 2]);
     });
 
     it('should ignore user and display selected annotations', () => {
@@ -345,17 +325,13 @@ describe('FilterStatus', () => {
   });
 
   context('(State 9): user-focus mode active, force-expanded threads', () => {
-    let filterState;
-
     beforeEach(() => {
-      filterState = {
-        ...getFilterState(),
-        focusActive: true,
-        focusConfigured: true,
-        focusDisplayName: 'Ebenezer Studentolog',
-        forcedVisibleCount: 3,
-      };
-      fakeStore.filterState.returns(filterState);
+      fakeStore.focusState.returns({
+        active: true,
+        configured: true,
+        displayName: 'Ebenezer Studentolog',
+      });
+      fakeStore.forcedVisibleAnnotations.returns([1, 2, 3]);
       fakeThreadUtil.countVisible.returns(7);
     });
 
@@ -367,8 +343,7 @@ describe('FilterStatus', () => {
     });
 
     it('should handle cases when there are no focused-user annotations', () => {
-      filterState = { ...filterState, forcedVisibleCount: 7 };
-      fakeStore.filterState.returns(filterState);
+      fakeStore.forcedVisibleAnnotations.returns([1, 2, 3, 4, 5, 6, 7]);
       assertFilterText(
         createComponent(),
         'No annotations by Ebenezer Studentolog (and 7 more)'
@@ -385,16 +360,12 @@ describe('FilterStatus', () => {
   });
 
   context('(State 10): user-focus mode configured but inactive', () => {
-    let filterState;
-
     beforeEach(() => {
-      filterState = {
-        ...getFilterState(),
-        focusActive: false,
-        focusConfigured: true,
-        focusDisplayName: 'Ebenezer Studentolog',
-      };
-      fakeStore.filterState.returns(filterState);
+      fakeStore.focusState.returns({
+        active: false,
+        configured: true,
+        displayName: 'Ebenezer Studentolog',
+      });
       fakeThreadUtil.countVisible.returns(7);
     });
 

src/sidebar/store/modules/filters.js

 import { createSelector } from 'reselect';
 
 import { actionTypes } from '../util';
-import { trueKeys } from '../../util/collections';
 
 /**
  * @typedef FocusConfig
@@ -24,7 +23,7 @@ import { trueKeys } from '../../util/collections';
  */
 
 /**
- * @typedef FocusState
+ * @typedef Focus
  * @prop {boolean} configured - Focus config contains valid `user` and
  *           is good to go
  * @prop {boolean} active - Focus mode is currently applied
@@ -32,14 +31,10 @@ import { trueKeys } from '../../util/collections';
  */
 
 /**
- * TODO potentially split into `FilterState` and `FocusState`?
- * @typedef FilterState
- * @prop {string|null} filterQuery
- * @prop {boolean} focusActive
- * @prop {boolean} focusConfigured
- * @prop {string|null} focusDisplayName
- * @prop {number} forcedVisibleCount
- * @prop {number} selectedCount
+ * @typedef FocusState
+ * @prop {boolean} active
+ * @prop {boolean} configured
+ * @prop {string} displayName
  */
 
 /**
@@ -54,7 +49,7 @@ import { trueKeys } from '../../util/collections';
  * and may be toggled via `toggleFocusMode`.
  *
  * @param {FocusConfig} focusConfig
- * @return {FocusState}
+ * @return {Focus}
  */
 function setFocus(focusConfig) {
   const focusDefaultState = {
@@ -172,39 +167,17 @@ function filterQuery(state) {
 }
 
 /**
- * Returns the display name for a user or the userid
- * if display name is not present. If both are missing
- * then this returns an empty string.
+ * Summary of focus state
  *
- * @return {string}
+ * @type {(state: any) => FocusState}
  */
-function focusModeUserPrettyName(state) {
-  if (!state.focus.configured) {
-    return '';
-  }
-  return state.focus.user.displayName;
-}
-
-/**
- * Summary of applied filters
- *
- * @type {(state: any) => FilterState}
- */
-const filterState = createSelector(
-  rootState => rootState.selection,
-  rootState => rootState.filters,
-  (selection, filters) => {
-    // TODO FIXME
-    const forcedVisibleCount = trueKeys(selection.forcedVisible).length;
-    // TODO FIXME
-    const selectedCount = trueKeys(selection.selected).length;
+const focusState = createSelector(
+  state => state.focus,
+  focus => {
     return {
-      filterQuery: filters.query,
-      focusActive: filters.focus.active,
-      focusConfigured: filters.focus.configured,
-      focusDisplayName: focusModeUserPrettyName(filters),
-      forcedVisibleCount,
-      selectedCount,
+      active: focus.active,
+      configured: focus.configured,
+      displayName: focus.configured ? focus.user.displayName : '',
     };
   }
 );
@@ -219,9 +192,7 @@ const filterState = createSelector(
  *
  * // Selectors
  * @prop {() => string|null} filterQuery
- *
- * // Root Selectors
- * @prop {() => FilterState} filterState
+ * @prop {() => FocusState} focusState
  *
  */
 
@@ -236,8 +207,6 @@ export default {
   },
   selectors: {
     filterQuery,
-  },
-  rootSelectors: {
-    filterState,
+    focusState,
   },
 };

src/sidebar/store/modules/test/filters-test.js

@@ -94,44 +94,24 @@ describe('sidebar/store/modules/filters', () => {
   });
 
   describe('selectors', () => {
-    describe('filterState', () => {
-      it('returns the current filter query', () => {
-        store.setFilterQuery('doodah, doodah');
-        assert.equal(store.filterState().filterQuery, 'doodah, doodah');
-      });
-
+    describe('focusState', () => {
       it('returns user focus information', () => {
         store.changeFocusModeUser({
           username: 'filbert',
           displayName: 'Pantomime Nutball',
         });
 
-        const filterState = store.filterState();
-        assert.isTrue(filterState.focusActive);
-        assert.isTrue(filterState.focusConfigured);
-        assert.equal(filterState.focusDisplayName, 'Pantomime Nutball');
-      });
-
-      it('returns a count of forced-visible annotations', () => {
-        store.setForcedVisible('kaboodle', true);
-        store.setForcedVisible('stampy', false);
-
-        assert.equal(store.filterState().forcedVisibleCount, 1);
-      });
-
-      it('returns a count of selected annotations', () => {
-        store.selectAnnotations(['tabulature', 'felonious']);
-        assert.equal(store.filterState().selectedCount, 2);
+        const focusState = store.focusState();
+        assert.isTrue(focusState.active);
+        assert.isTrue(focusState.configured);
+        assert.equal(focusState.displayName, 'Pantomime Nutball');
       });
 
-      it('returns empty filter states when no filters active', () => {
-        const filterState = store.filterState();
-        assert.isFalse(filterState.focusActive);
-        assert.isFalse(filterState.focusConfigured);
-        assert.isEmpty(filterState.focusDisplayName);
-        assert.isNull(filterState.filterQuery);
-        assert.equal(filterState.forcedVisibleCount, 0);
-        assert.equal(filterState.selectedCount, 0);
+      it('returns empty focus values when no focus is configured or set', () => {
+        const focusState = store.focusState();
+        assert.isFalse(focusState.active);
+        assert.isFalse(focusState.configured);
+        assert.isEmpty(focusState.displayName);
       });
     });
   });