Unverified Commit 877597b6 authored by Lyza Gardner's avatar Lyza Gardner Committed by GitHub

Merge pull request #1336 from hypothesis/annotation-counts

Add annotation counts and iterate on UX for user-focused mode
parents 20f10cf6 39b5a6f4
...@@ -32,11 +32,12 @@ function FocusedModeHeader() { ...@@ -32,11 +32,12 @@ function FocusedModeHeader() {
<div className="focused-mode-header__filter-status"> <div className="focused-mode-header__filter-status">
{selectors.focusModeFocused ? ( {selectors.focusModeFocused ? (
<span> <span>
Annotations by <strong>{selectors.focusModeUserPrettyName}</strong>{' '} Showing <strong>{selectors.focusModeUserPrettyName}</strong> only
only
</span> </span>
) : ( ) : (
<span>Everybody&rsquo;s annotations</span> <span>
Showing <strong>all</strong>
</span>
)} )}
</div> </div>
); );
...@@ -50,7 +51,7 @@ function FocusedModeHeader() { ...@@ -50,7 +51,7 @@ function FocusedModeHeader() {
})(); })();
return ( return (
<div className="focused-mode-header sheet"> <div className="focused-mode-header sheet sheet--short">
{filterStatus} {filterStatus}
<button onClick={toggleFocusedMode} className="focused-mode-header__btn"> <button onClick={toggleFocusedMode} className="focused-mode-header__btn">
{buttonText} {buttonText}
......
...@@ -8,6 +8,12 @@ const { withServices } = require('../util/service-context'); ...@@ -8,6 +8,12 @@ const { withServices } = require('../util/service-context');
const uiConstants = require('../ui-constants'); const uiConstants = require('../ui-constants');
const useStore = require('../store/use-store'); const useStore = require('../store/use-store');
/**
* Of the annotations in the thread `annThread`, how many
* are currently `visible` in the browser (sidebar)?
*
* TODO: This function should be a selector or a reusable util
*/
const countVisibleAnns = annThread => { const countVisibleAnns = annThread => {
return annThread.children.reduce( return annThread.children.reduce(
function(count, child) { function(count, child) {
...@@ -22,22 +28,23 @@ const countVisibleAnns = annThread => { ...@@ -22,22 +28,23 @@ const countVisibleAnns = annThread => {
* any search results were found. * any search results were found.
* */ * */
function SearchStatusBar({ rootThread }) { function SearchStatusBar({ rootThread }) {
const { const actions = useStore(store => ({
directLinkedGroupFetchFailed, clearSelection: store.clearSelection,
filterQuery, }));
selectedAnnotationMap,
selectedTab, const storeState = useStore(store => ({
} = useStore(store => ({ annotationCount: store.annotationCount(),
directLinkedGroupFetchFailed: store.getRootState().directLinked directLinkedGroupFetchFailed: store.getRootState().directLinked
.directLinkedGroupFetchFailed, .directLinkedGroupFetchFailed,
filterQuery: store.getRootState().selection.filterQuery, filterQuery: store.getRootState().selection.filterQuery,
focusModeFocused: store.focusModeFocused(),
focusModeUserPrettyName: store.focusModeUserPrettyName(),
noteCount: store.noteCount(),
selectedAnnotationMap: store.getRootState().selection.selectedAnnotationMap, selectedAnnotationMap: store.getRootState().selection.selectedAnnotationMap,
selectedTab: store.getRootState().selection.selectedTab, selectedTab: store.getRootState().selection.selectedTab,
})); }));
const clearSelection = useStore(store => store.clearSelection);
const filterActive = !!filterQuery; const filterActive = !!storeState.filterQuery;
const annotationCount = useStore(store => store.annotationCount());
const noteCount = useStore(store => store.noteCount());
const thread = useStore(store => rootThread.thread(store.getRootState())); const thread = useStore(store => rootThread.thread(store.getRootState()));
...@@ -45,23 +52,33 @@ function SearchStatusBar({ rootThread }) { ...@@ -45,23 +52,33 @@ function SearchStatusBar({ rootThread }) {
return countVisibleAnns(thread); return countVisibleAnns(thread);
}, [thread]); }, [thread]);
const filterResults = () => { const filterResults = (() => {
const resultsCount = visibleCount; switch (visibleCount) {
switch (resultsCount) {
case 0: case 0:
return 'No results for "' + filterQuery + '"'; return `No results for "${storeState.filterQuery}"`;
case 1: case 1:
return '1 search result'; return '1 search result';
default: default:
return resultsCount + ' search results'; return `${visibleCount} search results`;
} }
}; })();
const focusResults = (() => {
switch (visibleCount) {
case 0:
return `No annotations for ${storeState.focusModeUserPrettyName}`;
case 1:
return 'Showing 1 annotation';
default:
return `Showing ${visibleCount} annotations`;
}
})();
const areNotAllAnnotationsVisible = () => { const areNotAllAnnotationsVisible = () => {
if (directLinkedGroupFetchFailed) { if (storeState.directLinkedGroupFetchFailed) {
return true; return true;
} }
const selection = selectedAnnotationMap; const selection = storeState.selectedAnnotationMap;
if (!selection) { if (!selection) {
return false; return false;
} }
...@@ -74,35 +91,44 @@ function SearchStatusBar({ rootThread }) { ...@@ -74,35 +91,44 @@ function SearchStatusBar({ rootThread }) {
<div className="search-status-bar"> <div className="search-status-bar">
<button <button
className="primary-action-btn primary-action-btn--short" className="primary-action-btn primary-action-btn--short"
onClick={clearSelection} onClick={actions.clearSelection}
title="Clear the search filter and show all annotations" title="Clear the search filter and show all annotations"
> >
<i className="primary-action-btn__icon h-icon-close" /> <i className="primary-action-btn__icon h-icon-close" />
Clear search Clear search
</button> </button>
<span>{filterResults()}</span> <span>{filterResults}</span>
</div>
)}
{!filterActive && storeState.focusModeFocused && (
<div className="search-status-bar">
<strong>{focusResults}</strong>
</div> </div>
)} )}
{!filterActive && areNotAllAnnotationsVisible() && ( {!filterActive && areNotAllAnnotationsVisible() && (
<div className="search-status-bar"> <div className="search-status-bar">
<button <button
className="primary-action-btn primary-action-btn--short" className="primary-action-btn primary-action-btn--short"
onClick={clearSelection} onClick={actions.clearSelection}
title="Clear the selection and show all annotations" title="Clear the selection and show all annotations"
> >
{selectedTab === uiConstants.TAB_ORPHANS && ( {storeState.selectedTab === uiConstants.TAB_ORPHANS && (
<Fragment>Show all annotations and notes</Fragment> <Fragment>Show all annotations and notes</Fragment>
)} )}
{selectedTab === uiConstants.TAB_ANNOTATIONS && ( {storeState.selectedTab === uiConstants.TAB_ANNOTATIONS && (
<Fragment> <Fragment>
Show all annotations Show all annotations
{annotationCount > 1 && <span> ({annotationCount})</span>} {storeState.annotationCount > 1 && (
<span> ({storeState.annotationCount})</span>
)}
</Fragment> </Fragment>
)} )}
{selectedTab === uiConstants.TAB_NOTES && ( {storeState.selectedTab === uiConstants.TAB_NOTES && (
<Fragment> <Fragment>
Show all notes Show all notes
{noteCount > 1 && <span> ({noteCount})</span>} {storeState.noteCount > 1 && (
<span> ({storeState.noteCount})</span>
)}
</Fragment> </Fragment>
)} )}
</button> </button>
......
...@@ -48,7 +48,7 @@ describe('FocusedModeHeader', function() { ...@@ -48,7 +48,7 @@ describe('FocusedModeHeader', function() {
it("should render status text indicating only that user's annotations are visible", () => { it("should render status text indicating only that user's annotations are visible", () => {
const wrapper = createComponent(); const wrapper = createComponent();
assert.match(wrapper.text(), /Annotations by.+Fake User/); assert.match(wrapper.text(), /Showing.+Fake User.+only/);
}); });
it('should render a button allowing the user to view all annotations', () => { it('should render a button allowing the user to view all annotations', () => {
...@@ -68,7 +68,7 @@ describe('FocusedModeHeader', function() { ...@@ -68,7 +68,7 @@ describe('FocusedModeHeader', function() {
it("should render status text indicating that all user's annotations are visible", () => { it("should render status text indicating that all user's annotations are visible", () => {
const wrapper = createComponent(); const wrapper = createComponent();
assert.match(wrapper.text(), /Everybody.*s annotations/); assert.match(wrapper.text(), /Showing.+all/);
}); });
it("should render a button allowing the user to view only focus user's annotations", () => { it("should render a button allowing the user to view only focus user's annotations", () => {
......
...@@ -26,6 +26,8 @@ describe('SearchStatusBar', () => { ...@@ -26,6 +26,8 @@ describe('SearchStatusBar', () => {
directLinked: {}, directLinked: {},
}), }),
annotationCount: sinon.stub().returns(1), annotationCount: sinon.stub().returns(1),
focusModeFocused: sinon.stub().returns(false),
focusModeUserPrettyName: sinon.stub().returns('Fake User'),
noteCount: sinon.stub().returns(0), noteCount: sinon.stub().returns(0),
}; };
...@@ -116,6 +118,52 @@ describe('SearchStatusBar', () => { ...@@ -116,6 +118,52 @@ describe('SearchStatusBar', () => {
}); });
}); });
context('user-focused mode applied', () => {
beforeEach(() => {
fakeStore.focusModeFocused = sinon.stub().returns(true);
});
it('should not display a clear/show-all-annotations button when user-focused', () => {
const wrapper = createComponent({});
const buttons = wrapper.find('button');
assert.equal(buttons.length, 0);
});
[
{
description:
'shows pluralized annotation count when multiple annotations match for user',
children: [
{ id: '1', visible: true, children: [] },
{ id: '2', visible: true, children: [] },
],
expected: 'Showing 2 annotations',
},
{
description:
'shows single annotation count when one annotation matches for user',
children: [{ id: '1', visible: true, children: [] }],
expected: 'Showing 1 annotation',
},
{
description:
'shows "no annotations" wording when no annotations match for user',
children: [],
expected: 'No annotations for Fake User',
},
].forEach(test => {
it(test.description, () => {
fakeRootThread.thread.returns({
children: test.children,
});
const wrapper = createComponent({});
const resultText = wrapper.find('strong').text();
assert.equal(resultText, test.expected);
});
});
});
it('displays "Show all annotations" button when a direct-linked group fetch fails', () => { it('displays "Show all annotations" button when a direct-linked group fetch fails', () => {
fakeStore.getRootState.returns({ fakeStore.getRootState.returns({
selection: { selection: {
......
...@@ -12,11 +12,6 @@ ...@@ -12,11 +12,6 @@
height: 30px; height: 30px;
padding-left: 10px; padding-left: 10px;
padding-right: 10px; padding-right: 10px;
background-color: $grey-2;
color: $grey-5;
&:hover:enabled {
background-color: $grey-3;
}
} }
&__filter-status { &__filter-status {
......
...@@ -129,6 +129,10 @@ hypothesis-app { ...@@ -129,6 +129,10 @@ hypothesis-app {
} }
} }
.sheet--short {
padding: 0.5em 1em;
}
.sheet--is-theme-clean { .sheet--is-theme-clean {
padding-left: 30px; padding-left: 30px;
padding-bottom: 30px; padding-bottom: 30px;
......
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