Commit cef29a96 authored by Alejandro Celaya's avatar Alejandro Celaya Committed by Alejandro Celaya

Make ShareDialog receive the tabs to render

parent 3ea79419
...@@ -4,6 +4,7 @@ import { useEffect, useMemo } from 'preact/hooks'; ...@@ -4,6 +4,7 @@ import { useEffect, useMemo } from 'preact/hooks';
import { confirm } from '../../shared/prompts'; import { confirm } from '../../shared/prompts';
import type { SidebarSettings } from '../../types/config'; import type { SidebarSettings } from '../../types/config';
import { serviceConfig } from '../config/service-config'; import { serviceConfig } from '../config/service-config';
import { isThirdPartyService } from '../helpers/is-third-party-service';
import { shouldAutoDisplayTutorial } from '../helpers/session'; import { shouldAutoDisplayTutorial } from '../helpers/session';
import { applyTheme } from '../helpers/theme'; import { applyTheme } from '../helpers/theme';
import { withServices } from '../service-context'; import { withServices } from '../service-context';
...@@ -62,6 +63,12 @@ function HypothesisApp({ ...@@ -62,6 +63,12 @@ function HypothesisApp({
} }
}, [isSidebar, profile, settings, store]); }, [isSidebar, profile, settings, store]);
const isThirdParty = isThirdPartyService(settings);
const exportAnnotations = store.isFeatureEnabled('export_annotations');
const importAnnotations = store.isFeatureEnabled('import_annotations');
const showShareButton =
!isThirdParty || exportAnnotations || importAnnotations;
const login = async () => { const login = async () => {
if (serviceConfig(settings)) { if (serviceConfig(settings)) {
// Let the host page handle the login request // Let the host page handle the login request
...@@ -155,12 +162,19 @@ function HypothesisApp({ ...@@ -155,12 +162,19 @@ function HypothesisApp({
onSignUp={signUp} onSignUp={signUp}
onLogout={logout} onLogout={logout}
isSidebar={isSidebar} isSidebar={isSidebar}
showShareButton={showShareButton}
/> />
)} )}
<div className="container"> <div className="container">
<ToastMessages /> <ToastMessages />
<HelpPanel /> <HelpPanel />
<ShareDialog /> {showShareButton && (
<ShareDialog
shareTab={!isThirdParty}
exportTab={exportAnnotations}
importTab={importAnnotations}
/>
)}
{route && ( {route && (
<main> <main>
......
...@@ -9,23 +9,34 @@ import ShareAnnotations from './ShareAnnotations'; ...@@ -9,23 +9,34 @@ import ShareAnnotations from './ShareAnnotations';
import TabHeader from './TabHeader'; import TabHeader from './TabHeader';
import TabPanel from './TabPanel'; import TabPanel from './TabPanel';
export type ShareDialogProps = {
/** If true, the share tab will be rendered. Defaults to false */
shareTab?: boolean;
/** If true, the export tab will be rendered. Defaults to false */
exportTab?: boolean;
/** If true, the import tab will be rendered. Defaults to false */
importTab?: boolean;
};
/** /**
* Panel with sharing options. * Panel with sharing options.
* - If export feature flag is enabled, will show a tabbed interface with * - If provided tabs include `export` or `import`, will show a tabbed interface
* share and export tabs
* - Else, shows a single "Share annotations" interface * - Else, shows a single "Share annotations" interface
*/ */
export default function ShareDialog() { export default function ShareDialog({
shareTab,
exportTab,
importTab,
}: ShareDialogProps) {
const store = useSidebarStore(); const store = useSidebarStore();
const focusedGroup = store.focusedGroup(); const focusedGroup = store.focusedGroup();
const groupName = (focusedGroup && focusedGroup.name) || '...'; const groupName = (focusedGroup && focusedGroup.name) || '...';
const panelTitle = `Share Annotations in ${groupName}`; const panelTitle = `Share Annotations in ${groupName}`;
const showExportTab = store.isFeatureEnabled('export_annotations'); const tabbedDialog = exportTab || importTab;
const showImportTab = store.isFeatureEnabled('import_annotations');
const tabbedDialog = showExportTab || showImportTab;
const [selectedTab, setSelectedTab] = useState<'share' | 'export' | 'import'>( const [selectedTab, setSelectedTab] = useState<'share' | 'export' | 'import'>(
'share', // Determine initial selected tab, based on the first tab that will be displayed
shareTab ? 'share' : exportTab ? 'export' : 'import',
); );
return ( return (
...@@ -37,17 +48,19 @@ export default function ShareDialog() { ...@@ -37,17 +48,19 @@ export default function ShareDialog() {
{tabbedDialog && ( {tabbedDialog && (
<> <>
<TabHeader> <TabHeader>
<Tab {shareTab && (
id="share-panel-tab" <Tab
aria-controls="share-panel" id="share-panel-tab"
variant="tab" aria-controls="share-panel"
selected={selectedTab === 'share'} variant="tab"
onClick={() => setSelectedTab('share')} selected={selectedTab === 'share'}
textContent={'Share'} onClick={() => setSelectedTab('share')}
> textContent="Share"
Share >
</Tab> Share
{showExportTab && ( </Tab>
)}
{exportTab && (
<Tab <Tab
id="export-panel-tab" id="export-panel-tab"
aria-controls="export-panel" aria-controls="export-panel"
...@@ -59,7 +72,7 @@ export default function ShareDialog() { ...@@ -59,7 +72,7 @@ export default function ShareDialog() {
Export Export
</Tab> </Tab>
)} )}
{showImportTab && ( {importTab && (
<Tab <Tab
id="import-panel-tab" id="import-panel-tab"
aria-controls="import-panel" aria-controls="import-panel"
...@@ -85,7 +98,7 @@ export default function ShareDialog() { ...@@ -85,7 +98,7 @@ export default function ShareDialog() {
id="export-panel" id="export-panel"
active={selectedTab === 'export'} active={selectedTab === 'export'}
aria-labelledby="export-panel-tab" aria-labelledby="export-panel-tab"
title={`Export from ${focusedGroup?.name ?? '...'}`} title={`Export from ${groupName}`}
> >
<ExportAnnotations /> <ExportAnnotations />
</TabPanel> </TabPanel>
...@@ -93,14 +106,14 @@ export default function ShareDialog() { ...@@ -93,14 +106,14 @@ export default function ShareDialog() {
id="import-panel" id="import-panel"
active={selectedTab === 'import'} active={selectedTab === 'import'}
aria-labelledby="import-panel-tab" aria-labelledby="import-panel-tab"
title={`Import into ${focusedGroup?.name ?? '...'}`} title={`Import into ${groupName}`}
> >
<ImportAnnotations /> <ImportAnnotations />
</TabPanel> </TabPanel>
</Card> </Card>
</> </>
)} )}
{!tabbedDialog && <ShareAnnotations />} {shareTab && !tabbedDialog && <ShareAnnotations />}
</SidebarPanel> </SidebarPanel>
); );
} }
...@@ -14,12 +14,12 @@ describe('ShareDialog', () => { ...@@ -14,12 +14,12 @@ describe('ShareDialog', () => {
id: 'testprivate', id: 'testprivate',
}; };
const createComponent = () => mount(<ShareDialog />); const createComponent = (props = {}) =>
mount(<ShareDialog shareTab exportTab importTab {...props} />);
beforeEach(() => { beforeEach(() => {
fakeStore = { fakeStore = {
focusedGroup: sinon.stub().returns(fakePrivateGroup), focusedGroup: sinon.stub().returns(fakePrivateGroup),
isFeatureEnabled: sinon.stub().returns(false),
}; };
$imports.$mock(mockImportedComponents()); $imports.$mock(mockImportedComponents());
...@@ -58,10 +58,6 @@ describe('ShareDialog', () => { ...@@ -58,10 +58,6 @@ describe('ShareDialog', () => {
}); });
}); });
function enableFeature(feature) {
fakeStore.isFeatureEnabled.withArgs(feature).returns(true);
}
function selectTab(wrapper, name) { function selectTab(wrapper, name) {
wrapper wrapper
.find(`Tab[aria-controls="${name}-panel"]`) .find(`Tab[aria-controls="${name}-panel"]`)
...@@ -77,17 +73,20 @@ describe('ShareDialog', () => { ...@@ -77,17 +73,20 @@ describe('ShareDialog', () => {
return wrapper.find('TabPanel').filter({ active: true }); return wrapper.find('TabPanel').filter({ active: true });
} }
it('does not render a tabbed dialog if import/export feature flags are not enabled', () => { it('does not render a tabbed dialog if only share tab is provided', () => {
const wrapper = createComponent(); const wrapper = createComponent({ exportTab: false, importTab: false });
assert.isFalse(wrapper.find('TabHeader').exists()); assert.isFalse(wrapper.find('TabHeader').exists());
}); });
['export_annotations', 'import_annotations'].forEach(feature => { [
it(`renders a tabbed dialog when ${feature} feature is enabled`, () => { [{ shareTab: false }],
enableFeature('export_annotations'); [{ importTab: false }],
[{ exportTab: false }],
const wrapper = createComponent(); [{}],
].forEach(props => {
it(`renders a tabbed dialog when more than one tab is provided`, () => {
const wrapper = createComponent(props);
assert.isTrue(wrapper.find('TabHeader').exists()); assert.isTrue(wrapper.find('TabHeader').exists());
assert.isTrue( assert.isTrue(
...@@ -98,9 +97,6 @@ describe('ShareDialog', () => { ...@@ -98,9 +97,6 @@ describe('ShareDialog', () => {
}); });
it('shows correct tab panel when each tab is clicked', () => { it('shows correct tab panel when each tab is clicked', () => {
enableFeature('export_annotations');
enableFeature('import_annotations');
const wrapper = createComponent(); const wrapper = createComponent();
selectTab(wrapper, 'export'); selectTab(wrapper, 'export');
...@@ -119,24 +115,30 @@ describe('ShareDialog', () => { ...@@ -119,24 +115,30 @@ describe('ShareDialog', () => {
assert.equal(activeTabPanel(wrapper).props().id, 'share-panel'); assert.equal(activeTabPanel(wrapper).props().id, 'share-panel');
}); });
describe('a11y', () => { it('renders empty if no tabs should be displayed', () => {
beforeEach(() => { const wrapper = createComponent({
enableFeature('export_annotations'); shareTab: false,
enableFeature('import_annotations'); exportTab: false,
importTab: false,
}); });
assert.isFalse(wrapper.exists('TabHeader'));
assert.isFalse(wrapper.exists('ShareAnnotations'));
});
describe('a11y', () => {
it( it(
'should pass a11y checks', 'should pass a11y checks',
checkAccessibility({ checkAccessibility({
content: () => content: () =>
// ShareDialog renders a Fragment as its top-level component when // ShareDialog renders a Fragment as its top-level component when
// `export_annotations` feature is enabled. // it has import and/or export tabs.
// Wrapping it in a `div` ensures `checkAccessibility` internal logic // Wrapping it in a `div` ensures `checkAccessibility` internal logic
// does not discard all the Fragment children but the first one. // does not discard all the Fragment children but the first one.
// See https://github.com/hypothesis/client/issues/5671 // See https://github.com/hypothesis/client/issues/5671
mount( mount(
<div> <div>
<ShareDialog /> <ShareDialog shareTab exportTab importTab />
</div>, </div>,
), ),
}), }),
......
...@@ -8,7 +8,6 @@ import classnames from 'classnames'; ...@@ -8,7 +8,6 @@ import classnames from 'classnames';
import type { SidebarSettings } from '../../types/config'; import type { SidebarSettings } from '../../types/config';
import { serviceConfig } from '../config/service-config'; import { serviceConfig } from '../config/service-config';
import { isThirdPartyService } from '../helpers/is-third-party-service';
import { applyTheme } from '../helpers/theme'; import { applyTheme } from '../helpers/theme';
import { withServices } from '../service-context'; import { withServices } from '../service-context';
import type { FrameSyncService } from '../services/frame-sync'; import type { FrameSyncService } from '../services/frame-sync';
...@@ -21,6 +20,8 @@ import StreamSearchInput from './StreamSearchInput'; ...@@ -21,6 +20,8 @@ import StreamSearchInput from './StreamSearchInput';
import UserMenu from './UserMenu'; import UserMenu from './UserMenu';
export type TopBarProps = { export type TopBarProps = {
showShareButton: boolean;
/** Flag indicating whether the app is in a sidebar context */ /** Flag indicating whether the app is in a sidebar context */
isSidebar: boolean; isSidebar: boolean;
...@@ -49,8 +50,8 @@ function TopBar({ ...@@ -49,8 +50,8 @@ function TopBar({
onSignUp, onSignUp,
frameSync, frameSync,
settings, settings,
showShareButton,
}: TopBarProps) { }: TopBarProps) {
const showSharePageButton = !isThirdPartyService(settings);
const loginLinkStyle = applyTheme(['accentColor'], settings); const loginLinkStyle = applyTheme(['accentColor'], settings);
const store = useSidebarStore(); const store = useSidebarStore();
...@@ -106,7 +107,7 @@ function TopBar({ ...@@ -106,7 +107,7 @@ function TopBar({
onSearch={store.setFilterQuery} onSearch={store.setFilterQuery}
/> />
<SortMenu /> <SortMenu />
{showSharePageButton && ( {showShareButton && (
<IconButton <IconButton
icon={ShareIcon} icon={ShareIcon}
expanded={isAnnotationsPanelOpen} expanded={isAnnotationsPanelOpen}
......
...@@ -14,6 +14,7 @@ describe('HypothesisApp', () => { ...@@ -14,6 +14,7 @@ describe('HypothesisApp', () => {
let fakeShouldAutoDisplayTutorial = null; let fakeShouldAutoDisplayTutorial = null;
let fakeSettings = null; let fakeSettings = null;
let fakeToastMessenger = null; let fakeToastMessenger = null;
let fakeIsThirdPartyService;
const createComponent = (props = {}) => { const createComponent = (props = {}) => {
return mount( return mount(
...@@ -53,6 +54,7 @@ describe('HypothesisApp', () => { ...@@ -53,6 +54,7 @@ describe('HypothesisApp', () => {
route: sinon.stub().returns('sidebar'), route: sinon.stub().returns('sidebar'),
getLink: sinon.stub(), getLink: sinon.stub(),
isFeatureEnabled: sinon.stub().returns(true),
}; };
fakeAuth = {}; fakeAuth = {};
...@@ -76,6 +78,8 @@ describe('HypothesisApp', () => { ...@@ -76,6 +78,8 @@ describe('HypothesisApp', () => {
fakeConfirm = sinon.stub().resolves(false); fakeConfirm = sinon.stub().resolves(false);
fakeIsThirdPartyService = sinon.stub().returns(false);
$imports.$mock(mockImportedComponents()); $imports.$mock(mockImportedComponents());
$imports.$mock({ $imports.$mock({
'../config/service-config': { serviceConfig: fakeServiceConfig }, '../config/service-config': { serviceConfig: fakeServiceConfig },
...@@ -85,6 +89,9 @@ describe('HypothesisApp', () => { ...@@ -85,6 +89,9 @@ describe('HypothesisApp', () => {
}, },
'../helpers/theme': { applyTheme: fakeApplyTheme }, '../helpers/theme': { applyTheme: fakeApplyTheme },
'../../shared/prompts': { confirm: fakeConfirm }, '../../shared/prompts': { confirm: fakeConfirm },
'../helpers/is-third-party-service': {
isThirdPartyService: fakeIsThirdPartyService,
},
}); });
}); });
...@@ -400,4 +407,21 @@ describe('HypothesisApp', () => { ...@@ -400,4 +407,21 @@ describe('HypothesisApp', () => {
assert.isFalse(container.hasClass('theme-clean')); assert.isFalse(container.hasClass('theme-clean'));
}); });
}); });
context('when there are no sharing tabs to show', () => {
beforeEach(() => {
fakeStore.isFeatureEnabled.returns(false);
fakeIsThirdPartyService.returns(true);
});
it('does not render ShareDialog', () => {
const wrapper = createComponent();
assert.isFalse(wrapper.exists('ShareDialog'));
});
it('disables share button in TopBar', () => {
const wrapper = createComponent();
assert.isFalse(wrapper.find('TopBar').prop('showShareButton'));
});
});
}); });
...@@ -9,12 +9,9 @@ describe('TopBar', () => { ...@@ -9,12 +9,9 @@ describe('TopBar', () => {
let fakeFrameSync; let fakeFrameSync;
let fakeStore; let fakeStore;
let fakeStreamer; let fakeStreamer;
let fakeIsThirdPartyService;
let fakeServiceConfig; let fakeServiceConfig;
beforeEach(() => { beforeEach(() => {
fakeIsThirdPartyService = sinon.stub().returns(false);
fakeStore = { fakeStore = {
filterQuery: sinon.stub().returns(null), filterQuery: sinon.stub().returns(null),
hasFetchedProfile: sinon.stub().returns(false), hasFetchedProfile: sinon.stub().returns(false),
...@@ -38,9 +35,6 @@ describe('TopBar', () => { ...@@ -38,9 +35,6 @@ describe('TopBar', () => {
$imports.$mock({ $imports.$mock({
'../store': { useSidebarStore: () => fakeStore }, '../store': { useSidebarStore: () => fakeStore },
'../helpers/is-third-party-service': {
isThirdPartyService: fakeIsThirdPartyService,
},
'../config/service-config': { serviceConfig: fakeServiceConfig }, '../config/service-config': { serviceConfig: fakeServiceConfig },
}); });
}); });
...@@ -63,6 +57,7 @@ describe('TopBar', () => { ...@@ -63,6 +57,7 @@ describe('TopBar', () => {
isSidebar={true} isSidebar={true}
settings={fakeSettings} settings={fakeSettings}
streamer={fakeStreamer} streamer={fakeStreamer}
showShareButton
{...props} {...props}
/>, />,
); );
...@@ -149,13 +144,6 @@ describe('TopBar', () => { ...@@ -149,13 +144,6 @@ describe('TopBar', () => {
}); });
}); });
it("checks whether we're using a third-party service", () => {
createTopBar();
assert.called(fakeIsThirdPartyService);
assert.alwaysCalledWithExactly(fakeIsThirdPartyService, fakeSettings);
});
context('when using a first-party service', () => { context('when using a first-party service', () => {
it('shows the share annotations button', () => { it('shows the share annotations button', () => {
const wrapper = createTopBar(); const wrapper = createTopBar();
...@@ -163,13 +151,9 @@ describe('TopBar', () => { ...@@ -163,13 +151,9 @@ describe('TopBar', () => {
}); });
}); });
context('when using a third-party service', () => { context('when showShareButton is false', () => {
beforeEach(() => {
fakeIsThirdPartyService.returns(true);
});
it("doesn't show the share annotations button", () => { it("doesn't show the share annotations button", () => {
const wrapper = createTopBar(); const wrapper = createTopBar({ showShareButton: false });
assert.isFalse( assert.isFalse(
wrapper.exists('[title="Share annotations on this page"]'), wrapper.exists('[title="Share annotations on this page"]'),
); );
......
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