Unverified Commit 86dabe49 authored by Lyza Gardner's avatar Lyza Gardner Committed by GitHub

Merge pull request #1841 from hypothesis/fix-page-note-tab-selection

Fix page note initial tab selection when there are replies in the mix
parents e1d17fe9 67928814
...@@ -19,8 +19,7 @@ import immutable from 'seamless-immutable'; ...@@ -19,8 +19,7 @@ import immutable from 'seamless-immutable';
import uiConstants from '../../ui-constants'; import uiConstants from '../../ui-constants';
import * as metadata from '../../util/annotation-metadata'; import * as metadata from '../../util/annotation-metadata';
import * as arrayUtil from '../../util/array'; import { countIf, toSet } from '../../util/array';
import { toSet } from '../../util/array';
import * as util from '../util'; import * as util from '../util';
/** /**
...@@ -204,14 +203,18 @@ const update = { ...@@ -204,14 +203,18 @@ const update = {
return setTab(state.selectedTab, action.tab); return setTab(state.selectedTab, action.tab);
}, },
/**
* Automatically select the Page Notes tab, for convenience, if all of the
* top-level annotations in `action.annotations` are Page Notes and the
* previous annotation count was 0 (i.e. collection empty).
*/
ADD_ANNOTATIONS(state, action) { ADD_ANNOTATIONS(state, action) {
const noteCount = arrayUtil.countIf( const topLevelAnnotations = action.annotations.filter(
action.annotations, annotation => !metadata.isReply(annotation)
metadata.isPageNote
); );
// If there are no annotations at all, ADD_ANNOTATIONS will not be called. const noteCount = countIf(action.annotations, metadata.isPageNote);
const haveOnlyPageNotes = noteCount === action.annotations.length;
// If this is the init phase and there are only page notes, select the page notes tab. const haveOnlyPageNotes = noteCount === topLevelAnnotations.length;
if (action.currentAnnotationCount === 0 && haveOnlyPageNotes) { if (action.currentAnnotationCount === 0 && haveOnlyPageNotes) {
return { selectedTab: uiConstants.TAB_NOTES }; return { selectedTab: uiConstants.TAB_NOTES };
} }
...@@ -228,7 +231,7 @@ const update = { ...@@ -228,7 +231,7 @@ const update = {
let selectedTab = state.selectedTab; let selectedTab = state.selectedTab;
if ( if (
selectedTab === uiConstants.TAB_ORPHANS && selectedTab === uiConstants.TAB_ORPHANS &&
arrayUtil.countIf(action.remainingAnnotations, metadata.isOrphan) === 0 countIf(action.remainingAnnotations, metadata.isOrphan) === 0
) { ) {
selectedTab = uiConstants.TAB_ANNOTATIONS; selectedTab = uiConstants.TAB_ANNOTATIONS;
} }
......
...@@ -2,6 +2,7 @@ import uiConstants from '../../../ui-constants'; ...@@ -2,6 +2,7 @@ import uiConstants from '../../../ui-constants';
import createStore from '../../create-store'; import createStore from '../../create-store';
import annotations from '../annotations'; import annotations from '../annotations';
import selection from '../selection'; import selection from '../selection';
import * as fixtures from '../../../test/annotation-fixtures';
describe('sidebar/store/modules/selection', () => { describe('sidebar/store/modules/selection', () => {
let store; let store;
...@@ -423,4 +424,59 @@ describe('sidebar/store/modules/selection', () => { ...@@ -423,4 +424,59 @@ describe('sidebar/store/modules/selection', () => {
assert.equal(getSelectionState().sortKey, 'Newest'); assert.equal(getSelectionState().sortKey, 'Newest');
}); });
}); });
describe('ADD_ANNOTATIONS', () => {
it('should select the page notes tab if all top-level annotations are page notes', () => {
store.dispatch({
type: 'ADD_ANNOTATIONS',
annotations: [fixtures.oldPageNote(), fixtures.oldPageNote()],
currentAnnotationCount: 0,
});
assert.equal(getSelectionState().selectedTab, uiConstants.TAB_NOTES);
});
it('should select the page notes tab if page notes have replies', () => {
const pageNote = fixtures.oldPageNote();
const reply = fixtures.newReply();
reply.references = [pageNote.id];
store.dispatch({
type: 'ADD_ANNOTATIONS',
annotations: [pageNote, reply],
currentAnnotationCount: 0,
});
assert.equal(getSelectionState().selectedTab, uiConstants.TAB_NOTES);
});
it('should not select the page notes tab if there were previously annotations in the store', () => {
store.dispatch({
type: 'ADD_ANNOTATIONS',
annotations: [fixtures.oldPageNote(), fixtures.oldPageNote()],
currentAnnotationCount: 4,
});
assert.equal(
getSelectionState().selectedTab,
uiConstants.TAB_ANNOTATIONS
);
});
it('should not select the page notes tab if there are non-page-note annotations at the top level', () => {
store.dispatch({
type: 'ADD_ANNOTATIONS',
annotations: [
fixtures.oldPageNote(),
fixtures.oldPageNote(),
fixtures.oldHighlight(),
],
currentAnnotationCount: 0,
});
assert.equal(
getSelectionState().selectedTab,
uiConstants.TAB_ANNOTATIONS
);
});
});
}); });
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment