Migrate Help sidebar panel to tabs

hypothesis · Nov 28, 2023 · 5f0e7b7 · 5f0e7b7
1 parent b34060f
commit 5f0e7b7
Show file tree

Hide file tree

Showing 2 changed files with 93 additions and 122 deletions.
diff --git a/src/sidebar/components/HelpPanel.tsx b/src/sidebar/components/HelpPanel.tsx
@@ -1,39 +1,19 @@
-import { Link, LinkButton } from '@hypothesis/frontend-shared';
-import { ArrowRightIcon, ExternalIcon } from '@hypothesis/frontend-shared';
-import type { ComponentChildren as Children } from 'preact';
-import { useCallback, useMemo, useState } from 'preact/hooks';
+import { Card, Link, Tab } from '@hypothesis/frontend-shared';
+import { ExternalIcon } from '@hypothesis/frontend-shared';
+import classnames from 'classnames';
+import { useCallback, useId, useMemo, useState } from 'preact/hooks';
 
 import { username } from '../helpers/account-id';
 import { VersionData } from '../helpers/version-data';
 import { withServices } from '../service-context';
 import type { SessionService } from '../services/session';
 import { useSidebarStore } from '../store';
+import TabHeader from './ShareDialog/TabHeader';
+import TabPanel from './ShareDialog/TabPanel';
 import SidebarPanel from './SidebarPanel';
 import Tutorial from './Tutorial';
 import VersionInfo from './VersionInfo';
 
