Commit 1bf6e8f9 authored by Robert Knight's avatar Robert Knight

Compute thread counts for each tab as part of building root thread

Previously the thread counts shown on tabs in the sidebar were computed from the
full set of annotations in the store, not taking into account any filters that
are active. This was acceptable because the tabs were hidden if a filter was
active. However we are planning to change this and thus we need to compute tab
counts that include the effects of active filters.

 - Change `threadAnnotations` to compute thread counts per tab, as well as the
   thread itself. This is done by not filtering top-level threads in
   `buildThread` but instead doing a filtering pass later, which also computes
   the thread count for each tab.

 - Change `SelectionTabs` to take the tab counts as a prop, instead of
   getting the count from the store.
parent 099152fb
...@@ -23,7 +23,7 @@ function AnnotationView({ ...@@ -23,7 +23,7 @@ function AnnotationView({
}: AnnotationViewProps) { }: AnnotationViewProps) {
const store = useSidebarStore(); const store = useSidebarStore();
const annotationId = store.routeParams().id ?? ''; const annotationId = store.routeParams().id ?? '';
const rootThread = useRootThread(); const { rootThread } = useRootThread();
const userid = store.profile().userid; const userid = store.profile().userid;
const [fetchError, setFetchError] = useState(false); const [fetchError, setFetchError] = useState(false);
......
...@@ -32,7 +32,7 @@ function NotebookResultCount({ ...@@ -32,7 +32,7 @@ function NotebookResultCount({
isLoading, isLoading,
resultCount, resultCount,
}: NotebookResultCountProps) { }: NotebookResultCountProps) {
const rootThread = useRootThread(); const { rootThread } = useRootThread();
const visibleCount = isLoading ? resultCount : countVisible(rootThread); const visibleCount = isLoading ? resultCount : countVisible(rootThread);
const hasResults = rootThread.children.length > 0; const hasResults = rootThread.children.length > 0;
......
...@@ -33,7 +33,7 @@ function NotebookView({ loadAnnotationsService, streamer }: NotebookViewProps) { ...@@ -33,7 +33,7 @@ function NotebookView({ loadAnnotationsService, streamer }: NotebookViewProps) {
const isLoading = store.isLoading(); const isLoading = store.isLoading();
const resultCount = store.annotationResultCount(); const resultCount = store.annotationResultCount();
const rootThread = useRootThread(); const { rootThread } = useRootThread();
const groupName = focusedGroup?.name ?? '…'; const groupName = focusedGroup?.name ?? '…';
......
...@@ -86,6 +86,13 @@ export type SelectionTabProps = { ...@@ -86,6 +86,13 @@ export type SelectionTabProps = {
/** Are we waiting on any annotations from the server? */ /** Are we waiting on any annotations from the server? */
isLoading: boolean; isLoading: boolean;
/** Counts of threads in each tab, to be displayed next to the tab title. */
tabCounts: {
annotation: number;
note: number;
orphan: number;
};
// injected // injected
settings: SidebarSettings; settings: SidebarSettings;
annotationsService: AnnotationsService; annotationsService: AnnotationsService;
...@@ -98,12 +105,13 @@ function SelectionTabs({ ...@@ -98,12 +105,13 @@ function SelectionTabs({
annotationsService, annotationsService,
isLoading, isLoading,
settings, settings,
tabCounts,
}: SelectionTabProps) { }: SelectionTabProps) {
const store = useSidebarStore(); const store = useSidebarStore();
const selectedTab = store.selectedTab(); const selectedTab = store.selectedTab();
const noteCount = store.noteCount(); const noteCount = tabCounts.note;
const annotationCount = store.annotationCount(); const annotationCount = tabCounts.annotation;
const orphanCount = store.orphanCount(); const orphanCount = tabCounts.orphan;
const isWaitingToAnchorAnnotations = store.isWaitingToAnchorAnnotations(); const isWaitingToAnchorAnnotations = store.isWaitingToAnchorAnnotations();
const selectTab = (tabId: TabName) => { const selectTab = (tabId: TabName) => {
......
...@@ -35,7 +35,7 @@ function SidebarView({ ...@@ -35,7 +35,7 @@ function SidebarView({
loadAnnotationsService, loadAnnotationsService,
streamer, streamer,
}: SidebarViewProps) { }: SidebarViewProps) {
const rootThread = useRootThread(); const { rootThread, tabCounts } = useRootThread();
// Store state values // Store state values
const store = useSidebarStore(); const store = useSidebarStore();
...@@ -146,7 +146,9 @@ function SidebarView({ ...@@ -146,7 +146,9 @@ function SidebarView({
{hasDirectLinkedGroupError && ( {hasDirectLinkedGroupError && (
<SidebarContentError errorType="group" onLoginRequest={onLogin} /> <SidebarContentError errorType="group" onLoginRequest={onLogin} />
)} )}
{showTabs && <SelectionTabs isLoading={isLoading} />} {showTabs && (
<SelectionTabs isLoading={isLoading} tabCounts={tabCounts} />
)}
<ThreadList threads={rootThread.children} /> <ThreadList threads={rootThread.children} />
{showLoggedOutMessage && <LoggedOutMessage onLogin={onLogin} />} {showLoggedOutMessage && <LoggedOutMessage onLogin={onLogin} />}
</div> </div>
......
...@@ -58,7 +58,7 @@ function StreamView({ api, toastMessenger }: StreamViewProps) { ...@@ -58,7 +58,7 @@ function StreamView({ api, toastMessenger }: StreamViewProps) {
}); });
}, [currentQuery, loadAnnotations, store, toastMessenger]); }, [currentQuery, loadAnnotations, store, toastMessenger]);
const rootThread = useRootThread(); const { rootThread } = useRootThread();
return <ThreadList threads={rootThread.children} />; return <ThreadList threads={rootThread.children} />;
} }
......
...@@ -11,11 +11,15 @@ describe('sidebar/components/hooks/use-root-thread', () => { ...@@ -11,11 +11,15 @@ describe('sidebar/components/hooks/use-root-thread', () => {
fakeStore = { fakeStore = {
allAnnotations: sinon.stub().returns(['1', '2']), allAnnotations: sinon.stub().returns(['1', '2']),
filterQuery: sinon.stub().returns('itchy'), filterQuery: sinon.stub().returns('itchy'),
route: sinon.stub().returns('66'), route: sinon.stub().returns('sidebar'),
selectionState: sinon.stub().returns({ hi: 'there' }), selectionState: sinon.stub().returns({ hi: 'there' }),
getFilterValues: sinon.stub().returns({ user: 'hotspur' }), getFilterValues: sinon.stub().returns({ user: 'hotspur' }),
}; };
fakeThreadAnnotations = sinon.stub().returns('fakeThreadAnnotations'); fakeThreadAnnotations = sinon.stub().returns({
rootThread: {
children: [],
},
});
$imports.$mock({ $imports.$mock({
'../../store': { useSidebarStore: () => fakeStore }, '../../store': { useSidebarStore: () => fakeStore },
...@@ -23,28 +27,38 @@ describe('sidebar/components/hooks/use-root-thread', () => { ...@@ -23,28 +27,38 @@ describe('sidebar/components/hooks/use-root-thread', () => {
threadAnnotations: fakeThreadAnnotations, threadAnnotations: fakeThreadAnnotations,
}, },
}); });
// Mount a dummy component to be able to use the `useRootThread` hook
// Do things that cause `useRootThread` to recalculate in the store and
// test them (hint: use `act`)
function DummyComponent() {
lastRootThread = useRootThread();
}
mount(<DummyComponent />);
}); });
afterEach(() => { afterEach(() => {
$imports.$restore(); $imports.$restore();
}); });
function DummyComponent() {
lastRootThread = useRootThread();
}
it('should return results of `threadAnnotations` with current thread state', () => { it('should return results of `threadAnnotations` with current thread state', () => {
const threadState = fakeThreadAnnotations.getCall(0).args[0]; mount(<DummyComponent />);
const threadState = fakeThreadAnnotations.getCall(0).args[0];
assert.deepEqual(threadState.annotations, ['1', '2']); assert.deepEqual(threadState.annotations, ['1', '2']);
assert.equal(threadState.selection.filterQuery, 'itchy'); assert.equal(threadState.selection.filterQuery, 'itchy');
assert.equal(threadState.route, '66'); assert.equal(threadState.showTabs, true);
assert.equal(threadState.selection.filters.user, 'hotspur'); assert.equal(threadState.selection.filters.user, 'hotspur');
assert.equal(lastRootThread, fakeThreadAnnotations());
});
assert.equal(lastRootThread, 'fakeThreadAnnotations'); [
{ route: 'sidebar', showTabs: true },
{ route: 'notebook', showTabs: false },
].forEach(({ route, showTabs }) => {
it('shows tabs in the sidebar only', () => {
fakeStore.route.returns(route);
mount(<DummyComponent />);
const threadState = fakeThreadAnnotations.getCall(0).args[0];
assert.equal(threadState.showTabs, showTabs);
});
}); });
}); });
import { useMemo } from 'preact/hooks'; import { useMemo } from 'preact/hooks';
import type { Thread } from '../../helpers/build-thread';
import { threadAnnotations } from '../../helpers/thread-annotations'; import { threadAnnotations } from '../../helpers/thread-annotations';
import type { ThreadState } from '../../helpers/thread-annotations'; import type {
ThreadAnnotationsResult,
ThreadState,
} from '../../helpers/thread-annotations';
import { useSidebarStore } from '../../store'; import { useSidebarStore } from '../../store';
/** /**
* Gather together state relevant to building a root thread of annotations and * Gather together state relevant to building a root thread of annotations and
* replies and return an updated root thread when changes occur. * replies and return an updated root thread when changes occur.
*/ */
export function useRootThread(): Thread { export function useRootThread(): ThreadAnnotationsResult {
const store = useSidebarStore(); const store = useSidebarStore();
const annotations = store.allAnnotations(); const annotations = store.allAnnotations();
const query = store.filterQuery(); const query = store.filterQuery();
...@@ -20,8 +22,8 @@ export function useRootThread(): Thread { ...@@ -20,8 +22,8 @@ export function useRootThread(): Thread {
const threadState = useMemo((): ThreadState => { const threadState = useMemo((): ThreadState => {
return { return {
annotations, annotations,
route,
selection: { ...selectionState, filterQuery: query, filters }, selection: { ...selectionState, filterQuery: query, filters },
showTabs: route === 'sidebar',
}; };
}, [annotations, query, route, selectionState, filters]); }, [annotations, query, route, selectionState, filters]);
......
...@@ -136,7 +136,7 @@ function FilterStatusMessage({ ...@@ -136,7 +136,7 @@ function FilterStatusMessage({
*/ */
export default function FilterStatus() { export default function FilterStatus() {
const store = useSidebarStore(); const store = useSidebarStore();
const rootThread = useRootThread(); const { rootThread } = useRootThread();
const annotationCount = store.annotationCount(); const annotationCount = store.annotationCount();
const directLinkedId = store.directLinkedAnnotationId(); const directLinkedId = store.directLinkedAnnotationId();
......
...@@ -36,7 +36,9 @@ describe('FilterStatus', () => { ...@@ -36,7 +36,9 @@ describe('FilterStatus', () => {
toggleFocusMode: sinon.stub(), toggleFocusMode: sinon.stub(),
}; };
fakeUseRootThread = sinon.stub().returns({}); fakeUseRootThread = sinon.stub().returns({
rootThread: { children: [] },
});
$imports.$mock(mockImportedComponents()); $imports.$mock(mockImportedComponents());
$imports.$mock({ $imports.$mock({
...@@ -46,6 +48,10 @@ describe('FilterStatus', () => { ...@@ -46,6 +48,10 @@ describe('FilterStatus', () => {
}); });
}); });
afterEach(() => {
$imports.$restore();
});
function assertFilterText(wrapper, text) { function assertFilterText(wrapper, text) {
const filterText = wrapper.find('[role="status"]').text(); const filterText = wrapper.find('[role="status"]').text();
assert.equal(filterText, text); assert.equal(filterText, text);
...@@ -155,23 +161,25 @@ describe('FilterStatus', () => { ...@@ -155,23 +161,25 @@ describe('FilterStatus', () => {
// the selectedCount from the count of all visible top-level threads // the selectedCount from the count of all visible top-level threads
// (children/replies are ignored in this count) // (children/replies are ignored in this count)
fakeUseRootThread.returns({ fakeUseRootThread.returns({
id: '__default__', rootThread: {
children: [ id: '__default__',
{ id: '1', annotation: { $tag: '1' }, visible: true, children: [] }, children: [
{ { id: '1', annotation: { $tag: '1' }, visible: true, children: [] },
id: '2', {
annotation: { $tag: '2' }, id: '2',
visible: true, annotation: { $tag: '2' },
children: [ visible: true,
{ children: [
id: '2a', {
annotation: { $tag: '2a' }, id: '2a',
visible: true, annotation: { $tag: '2a' },
children: [], visible: true,
}, children: [],
], },
}, ],
], },
],
},
}); });
assertFilterText(createComponent(), 'Showing 1 annotation (and 1 more)'); assertFilterText(createComponent(), 'Showing 1 annotation (and 1 more)');
}); });
......
...@@ -133,7 +133,7 @@ function FilterStatusMessage({ ...@@ -133,7 +133,7 @@ function FilterStatusMessage({
*/ */
export default function FilterAnnotationsStatus() { export default function FilterAnnotationsStatus() {
const store = useSidebarStore(); const store = useSidebarStore();
const rootThread = useRootThread(); const { rootThread } = useRootThread();
const annotationCount = store.annotationCount(); const annotationCount = store.annotationCount();
const directLinkedId = store.directLinkedAnnotationId(); const directLinkedId = store.directLinkedAnnotationId();
......
...@@ -7,7 +7,7 @@ import { useRootThread } from '../hooks/use-root-thread'; ...@@ -7,7 +7,7 @@ import { useRootThread } from '../hooks/use-root-thread';
export default function SearchStatus() { export default function SearchStatus() {
const store = useSidebarStore(); const store = useSidebarStore();
const rootThread = useRootThread(); const { rootThread } = useRootThread();
const filterQuery = store.filterQuery(); const filterQuery = store.filterQuery();
const forcedVisibleCount = store.forcedVisibleThreads().length; const forcedVisibleCount = store.forcedVisibleThreads().length;
......
...@@ -35,7 +35,7 @@ describe('FilterAnnotationsStatus', () => { ...@@ -35,7 +35,7 @@ describe('FilterAnnotationsStatus', () => {
toggleFocusMode: sinon.stub(), toggleFocusMode: sinon.stub(),
}; };
fakeUseRootThread = sinon.stub().returns({}); fakeUseRootThread = sinon.stub().returns({ rootThread: { children: [] } });
$imports.$mock(mockImportedComponents()); $imports.$mock(mockImportedComponents());
$imports.$mock({ $imports.$mock({
...@@ -45,6 +45,10 @@ describe('FilterAnnotationsStatus', () => { ...@@ -45,6 +45,10 @@ describe('FilterAnnotationsStatus', () => {
}); });
}); });
afterEach(() => {
$imports.$restore();
});
function assertFilterText(wrapper, text) { function assertFilterText(wrapper, text) {
const filterText = wrapper.find('[role="status"]').text(); const filterText = wrapper.find('[role="status"]').text();
assert.equal(filterText, text); assert.equal(filterText, text);
...@@ -101,23 +105,25 @@ describe('FilterAnnotationsStatus', () => { ...@@ -101,23 +105,25 @@ describe('FilterAnnotationsStatus', () => {
// the selectedCount from the count of all visible top-level threads // the selectedCount from the count of all visible top-level threads
// (children/replies are ignored in this count) // (children/replies are ignored in this count)
fakeUseRootThread.returns({ fakeUseRootThread.returns({
id: '__default__', rootThread: {
children: [ id: '__default__',
{ id: '1', annotation: { $tag: '1' }, visible: true, children: [] }, children: [
{ { id: '1', annotation: { $tag: '1' }, visible: true, children: [] },
id: '2', {
annotation: { $tag: '2' }, id: '2',
visible: true, annotation: { $tag: '2' },
children: [ visible: true,
{ children: [
id: '2a', {
annotation: { $tag: '2a' }, id: '2a',
visible: true, annotation: { $tag: '2a' },
children: [], visible: true,
}, children: [],
], },
}, ],
], },
],
},
}); });
assertFilterText(createComponent(), 'Showing 1 annotation (and 1 more)'); assertFilterText(createComponent(), 'Showing 1 annotation (and 1 more)');
}); });
......
...@@ -25,7 +25,9 @@ describe('AnnotationView', () => { ...@@ -25,7 +25,9 @@ describe('AnnotationView', () => {
fakeOnLogin = sinon.stub(); fakeOnLogin = sinon.stub();
fakeUseRootThread = sinon.stub().returns({ fakeUseRootThread = sinon.stub().returns({
children: [], rootThread: {
children: [],
},
}); });
$imports.$mock(mockImportedComponents()); $imports.$mock(mockImportedComponents());
......
...@@ -21,7 +21,7 @@ describe('NotebookResultCount', () => { ...@@ -21,7 +21,7 @@ describe('NotebookResultCount', () => {
beforeEach(() => { beforeEach(() => {
fakeCountVisible = sinon.stub().returns(0); fakeCountVisible = sinon.stub().returns(0);
fakeUseRootThread = sinon.stub().returns({ children: [] }); fakeUseRootThread = sinon.stub().returns({ rootThread: { children: [] } });
$imports.$mock({ $imports.$mock({
'./hooks/use-root-thread': { useRootThread: fakeUseRootThread }, './hooks/use-root-thread': { useRootThread: fakeUseRootThread },
...@@ -35,16 +35,12 @@ describe('NotebookResultCount', () => { ...@@ -35,16 +35,12 @@ describe('NotebookResultCount', () => {
context('when there are no results', () => { context('when there are no results', () => {
it('should show "No Results" if no filters are applied', () => { it('should show "No Results" if no filters are applied', () => {
fakeUseRootThread.returns({ children: [] });
const wrapper = createComponent({ isFiltered: false }); const wrapper = createComponent({ isFiltered: false });
assert.equal(wrapper.text(), 'No results'); assert.equal(wrapper.text(), 'No results');
}); });
it('should show "No Results" if filters are applied', () => { it('should show "No Results" if filters are applied', () => {
fakeUseRootThread.returns({ children: [] });
const wrapper = createComponent({ isFiltered: true }); const wrapper = createComponent({ isFiltered: true });
assert.equal(wrapper.text(), 'No results'); assert.equal(wrapper.text(), 'No results');
...@@ -71,7 +67,7 @@ describe('NotebookResultCount', () => { ...@@ -71,7 +67,7 @@ describe('NotebookResultCount', () => {
].forEach(test => { ].forEach(test => {
it('should render a count of threads and annotations', () => { it('should render a count of threads and annotations', () => {
fakeCountVisible.returns(test.visibleCount); fakeCountVisible.returns(test.visibleCount);
fakeUseRootThread.returns(test.thread); fakeUseRootThread.returns({ rootThread: test.thread });
const wrapper = createComponent(); const wrapper = createComponent();
...@@ -102,7 +98,7 @@ describe('NotebookResultCount', () => { ...@@ -102,7 +98,7 @@ describe('NotebookResultCount', () => {
}, },
].forEach(test => { ].forEach(test => {
it('should render a count of results', () => { it('should render a count of results', () => {
fakeUseRootThread.returns(test.thread); fakeUseRootThread.returns({ rootThread: test.thread });
fakeCountVisible.returns(test.visibleCount); fakeCountVisible.returns(test.visibleCount);
const wrapper = createComponent({ const wrapper = createComponent({
...@@ -122,7 +118,7 @@ describe('NotebookResultCount', () => { ...@@ -122,7 +118,7 @@ describe('NotebookResultCount', () => {
}); });
it('shows annotation count if there are any matching annotations being fetched', () => { it('shows annotation count if there are any matching annotations being fetched', () => {
fakeUseRootThread.returns({ children: [1, 2] }); fakeUseRootThread.returns({ rootThread: { children: [1, 2] } });
// Setting countVisible to something different to demonstrate that // Setting countVisible to something different to demonstrate that
// resultCount is used while loading // resultCount is used while loading
fakeCountVisible.returns(5); fakeCountVisible.returns(5);
...@@ -144,7 +140,7 @@ describe('NotebookResultCount', () => { ...@@ -144,7 +140,7 @@ describe('NotebookResultCount', () => {
name: 'with results', name: 'with results',
content: () => { content: () => {
fakeCountVisible.returns(2); fakeCountVisible.returns(2);
fakeUseRootThread.returns({ children: [1, 2] }); fakeUseRootThread.returns({ rootThread: { children: [1, 2] } });
return createComponent(); return createComponent();
}, },
}, },
...@@ -152,7 +148,7 @@ describe('NotebookResultCount', () => { ...@@ -152,7 +148,7 @@ describe('NotebookResultCount', () => {
name: 'with results and filters applied', name: 'with results and filters applied',
content: () => { content: () => {
fakeCountVisible.returns(3); fakeCountVisible.returns(3);
fakeUseRootThread.returns({ children: [1] }); fakeUseRootThread.returns({ rootThread: { children: [1] } });
return createComponent({ forcedVisibleCount: 1, isFiltered: true }); return createComponent({ forcedVisibleCount: 1, isFiltered: true });
}, },
}, },
......
...@@ -20,7 +20,9 @@ describe('NotebookView', () => { ...@@ -20,7 +20,9 @@ describe('NotebookView', () => {
load: sinon.stub(), load: sinon.stub(),
}; };
fakeUseRootThread = sinon.stub().returns({}); fakeUseRootThread = sinon.stub().returns({
rootThread: { children: [] },
});
fakeScrollIntoView = sinon.stub(); fakeScrollIntoView = sinon.stub();
......
...@@ -12,9 +12,13 @@ describe('SelectionTabs', () => { ...@@ -12,9 +12,13 @@ describe('SelectionTabs', () => {
let fakeSettings; let fakeSettings;
let fakeStore; let fakeStore;
// default props
const defaultProps = { const defaultProps = {
isLoading: false, isLoading: false,
tabCounts: {
annotation: 123,
note: 456,
orphan: 0,
},
}; };
function createComponent(props) { function createComponent(props) {
...@@ -38,9 +42,6 @@ describe('SelectionTabs', () => { ...@@ -38,9 +42,6 @@ describe('SelectionTabs', () => {
fakeStore = { fakeStore = {
clearSelection: sinon.stub(), clearSelection: sinon.stub(),
selectTab: sinon.stub(), selectTab: sinon.stub(),
annotationCount: sinon.stub().returns(123),
noteCount: sinon.stub().returns(456),
orphanCount: sinon.stub().returns(0),
isWaitingToAnchorAnnotations: sinon.stub().returns(false), isWaitingToAnchorAnnotations: sinon.stub().returns(false),
selectedTab: sinon.stub().returns('annotation'), selectedTab: sinon.stub().returns('annotation'),
}; };
...@@ -155,9 +156,12 @@ describe('SelectionTabs', () => { ...@@ -155,9 +156,12 @@ describe('SelectionTabs', () => {
describe('orphans tab', () => { describe('orphans tab', () => {
it('should display orphans tab if there is 1 or more orphans', () => { it('should display orphans tab if there is 1 or more orphans', () => {
fakeStore.orphanCount.returns(1); const wrapper = createComponent({
tabCounts: {
const wrapper = createComponent(); ...defaultProps.tabCounts,
orphan: 1,
},
});
const orphanTab = wrapper.find('Tab[label="Orphans"]'); const orphanTab = wrapper.find('Tab[label="Orphans"]');
assert.isTrue(orphanTab.exists()); assert.isTrue(orphanTab.exists());
...@@ -165,17 +169,14 @@ describe('SelectionTabs', () => { ...@@ -165,17 +169,14 @@ describe('SelectionTabs', () => {
it('should display orphans tab as selected when it is active', () => { it('should display orphans tab as selected when it is active', () => {
fakeStore.selectedTab.returns('orphan'); fakeStore.selectedTab.returns('orphan');
fakeStore.orphanCount.returns(1);
const wrapper = createComponent(); const wrapper = createComponent({ tabCounts: { orphan: 1 } });
const orphanTab = wrapper.find('Tab[label="Orphans"]'); const orphanTab = wrapper.find('Tab[label="Orphans"]');
assert.isTrue(orphanTab.find('LinkButton').prop('pressed')); assert.isTrue(orphanTab.find('LinkButton').prop('pressed'));
}); });
it('should not display orphans tab if there are 0 orphans', () => { it('should not display orphans tab if there are 0 orphans', () => {
fakeStore.orphanCount.returns(0);
const wrapper = createComponent(); const wrapper = createComponent();
const orphanTab = wrapper.find('Tab[label="Orphans"]'); const orphanTab = wrapper.find('Tab[label="Orphans"]');
...@@ -186,9 +187,12 @@ describe('SelectionTabs', () => { ...@@ -186,9 +187,12 @@ describe('SelectionTabs', () => {
describe('tab display and counts', () => { describe('tab display and counts', () => {
it('should not render count if there are no page notes', () => { it('should not render count if there are no page notes', () => {
fakeStore.noteCount.returns(0); const wrapper = createComponent({
tabCounts: {
const wrapper = createComponent({}); ...defaultProps.tabCounts,
note: 0,
},
});
const noteTab = wrapper.find('Tab[label="Page notes"]'); const noteTab = wrapper.find('Tab[label="Page notes"]');
...@@ -196,9 +200,12 @@ describe('SelectionTabs', () => { ...@@ -196,9 +200,12 @@ describe('SelectionTabs', () => {
}); });
it('should not display a message when its loading annotation count is 0', () => { it('should not display a message when its loading annotation count is 0', () => {
fakeStore.annotationCount.returns(0);
const wrapper = createComponent({ const wrapper = createComponent({
isLoading: true, isLoading: true,
tabCounts: {
...defaultProps.tabCounts,
annotation: 0,
},
}); });
assert.isFalse( assert.isFalse(
wrapper.exists('[data-testid="annotations-unavailable-message"]'), wrapper.exists('[data-testid="annotations-unavailable-message"]'),
...@@ -207,9 +214,12 @@ describe('SelectionTabs', () => { ...@@ -207,9 +214,12 @@ describe('SelectionTabs', () => {
it('should not display a message when its loading notes count is 0', () => { it('should not display a message when its loading notes count is 0', () => {
fakeStore.selectedTab.returns('note'); fakeStore.selectedTab.returns('note');
fakeStore.noteCount.returns(0);
const wrapper = createComponent({ const wrapper = createComponent({
isLoading: true, isLoading: true,
tabCounts: {
...defaultProps.tabCounts,
note: 0,
},
}); });
assert.isFalse( assert.isFalse(
wrapper.exists('[data-testid="notes-unavailable-message"]'), wrapper.exists('[data-testid="notes-unavailable-message"]'),
...@@ -217,10 +227,13 @@ describe('SelectionTabs', () => { ...@@ -217,10 +227,13 @@ describe('SelectionTabs', () => {
}); });
it('should not display the longer version of the no annotations message when there are no annotations and isWaitingToAnchorAnnotations is true', () => { it('should not display the longer version of the no annotations message when there are no annotations and isWaitingToAnchorAnnotations is true', () => {
fakeStore.annotationCount.returns(0);
fakeStore.isWaitingToAnchorAnnotations.returns(true); fakeStore.isWaitingToAnchorAnnotations.returns(true);
const wrapper = createComponent({ const wrapper = createComponent({
isLoading: false, isLoading: false,
tabCounts: {
...defaultProps.tabCounts,
annotationn: 0,
},
}); });
assert.isFalse( assert.isFalse(
wrapper.exists('[data-testid="annotations-unavailable-message"]'), wrapper.exists('[data-testid="annotations-unavailable-message"]'),
...@@ -229,8 +242,12 @@ describe('SelectionTabs', () => { ...@@ -229,8 +242,12 @@ describe('SelectionTabs', () => {
it('should display the longer version of the no notes message when there are no notes', () => { it('should display the longer version of the no notes message when there are no notes', () => {
fakeStore.selectedTab.returns('note'); fakeStore.selectedTab.returns('note');
fakeStore.noteCount.returns(0); const wrapper = createComponent({
const wrapper = createComponent({}); tabCounts: {
...defaultProps.tabCounts,
note: 0,
},
});
assert.include( assert.include(
wrapper.find('Card[data-testid="notes-unavailable-message"]').text(), wrapper.find('Card[data-testid="notes-unavailable-message"]').text(),
...@@ -239,8 +256,12 @@ describe('SelectionTabs', () => { ...@@ -239,8 +256,12 @@ describe('SelectionTabs', () => {
}); });
it('should display the longer version of the no annotations message when there are no annotations', () => { it('should display the longer version of the no annotations message when there are no annotations', () => {
fakeStore.annotationCount.returns(0); const wrapper = createComponent({
const wrapper = createComponent({}); tabCounts: {
...defaultProps.tabCounts,
annotation: 0,
},
});
assert.include( assert.include(
wrapper wrapper
.find('Card[data-testid="annotations-unavailable-message"]') .find('Card[data-testid="annotations-unavailable-message"]')
...@@ -264,9 +285,13 @@ describe('SelectionTabs', () => { ...@@ -264,9 +285,13 @@ describe('SelectionTabs', () => {
// Pre-select a different tab than the one we are about to click. // Pre-select a different tab than the one we are about to click.
fakeStore.selectedTab.returns('other-tab'); fakeStore.selectedTab.returns('other-tab');
// Make the "Orphans" tab appear. const wrapper = createComponent({
fakeStore.orphanCount.returns(1); // Make the "Orphans" tab appear.
const wrapper = createComponent({}); tabCounts: {
...defaultProps.tabCounts,
orphan: 1,
},
});
findButton(wrapper, label).simulate('click'); findButton(wrapper, label).simulate('click');
...@@ -289,10 +314,13 @@ describe('SelectionTabs', () => { ...@@ -289,10 +314,13 @@ describe('SelectionTabs', () => {
'should pass a11y checks', 'should pass a11y checks',
checkAccessibility({ checkAccessibility({
content: () => { content: () => {
fakeStore.annotationCount.returns(1); return createComponent({
fakeStore.noteCount.returns(2); tabCounts: {
fakeStore.orphanCount.returns(3); annotation: 1,
return createComponent({}); note: 2,
orphan: 3,
},
});
}, },
}), }),
); );
......
...@@ -35,7 +35,14 @@ describe('SidebarView', () => { ...@@ -35,7 +35,14 @@ describe('SidebarView', () => {
load: sinon.stub(), load: sinon.stub(),
}; };
fakeUseRootThread = sinon.stub().returns({ fakeUseRootThread = sinon.stub().returns({
children: [], rootThread: {
children: [],
},
tabCounts: {
annotation: 1,
note: 2,
orphan: 0,
},
}); });
fakeStreamer = { fakeStreamer = {
connect: sinon.stub(), connect: sinon.stub(),
......
...@@ -16,7 +16,9 @@ describe('StreamView', () => { ...@@ -16,7 +16,9 @@ describe('StreamView', () => {
}; };
fakeUseRootThread = sinon.stub().returns({ fakeUseRootThread = sinon.stub().returns({
children: [], rootThread: {
children: [],
},
}); });
fakeQueryParser = { fakeQueryParser = {
......
...@@ -24,7 +24,7 @@ describe('sidebar/helpers/thread-annotations', () => { ...@@ -24,7 +24,7 @@ describe('sidebar/helpers/thread-annotations', () => {
beforeEach(() => { beforeEach(() => {
fakeThreadState = { fakeThreadState = {
annotations: [], annotations: [],
route: 'sidebar', showTabs: true,
selection: { selection: {
expanded: {}, expanded: {},
forcedVisible: [], forcedVisible: [],
...@@ -36,7 +36,9 @@ describe('sidebar/helpers/thread-annotations', () => { ...@@ -36,7 +36,9 @@ describe('sidebar/helpers/thread-annotations', () => {
}, },
}; };
fakeBuildThread = sinon.stub().returns(fixtures.emptyThread); fakeBuildThread = sinon
.stub()
.callsFake(() => structuredClone(fixtures.emptyThread));
fakeFilterAnnotations = sinon.stub(); fakeFilterAnnotations = sinon.stub();
fakeQueryParser = { fakeQueryParser = {
parseFilterQuery: sinon.stub(), parseFilterQuery: sinon.stub(),
...@@ -55,12 +57,15 @@ describe('sidebar/helpers/thread-annotations', () => { ...@@ -55,12 +57,15 @@ describe('sidebar/helpers/thread-annotations', () => {
describe('threadAnnotations', () => { describe('threadAnnotations', () => {
it('returns the result of buildThread', () => { it('returns the result of buildThread', () => {
assert.equal(threadAnnotations(fakeThreadState), fixtures.emptyThread); assert.deepEqual(
threadAnnotations(fakeThreadState).rootThread,
fixtures.emptyThread,
);
}); });
it('memoizes on `threadState`', () => { it('memoizes on `threadState`', () => {
fakeBuildThread.onCall(0).returns({ brisket: 'fingers' }); fakeBuildThread.onCall(0).returns({ children: [] });
fakeBuildThread.onCall(1).returns({ brisket: 'bananas' }); fakeBuildThread.onCall(1).returns({ children: [] });
const thread1 = threadAnnotations(fakeThreadState); const thread1 = threadAnnotations(fakeThreadState);
const thread2 = threadAnnotations(fakeThreadState); const thread2 = threadAnnotations(fakeThreadState);
...@@ -113,39 +118,115 @@ describe('sidebar/helpers/thread-annotations', () => { ...@@ -113,39 +118,115 @@ describe('sidebar/helpers/thread-annotations', () => {
}); });
describe('annotation and thread filtering', () => { describe('annotation and thread filtering', () => {
context('sidebar route', () => { context('when `showTabs` is true', () => {
function annotationForTab(tab) {
switch (tab) {
case 'annotation':
return {
...annotationFixtures.defaultAnnotation(),
$orphan: false,
};
case 'note':
return annotationFixtures.oldPageNote();
case 'orphan':
return {
...annotationFixtures.defaultAnnotation(),
$orphan: true,
};
default:
throw new Error('Invalid tab');
}
}
[
// Tabs enabled, annotations in each tab.
{
annotations: [
annotationForTab('annotation'),
annotationForTab('annotation'),
annotationForTab('annotation'),
annotationForTab('note'),
annotationForTab('note'),
annotationForTab('orphan'),
],
showTabs: true,
expectedCounts: {
annotation: 3,
note: 2,
orphan: 1,
},
},
// Tabs enabled, no annotations
{
annotations: [],
showTabs: true,
expectedCounts: {
annotation: 0,
note: 0,
orphan: 0,
},
},
// Tabs disabled
{
annotations: [
annotationForTab('annotation'),
annotationForTab('note'),
annotationForTab('orphan'),
],
showTabs: false,
expectedCounts: {
annotation: 0,
note: 0,
orphan: 0,
},
},
].forEach(({ annotations, showTabs, expectedCounts }) => {
it('returns thread count for each tab', () => {
fakeThreadState.annotations = annotations;
fakeBuildThread.returns({
children: fakeThreadState.annotations.map(annotation => ({
annotation,
})),
});
fakeThreadState.showTabs = showTabs;
fakeThreadState.selection.selectedTab = 'annotation';
const { tabCounts } = threadAnnotations(fakeThreadState);
assert.deepEqual(tabCounts, expectedCounts);
});
});
['note', 'annotation', 'orphan'].forEach(selectedTab => { ['note', 'annotation', 'orphan'].forEach(selectedTab => {
it(`should filter the thread for the tab '${selectedTab}'`, () => { it(`should filter the thread for the tab '${selectedTab}'`, () => {
const annotations = { fakeThreadState.annotations = [
['annotation']: { {
...annotationFixtures.defaultAnnotation(), ...annotationFixtures.defaultAnnotation(),
$orphan: false, $orphan: false,
}, },
['note']: annotationFixtures.oldPageNote(), annotationFixtures.oldPageNote(),
['orphan']: { {
...annotationFixtures.defaultAnnotation(), ...annotationFixtures.defaultAnnotation(),
$orphan: true, $orphan: true,
}, },
};
const fakeThreads = [
{},
{ annotation: annotations.annotation },
{ annotation: annotations.note },
{ annotation: annotations.orphan },
]; ];
fakeBuildThread.returns({
children: fakeThreadState.annotations.map(annotation => ({
annotation,
})),
});
fakeThreadState.showTabs = true;
fakeThreadState.selection.selectedTab = selectedTab; fakeThreadState.selection.selectedTab = selectedTab;
threadAnnotations(fakeThreadState); const { rootThread } = threadAnnotations(fakeThreadState);
const filteredThreads = rootThread.children;
const threadFilterFn = fakeBuildThread.args[0][1].threadFilterFn;
const filteredThreads = fakeThreads.filter(thread =>
threadFilterFn(thread),
);
assert.lengthOf(filteredThreads, 1); assert.lengthOf(filteredThreads, 1);
assert.equal( assert.equal(
filteredThreads[0].annotation, fakeThreadState.annotations.indexOf(
annotations[selectedTab], filteredThreads[0].annotation,
),
['annotation', 'note', 'orphan'].indexOf(selectedTab),
); );
}); });
}); });
......
import type { Annotation } from '../../types/api'; import type { Annotation } from '../../types/api';
import { memoize } from '../util/memoize'; import { memoize } from '../util/memoize';
import { isWaitingToAnchor } from './annotation-metadata';
import { buildThread } from './build-thread'; import { buildThread } from './build-thread';
import type { Thread, BuildThreadOptions } from './build-thread'; import type { Thread, BuildThreadOptions } from './build-thread';
import { filterAnnotations } from './filter-annotations'; import { filterAnnotations } from './filter-annotations';
import { parseFilterQuery } from './query-parser'; import { parseFilterQuery } from './query-parser';
import type { FilterField } from './query-parser'; import type { FilterField } from './query-parser';
import { shouldShowInTab } from './tabs'; import { tabForAnnotation } from './tabs';
import { sorters } from './thread-sorters'; import { sorters } from './thread-sorters';
export type ThreadState = { export type ThreadState = {
annotations: Annotation[]; annotations: Annotation[];
route: string | null; showTabs: boolean;
selection: { selection: {
expanded: Record<string, boolean>; expanded: Record<string, boolean>;
filterQuery: string | null; filterQuery: string | null;
...@@ -22,11 +23,32 @@ export type ThreadState = { ...@@ -22,11 +23,32 @@ export type ThreadState = {
}; };
}; };
export type ThreadAnnotationsResult = {
/**
* Count of annotations for each tab.
*
* These are only computed if {@link ThreadState.showTabs} is true.
*/
tabCounts: {
annotation: number;
note: number;
orphan: number;
};
/**
* Root thread containing all annotation threads that match the current
* filters and selected tab.
*/
rootThread: Thread;
};
/** /**
* Cobble together the right set of options and filters based on current * Cobble together the right set of options and filters based on current
* `threadState` to build the root thread. * `threadState` to build the root thread.
*/ */
function buildRootThread(threadState: ThreadState): Thread { function threadAnnotationsImpl(
threadState: ThreadState,
): ThreadAnnotationsResult {
const selection = threadState.selection; const selection = threadState.selection;
const options: BuildThreadOptions = { const options: BuildThreadOptions = {
expanded: selection.expanded, expanded: selection.expanded,
...@@ -48,21 +70,30 @@ function buildRootThread(threadState: ThreadState): Thread { ...@@ -48,21 +70,30 @@ function buildRootThread(threadState: ThreadState): Thread {
options.filterFn = ann => filterAnnotations([ann], filters).length > 0; options.filterFn = ann => filterAnnotations([ann], filters).length > 0;
} }
// If annotations aren't filtered, should we filter out tab-irrelevant const rootThread = buildThread(threadState.annotations, options);
// annotations (e.g. we should only show notes in the `Notes` tab)
// in the sidebar?
const threadFiltered =
!annotationsFiltered && threadState.route === 'sidebar';
if (threadFiltered) { const tabCounts = {
options.threadFilterFn = thread => { annotation: 0,
if (!thread.annotation) { note: 0,
orphan: 0,
};
if (threadState.showTabs) {
rootThread.children = rootThread.children.filter(thread => {
if (thread.annotation && isWaitingToAnchor(thread.annotation)) {
// Until this annotation anchors or fails to anchor, we do not know which
// tab it should be displayed in.
return false; return false;
} }
return shouldShowInTab(thread.annotation, selection.selectedTab); const tab = thread.annotation
}; ? tabForAnnotation(thread.annotation)
: 'annotation';
tabCounts[tab] += 1;
return tab === selection.selectedTab;
});
} }
return buildThread(threadState.annotations, options);
return { tabCounts, rootThread };
} }
export const threadAnnotations = memoize(buildRootThread); export const threadAnnotations = memoize(threadAnnotationsImpl);
...@@ -62,7 +62,7 @@ describe('integration: annotation threading', () => { ...@@ -62,7 +62,7 @@ describe('integration: annotation threading', () => {
// Do things that cause `useRootThread` to recalculate in the store and // Do things that cause `useRootThread` to recalculate in the store and
// test them (hint: use `act`) // test them (hint: use `act`)
function DummyComponent() { function DummyComponent() {
lastRootThread = useRootThread(); lastRootThread = useRootThread().rootThread;
[, forceUpdate] = useReducer(x => x + 1, 0); [, forceUpdate] = useReducer(x => x + 1, 0);
} }
......
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