Skip to content

Commit

Permalink
Correctly select Orphan tab when viewing direct-linked orphan
Browse files Browse the repository at this point in the history
Previously, tab selection for direct-linked annotations was happening
before anchoring was complete—that means that orphaned annotations had
not yet been marked as being orphans (`$orphan`). Make sure that
direct-linked tab selection is checked any time the direct-linked
annotation changes.

Also fix a UI confusion in which the "Show All" button would show
the count of all annotations in this (direct-linked) state, which is
confusing because orphans don't count toward "All annotations" (nor do
page notes, FWIW).

Fixes #2686
  • Loading branch information
lyzadanger committed Dec 3, 2020
1 parent 1ccf10d commit d7c316e
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 4 deletions.
7 changes: 4 additions & 3 deletions src/sidebar/components/filter-status.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,7 @@ FilterStatusPanel.propTypes = {
*/
function SelectionFilterStatus({ filterState, rootThread }) {
const clearSelection = useStore(store => store.clearSelection);
const directLinkedId = useStore(store => store.directLinkedAnnotationId());
// The total number of top-level annotations (visible or not)
const totalCount = useStore(store => store.annotationCount());
// Count the number of visible annotations—top-level only
Expand All @@ -134,9 +135,9 @@ function SelectionFilterStatus({ filterState, rootThread }) {
// Because of the confusion between counts of entities between selected
// annotations and filtered annotations, don't display the total number
// when in user-focus mode because the numbers won't appear to make sense.
const buttonText = filterState.focusConfigured
? 'Show all'
: `Show all (${totalCount})`;
// Don't display total count, either, when viewing a direct-linked annotation.
const showCount = !filterState.focusConfigured && !directLinkedId;
const buttonText = showCount ? `Show all (${totalCount})` : 'Show all';

const button = (
<Button
Expand Down
13 changes: 12 additions & 1 deletion src/sidebar/components/sidebar-view.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ function SidebarView({
const hasAppliedFilter = useStore(store => store.hasAppliedFilter());
const isLoading = useStore(store => store.isLoading());
const isLoggedIn = useStore(store => store.isLoggedIn());

const linkedAnnotationId = useStore(store =>
store.directLinkedAnnotationId()
);
Expand All @@ -53,6 +54,7 @@ function SidebarView({
const directLinkedTab = linkedAnnotation
? tabForAnnotation(linkedAnnotation)
: 'annotation';

const searchUris = useStore(store => store.searchUris());
const sidebarHasOpened = useStore(store => store.hasSidebarOpened());
const userId = useStore(store => store.profile().userid);
Expand Down Expand Up @@ -122,8 +124,17 @@ function SidebarView({
frameSync.focusAnnotations([linkedAnnotationAnchorTag]);
frameSync.scrollToAnnotation(linkedAnnotationAnchorTag);
selectTab(directLinkedTab);
} else if (linkedAnnotation) {
// Make sure to allow for orphaned annotations (which won't have an anchor)
selectTab(directLinkedTab);
}
}, [directLinkedTab, frameSync, linkedAnnotationAnchorTag, selectTab]);
}, [
directLinkedTab,
frameSync,
linkedAnnotation,
linkedAnnotationAnchorTag,
selectTab,
]);

// Connect to the streamer when the sidebar has opened or if user is logged in
useEffect(() => {
Expand Down
11 changes: 11 additions & 0 deletions src/sidebar/components/test/filter-status-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ describe('FilterStatus', () => {
fakeStore = {
annotationCount: sinon.stub(),
clearSelection: sinon.stub(),
directLinkedAnnotationId: sinon.stub(),
filterState: sinon.stub().returns(getFilterState()),
toggleFocusMode: sinon.stub(),
};
Expand Down Expand Up @@ -184,6 +185,16 @@ describe('FilterStatus', () => {
callback: fakeStore.clearSelection,
});
});

it('should not show count of annotations on "Show All" button if direct-linked annotation present', () => {
fakeStore.annotationCount.returns(5);
fakeStore.directLinkedAnnotationId.returns(1);
assertButton(createComponent(), {
text: 'Show all',
icon: 'cancel',
callback: fakeStore.clearSelection,
});
});
});

context('(State 5): user-focus mode active', () => {
Expand Down
10 changes: 10 additions & 0 deletions src/sidebar/components/test/sidebar-view-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,16 @@ describe('SidebarView', () => {
assert.calledWith(fakeStore.selectTab, 'annotation');
});

it('selects the correct tab for direct-linked orphaned annotations', () => {
fakeStore.findAnnotationByID
.withArgs('someId')
.returns({ $orphan: true, $tag: 'myTag' });
fakeTabsUtil.tabForAnnotation.returns('orphan');
createComponent();
assert.calledOnce(fakeStore.selectTab);
assert.calledWith(fakeStore.selectTab, 'orphan');
});

it('renders a logged-out message CTA if user is not logged in', () => {
fakeStore.isLoggedIn.returns(false);
const wrapper = createComponent();
Expand Down

0 comments on commit d7c316e

Please sign in to comment.