Commit d823e0f6 authored by Robert Knight's avatar Robert Knight

Align `showTabs` argument to `threadAnnotations` with actual tab visibility

Align how `useRootThread` determines the `showTabs` argument it passes to
`threadAnnotations` with the way `SidebarView` determines whether to render the
tabs. The logic is not quite the same because `SidebarView` has some additional
conditions related to direct-linking errors. However this has been ignored to
align with the way `useRootThread` worked prior to the most recent changes.

The changes here are intended to be temporary: once the new search UI is enabled
for everyone, the tabs will always be shown and the logic will become much
simpler.
parent bb6a181b
...@@ -11,6 +11,9 @@ describe('sidebar/components/hooks/use-root-thread', () => { ...@@ -11,6 +11,9 @@ 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'),
hasAppliedFilter: sinon.stub().returns(false),
hasSelectedAnnotations: sinon.stub().returns(false),
isFeatureEnabled: sinon.stub().returns(true),
route: sinon.stub().returns('sidebar'), 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' }),
...@@ -52,7 +55,7 @@ describe('sidebar/components/hooks/use-root-thread', () => { ...@@ -52,7 +55,7 @@ describe('sidebar/components/hooks/use-root-thread', () => {
{ route: 'sidebar', showTabs: true }, { route: 'sidebar', showTabs: true },
{ route: 'notebook', showTabs: false }, { route: 'notebook', showTabs: false },
].forEach(({ route, showTabs }) => { ].forEach(({ route, showTabs }) => {
it('shows tabs in the sidebar only', () => { it('filters by tab in the sidebar only', () => {
fakeStore.route.returns(route); fakeStore.route.returns(route);
mount(<DummyComponent />); mount(<DummyComponent />);
...@@ -61,4 +64,42 @@ describe('sidebar/components/hooks/use-root-thread', () => { ...@@ -61,4 +64,42 @@ describe('sidebar/components/hooks/use-root-thread', () => {
assert.equal(threadState.showTabs, showTabs); assert.equal(threadState.showTabs, showTabs);
}); });
}); });
[
// When using search UI, always filter by tab.
{ newSearchUI: true, hasFilter: true, hasSelection: false, showTabs: true },
// When using old search UI, only filter by tab if no selection or filter
// is active.
{
newSearchUI: false,
hasFilter: true,
hasSelection: false,
showTabs: false,
},
{
newSearchUI: false,
hasFilter: false,
hasSelection: true,
showTabs: false,
},
{
newSearchUI: false,
hasFilter: false,
hasSelection: false,
showTabs: true,
},
].forEach(({ newSearchUI, hasFilter, hasSelection, showTabs }) => {
it('if `search_panel` is disabled, does not filter by tab if there is a filter active', () => {
fakeStore.route.returns('sidebar');
fakeStore.isFeatureEnabled.withArgs('search_panel').returns(newSearchUI);
fakeStore.hasAppliedFilter.returns(hasFilter);
fakeStore.hasSelectedAnnotations.returns(hasSelection);
mount(<DummyComponent />);
const threadState = fakeThreadAnnotations.getCall(0).args[0];
assert.equal(threadState.showTabs, showTabs);
});
});
}); });
...@@ -19,13 +19,22 @@ export function useRootThread(): ThreadAnnotationsResult { ...@@ -19,13 +19,22 @@ export function useRootThread(): ThreadAnnotationsResult {
const selectionState = store.selectionState(); const selectionState = store.selectionState();
const filters = store.getFilterValues(); const filters = store.getFilterValues();
// This logic mirrors code in `SidebarView`. It can be simplified once
// the "search_panel" feature is turned on everywhere.
const searchPanelEnabled = store.isFeatureEnabled('search_panel');
const hasAppliedFilter =
store.hasAppliedFilter() || store.hasSelectedAnnotations();
const showTabs =
route === 'sidebar' && (searchPanelEnabled || !hasAppliedFilter);
const threadState = useMemo((): ThreadState => { const threadState = useMemo((): ThreadState => {
const selection = { ...selectionState, filterQuery: query, filters };
return { return {
annotations, annotations,
selection: { ...selectionState, filterQuery: query, filters }, selection,
showTabs: route === 'sidebar', showTabs,
}; };
}, [annotations, query, route, selectionState, filters]); }, [annotations, query, selectionState, filters, showTabs]);
return threadAnnotations(threadState); return threadAnnotations(threadState);
} }
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