Merge pull request #2020 from hypothesis/clear-selection-when-changing-user

Clear selection when changing user

src/sidebar/components/search-status-bar.js

@@ -47,7 +47,7 @@ function SearchStatusBar({ rootThread }) {
     filterQuery: store.getState().selection.filterQuery,
     focusModeFocused: store.focusModeFocused(),
     focusModeUserPrettyName: store.focusModeUserPrettyName(),
-    selectionMap: store.getState().selection.selectedAnnotationMap,
+    selectionMap: store.getSelectedAnnotationMap(),
     selectedTab: store.getState().selection.selectedTab,
   }));
 

src/sidebar/components/sidebar-content.js

@@ -39,10 +39,9 @@ function SidebarContentController(
    * not the order in which they appear in the document.
    */
   function firstSelectedAnnotation() {
-    if (store.getState().selection.selectedAnnotationMap) {
-      const id = Object.keys(
-        store.getState().selection.selectedAnnotationMap
-      )[0];
+    const selectedAnnotationMap = store.getSelectedAnnotationMap();
+    if (selectedAnnotationMap) {
+      const id = Object.keys(selectedAnnotationMap)[0];
       return store.getState().annotations.annotations.find(function (annot) {
         return annot.id === id;
       });

src/sidebar/components/test/search-status-bar-test.js

@@ -26,6 +26,7 @@ describe('SearchStatusBar', () => {
       annotationCount: sinon.stub().returns(1),
       focusModeFocused: sinon.stub().returns(false),
       focusModeUserPrettyName: sinon.stub().returns('Fake User'),
+      getSelectedAnnotationMap: sinon.stub(),
       noteCount: sinon.stub().returns(0),
     };
 
@@ -237,12 +238,12 @@ describe('SearchStatusBar', () => {
           fakeStore.getState.returns({
             selection: {
               filterQuery: null,
-              selectedAnnotationMap: { annId: true },
               selectedTab: test.tab,
             },
           });
           fakeStore.annotationCount.returns(test.annotationCount);
           fakeStore.noteCount.returns(test.noteCount);
+          fakeStore.getSelectedAnnotationMap.returns({ annId: true });
 
           const wrapper = createComponent({});
 
@@ -266,10 +267,10 @@ describe('SearchStatusBar', () => {
             selection: {
               filterQuery: null,
               selectedTab: test.tab,
-              selectedAnnotationMap: { annId: true },
             },
           });
           fakeStore.annotationCount.returns(5);
+          fakeStore.getSelectedAnnotationMap.returns({ annId: true });
           fakeStore.noteCount.returns(3);
 
           const wrapper = createComponent({});

src/sidebar/components/test/sidebar-content-test.js

@@ -226,7 +226,7 @@ describe('sidebar.components.sidebar-content', function () {
   describe('when an annotation is anchored', function () {
     it('focuses and scrolls to the annotation if already selected', function () {
       const uri = 'http://example.com';
-      store.selectAnnotations(['123']);
+      store.getSelectedAnnotationMap = sinon.stub().returns({ '123': true });
       setFrames([{ uri: uri }]);
       const annot = {
         $tag: 'atag',

src/sidebar/services/root-thread.js

@@ -89,9 +89,9 @@ export default function RootThread(
     // determines what is visible and build the visible thread structure
     return buildThread(state.annotations.annotations, {
       forceVisible: truthyKeys(state.selection.forceVisible),
-      expanded: state.selection.expanded,
+      expanded: store.expandedThreads(),
       highlighted: state.selection.highlighted,
-      selected: truthyKeys(state.selection.selectedAnnotationMap || {}),
+      selected: truthyKeys(store.getSelectedAnnotationMap() || {}),
       sortCompareFn: sortFn,
       filterFn: filterFn,
       threadFilterFn: threadFilterFn,

src/sidebar/services/test/root-thread-test.js

@@ -46,12 +46,9 @@ describe('rootThread', function () {
         },
         drafts: [],
         selection: {
-          expanded: {},
           filterQuery: null,
-          focusedAnnotationMap: null,
           forceVisible: {},
           highlighted: [],
-          selectedAnnotationMap: null,
           sortKey: 'Location',
           sortKeysAvailable: ['Location'],
         },
@@ -64,6 +61,8 @@ describe('rootThread', function () {
         return this.state;
       },
 
+      expandedThreads: sinon.stub().returns({}),
+      getSelectedAnnotationMap: sinon.stub().returns(null),
       subscribe: sinon.stub(),
       removeAnnotations: sinon.stub(),
       removeSelectedAnnotation: sinon.stub(),
@@ -129,10 +128,10 @@ describe('rootThread', function () {
     });
 
     it('passes the current selection to buildThread()', function () {
-      fakeStore.state.selection.selectedAnnotationMap = {
+      fakeStore.getSelectedAnnotationMap.returns({
         id1: true,
         id2: true,
-      };
+      });
       rootThread.thread(fakeStore.state);
       assert.calledWith(
         fakeBuildThread,
@@ -144,7 +143,7 @@ describe('rootThread', function () {
     });
 
     it('passes the current expanded set to buildThread()', function () {
-      fakeStore.state.selection.expanded = { id1: true, id2: true };
+      fakeStore.expandedThreads.returns({ id1: true, id2: true });
       rootThread.thread(fakeStore.state);
       assert.calledWith(
         fakeBuildThread,

src/sidebar/store/modules/selection.js

@@ -138,6 +138,7 @@ const update = {
     const tabSettings = setTab(state.selectedTab, selectedTab);
     return {
       filterQuery: null,
+      forceVisible: {},
       selectedAnnotationMap: null,
       ...tabSettings,
     };
@@ -374,14 +375,15 @@ function setFocusModeFocused(focused) {
 }
 
 /**
- * Changes the focused user and sets focused enabled to `true`.
+ * Clears any applied filters, changes the focused user and sets
+ * focused enabled to `true`.
  *
  * @param {User} user - The user to focus on
  */
 function changeFocusModeUser(user) {
-  return {
-    type: actions.CHANGE_FOCUS_MODE_USER,
-    user,
+  return function (dispatch) {
+    dispatch({ type: actions.CLEAR_SELECTION });
+    dispatch({ type: actions.CHANGE_FOCUS_MODE_USER, user });
   };
 }
 
@@ -428,6 +430,10 @@ const getFirstSelectedAnnotationId = createSelector(
   selected => (selected ? Object.keys(selected)[0] : null)
 );
 
+function expandedThreads(state) {
+  return state.selection.expanded;
+}
+
 function filterQuery(state) {
   return state.selection.filterQuery;
 }
@@ -502,6 +508,10 @@ function focusModeUserPrettyName(state) {
   }
 }
 
+function getSelectedAnnotationMap(state) {
+  return state.selection.selectedAnnotationMap;
+}
+
 export default {
   init: init,
   namespace: 'selection',
@@ -525,6 +535,7 @@ export default {
 
   selectors: {
     hasSelectedAnnotations,
+    expandedThreads,
     filterQuery,
     focusModeFocused,
     focusModeEnabled,
@@ -533,5 +544,6 @@ export default {
     focusModeUserPrettyName,
     isAnnotationSelected,
     getFirstSelectedAnnotationId,
+    getSelectedAnnotationMap,
   },
 };

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

@@ -38,7 +38,7 @@ describe('sidebar/store/modules/selection', () => {
   describe('setCollapsed()', function () {
     it('sets the expanded state of the annotation', function () {
       store.setCollapsed('parent_id', false);
-      assert.deepEqual(getSelectionState().expanded, { parent_id: true });
+      assert.deepEqual(store.expandedThreads(), { parent_id: true });
     });
   });
 
@@ -109,7 +109,7 @@ describe('sidebar/store/modules/selection', () => {
   describe('selectAnnotations()', function () {
     it('adds the passed annotations to the selectedAnnotationMap', function () {
       store.selectAnnotations([1, 2, 3]);
-      assert.deepEqual(getSelectionState().selectedAnnotationMap, {
+      assert.deepEqual(store.getSelectedAnnotationMap(), {
         1: true,
         2: true,
         3: true,
@@ -119,7 +119,7 @@ describe('sidebar/store/modules/selection', () => {
     it('replaces any annotations originally in the map', function () {
       store.selectAnnotations([1]);
       store.selectAnnotations([2, 3]);
-      assert.deepEqual(getSelectionState().selectedAnnotationMap, {
+      assert.deepEqual(store.getSelectedAnnotationMap(), {
         2: true,
         3: true,
       });
@@ -128,7 +128,7 @@ describe('sidebar/store/modules/selection', () => {
     it('nulls the map if no annotations are selected', function () {
       store.selectAnnotations([1]);
       store.selectAnnotations([]);
-      assert.isNull(getSelectionState().selectedAnnotationMap);
+      assert.isNull(store.getSelectedAnnotationMap());
     });
   });
 
@@ -136,7 +136,7 @@ describe('sidebar/store/modules/selection', () => {
     it('adds annotations missing from the selectedAnnotationMap', function () {
       store.selectAnnotations([1, 2]);
       store.toggleSelectedAnnotations([3, 4]);
-      assert.deepEqual(getSelectionState().selectedAnnotationMap, {
+      assert.deepEqual(store.getSelectedAnnotationMap(), {
         1: true,
         2: true,
         3: true,
@@ -147,7 +147,7 @@ describe('sidebar/store/modules/selection', () => {
     it('removes annotations already in the selectedAnnotationMap', function () {
       store.selectAnnotations([1, 3]);
       store.toggleSelectedAnnotations([1, 2]);
-      assert.deepEqual(getSelectionState().selectedAnnotationMap, {
+      assert.deepEqual(store.getSelectedAnnotationMap(), {
         2: true,
         3: true,
       });
@@ -156,7 +156,7 @@ describe('sidebar/store/modules/selection', () => {
     it('nulls the map if no annotations are selected', function () {
       store.selectAnnotations([1]);
       store.toggleSelectedAnnotations([1]);
-      assert.isNull(getSelectionState().selectedAnnotationMap);
+      assert.isNull(store.getSelectedAnnotationMap());
     });
   });
 
@@ -164,7 +164,7 @@ describe('sidebar/store/modules/selection', () => {
     it('removing an annotation should also remove it from selectedAnnotationMap', function () {
       store.selectAnnotations([1, 2, 3]);
       store.removeAnnotations([{ id: 2 }]);
-      assert.deepEqual(getSelectionState().selectedAnnotationMap, {
+      assert.deepEqual(store.getSelectedAnnotationMap(), {
         1: true,
         3: true,
       });
@@ -175,7 +175,7 @@ describe('sidebar/store/modules/selection', () => {
     it('removes all annotations from the selection', function () {
       store.selectAnnotations([1]);
       store.clearSelectedAnnotations();
-      assert.isNull(getSelectionState().selectedAnnotationMap);
+      assert.isNull(store.getSelectedAnnotationMap());
     });
 
     it('clears the current search query', function () {
@@ -229,6 +229,19 @@ describe('sidebar/store/modules/selection', () => {
       assert.equal(store.focusModeFocused(), false);
       assert.equal(store.focusModeEnabled(), false);
     });
+
+    it('clears other applied selections', () => {
+      store.setFocusModeFocused(true);
+      store.setForceVisible('someAnnotationId');
+      store.setFilterQuery('somequery');
+      store.changeFocusModeUser({
+        username: 'testuser',
+        displayName: 'Test User',
+      });
+
+      assert.isEmpty(getSelectionState().forceVisible);
+      assert.isNull(store.filterQuery());
+    });
   });
 
   describe('setFocusModeFocused()', function () {

src/sidebar/store/test/index-test.js

@@ -37,19 +37,19 @@ describe('store', function () {
   describe('initialization', function () {
     it('does not set a selection when settings.annotations is null', function () {
       assert.isFalse(store.hasSelectedAnnotations());
-      assert.equal(Object.keys(store.getState().selection.expanded).length, 0);
+      assert.equal(Object.keys(store.expandedThreads()).length, 0);
     });
 
     it('sets the selection when settings.annotations is set', function () {
       store = storeFactory(fakeRootScope, { annotations: 'testid' });
-      assert.deepEqual(store.getState().selection.selectedAnnotationMap, {
+      assert.deepEqual(store.getSelectedAnnotationMap(), {
         testid: true,
       });
     });
 
     it('expands the selected annotations when settings.annotations is set', function () {
       store = storeFactory(fakeRootScope, { annotations: 'testid' });
-      assert.deepEqual(store.getState().selection.expanded, {
+      assert.deepEqual(store.expandedThreads(), {
         testid: true,
       });
     });
@@ -60,7 +60,7 @@ describe('store', function () {
     // CLEAR_SELECTION action in multiple store modules.
     it('sets `selectedAnnotationMap` to null', () => {
       store.clearSelection();
-      assert.isNull(store.getState().selection.selectedAnnotationMap);
+      assert.isNull(store.getSelectedAnnotationMap());
     });
 
     it('sets `filterQuery` to null', () => {