-type HelpPanelNavigationButtonProps = {
-  children: Children;
-  onClick: (e: Event) => void;
-};
-
-/**
- * Navigation link-button to swap between sub-panels in the help panel
- */
-function HelpPanelNavigationButton({
-  children,
-  onClick,
-}: HelpPanelNavigationButtonProps) {
-  return (
-    <LinkButton variant="brand" onClick={onClick} underline="hover">
-      <div className="flex items-center gap-x-1">
-        {children}
-        <ArrowRightIcon className="w-em h-em" />
-      </div>
-    </LinkButton>
-  );
-}
-
 type HelpPanelTabProps = {
   /** What the tab's link should say. */
   linkText: string;
@@ -42,7 +22,7 @@ type HelpPanelTabProps = {
 };
 
 /**
- * External link "tabs" inside of the help panel.
+ * External link "tabs" at the bottom of the help panel.
  */
 function HelpPanelTab({ linkText, url }: HelpPanelTabProps) {
   return (
@@ -64,6 +44,8 @@ type HelpPanelProps = {
   session: SessionService;
 };
 
+type PanelKey = 'tutorial' | 'versionInfo';
+
 /**
  * A help sidebar panel with two sub-panels: tutorial and version info.
  */
@@ -74,6 +56,10 @@ function HelpPanel({ session }: HelpPanelProps) {
   const profile = store.profile();
   const displayName =
     profile.user_info?.display_name ?? username(profile.userid);
+  const tutorialTabId = useId();
+  const tutorialPanelId = useId();
+  const versionTabId = useId();
+  const versionPanelId = useId();
 
   // Should this panel be auto-opened at app launch? Note that the actual
   // auto-open triggering of this panel is owned by the `HypothesisApp` component.
@@ -82,12 +68,6 @@ function HelpPanel({ session }: HelpPanelProps) {
   const hasAutoDisplayPreference =
     !!store.profile().preferences.show_sidebar_tutorial;
 
-  const subPanelTitles = {
-    tutorial: 'Getting started',
-    versionInfo: 'About this version',
-  };
-  type PanelKey = keyof typeof subPanelTitles;
-
   // The "Tutorial" (getting started) subpanel is the default panel shown
   const [activeSubPanel, setActiveSubPanel] = useState<PanelKey>('tutorial');
 
@@ -115,11 +95,6 @@ function HelpPanel({ session }: HelpPanelProps) {
   // create-new-ticket form
   const supportTicketURL = `https://web.hypothes.is/get-help/?sys_info=${versionData.asEncodedURLString()}`;
 
-  const openSubPanel = (e: Event, panelName: PanelKey) => {
-    e.preventDefault();
-    setActiveSubPanel(panelName);
-  };
-
   const onActiveChanged = useCallback(
     (active: boolean) => {
       if (!active && hasAutoDisplayPreference) {
@@ -137,41 +112,63 @@ function HelpPanel({ session }: HelpPanelProps) {
       title="Help"
       panelName="help"
       onActiveChanged={onActiveChanged}
+      variant="custom"
     >
-      <div className="space-y-4">
-        <div className="flex items-center">
-          <h3 className="grow text-md font-medium" data-testid="subpanel-title">
-            {subPanelTitles[activeSubPanel]}
-          </h3>
-          {activeSubPanel === 'versionInfo' && (
-            <HelpPanelNavigationButton
-              onClick={e => openSubPanel(e, 'tutorial')}
-            >
-              Getting started
-            </HelpPanelNavigationButton>
-          )}
-          {activeSubPanel === 'tutorial' && (
-            <HelpPanelNavigationButton
-              onClick={e => openSubPanel(e, 'versionInfo')}
-            >
-              About this version
-            </HelpPanelNavigationButton>
-          )}
-        </div>
-        <div className="border-y py-4">
-          {activeSubPanel === 'tutorial' && <Tutorial />}
-          {activeSubPanel === 'versionInfo' && (
+      <TabHeader>
+        <Tab
+          id={tutorialTabId}
+          aria-controls={tutorialPanelId}
+          variant="tab"
+          textContent="Help"
+          selected={activeSubPanel === 'tutorial'}
+          onClick={() => setActiveSubPanel('tutorial')}
+          data-testid="tutorial-tab"
+        >
+          Help
+        </Tab>
+        <Tab
+          id={versionTabId}
+          aria-controls={versionPanelId}
+          variant="tab"
+          textContent="Version"
+          selected={activeSubPanel === 'versionInfo'}
+          onClick={() => setActiveSubPanel('versionInfo')}
+          data-testid="version-info-tab"
+        >
+          Version
+        </Tab>
+      </TabHeader>
+      <Card
+        classes={classnames({
+          'rounded-tl-none': activeSubPanel === 'tutorial',
+        })}
+      >
+        <div className="border-b">
+          <TabPanel
+            id={tutorialPanelId}
+            aria-labelledby={tutorialTabId}
+            active={activeSubPanel === 'tutorial'}
+            title="Getting started"
+          >
+            <Tutorial />
+          </TabPanel>
+          <TabPanel
+            id={versionPanelId}
+            aria-labelledby={versionTabId}
+            active={activeSubPanel === 'versionInfo'}
+            title="Version details"
+          >
             <VersionInfo versionData={versionData} />
-          )}
+          </TabPanel>
         </div>
-        <div className="flex items-center">
+        <div className="flex items-center p-3">
           <HelpPanelTab
             linkText="Help topics"
             url="https://web.hypothes.is/help/"
           />
           <HelpPanelTab linkText="New support ticket" url={supportTicketURL} />
         </div>
-      </div>
+      </Card>
     </SidebarPanel>
   );
 }

diff --git a/src/sidebar/components/test/HelpPanel-test.js b/src/sidebar/components/test/HelpPanel-test.js
@@ -1,3 +1,4 @@
+import { Tab } from '@hypothesis/frontend-shared';
 import {
   checkAccessibility,
   mockImportedComponents,
@@ -45,45 +46,46 @@ describe('HelpPanel', () => {
       '../store': { useSidebarStore: () => fakeStore },
       '../helpers/version-data': { VersionData: FakeVersionData },
     });
+    $imports.$restore({
+      // Rendering TabHeader and TabPanel is needed for a11y tests
+      './ShareDialog/TabHeader': true,
+      './ShareDialog/TabPanel': true,
+    });
   });
 
   afterEach(() => {
     $imports.$restore();
   });
 
+  const getActivePanel = wrapper => wrapper.find('TabPanel[active=true]');
+  const clickTab = (wrapper, tabId) =>
+    wrapper.find(`button[data-testid="${tabId}"]`).simulate('click');
+
   context('when viewing tutorial sub-panel', () => {
     it('should show tutorial by default', () => {
       const wrapper = createComponent();
-      const subHeader = wrapper.find('[data-testid="subpanel-title"]');
-
-      assert.include(subHeader.text(), 'Getting started');
-      assert.isTrue(wrapper.find('Tutorial').exists());
-      assert.isFalse(wrapper.find('VersionInfo').exists());
+      const selectedTab = wrapper
+        .find(Tab)
+        .findWhere(tab => tab.prop('selected'));
+      const activePanel = getActivePanel(wrapper);
+
+      assert.include(selectedTab.text(), 'Help');
+      assert.isTrue(activePanel.find('Tutorial').exists());
+      assert.isFalse(activePanel.find('VersionInfo').exists());
     });
 
-    it('should show navigation link to versionInfo sub-panel', () => {
+    it('should switch to versionInfo sub-panel when second tab is clicked', async () => {
       const wrapper = createComponent();
-      const button = wrapper.find('HelpPanelNavigationButton');
+      clickTab(wrapper, 'version-info-tab');
 
-      assert.include(button.text(), 'About this version');
-    });
+      const activePanel = getActivePanel(wrapper);
 
-    it('should switch to versionInfo sub-panel when navigation button clicked', async () => {
-      const wrapper = createComponent();
-      act(() => {
-        wrapper
-          .find('LinkButton')
-          .getDOMNode()
-          .dispatchEvent(new Event('click'));
-      });
-      wrapper.update();
-
-      assert.isTrue(wrapper.find('VersionInfo').exists());
+      assert.isTrue(activePanel.find('VersionInfo').exists());
       assert.equal(
-        wrapper.find('VersionInfo').prop('versionData'),
+        activePanel.find('VersionInfo').prop('versionData'),
         fakeVersionData,
       );
-      assert.isFalse(wrapper.find('Tutorial').exists());
+      assert.isFalse(activePanel.find('Tutorial').exists());
     });
   });
 
@@ -137,46 +139,18 @@ describe('HelpPanel', () => {
       ]);
     });
 
-    it('should show navigation link back to tutorial sub-panel', () => {
-      const wrapper = createComponent();
-      act(() => {
-        wrapper
-          .find('LinkButton')
-          .getDOMNode()
-          .dispatchEvent(new Event('click'));
-      });
-      wrapper.update();
-
-      const link = wrapper.find('LinkButton');
-
-      assert.isTrue(wrapper.find('VersionInfo').exists());
-      assert.isFalse(wrapper.find('Tutorial').exists());
-      assert.include(link.text(), 'Getting started');
-    });
-
-    it('should switch to tutorial sub-panel when link clicked', () => {
+    it('should switch to tutorial sub-panel when first tab is clicked', () => {
       const wrapper = createComponent();
 
       // Click to get to VersionInfo sub-panel...
-      act(() => {
-        wrapper
-          .find('LinkButton')
-          .getDOMNode()
-          .dispatchEvent(new Event('click'));
-      });
-      wrapper.update();
-
+      clickTab(wrapper, 'version-info-tab');
       // Click again to get back to tutorial sub-panel
-      act(() => {
-        wrapper
-          .find('LinkButton')
-          .getDOMNode()
-          .dispatchEvent(new Event('click'));
-      });
-      wrapper.update();
+      clickTab(wrapper, 'tutorial-tab');
+
+      const activePanel = getActivePanel(wrapper);
 
-      assert.isFalse(wrapper.find('VersionInfo').exists());
-      assert.isTrue(wrapper.find('Tutorial').exists());
+      assert.isFalse(activePanel.find('VersionInfo').exists());
+      assert.isTrue(activePanel.find('Tutorial').exists());
     });
   });
 
@@ -272,7 +246,7 @@ describe('HelpPanel', () => {
   it(
     'should pass a11y checks',
     checkAccessibility({
-      content: () => createComponent(),
+      content: () => <HelpPanel session={fakeSessionService} />,
     }),
   );
 });