Use SelectNext for import and export user selectors

package.json

@@ -12,7 +12,7 @@
     "@babel/preset-react": "^7.0.0",
     "@babel/preset-typescript": "^7.16.7",
     "@hypothesis/frontend-build": "^2.0.0",
-    "@hypothesis/frontend-shared": "^6.5.0",
+    "@hypothesis/frontend-shared": "^6.8.0",
     "@npmcli/arborist": "^7.0.0",
     "@octokit/rest": "^20.0.1",
     "@rollup/plugin-babel": "^6.0.0",

src/sidebar/components/ShareDialog/ExportAnnotations.tsx

@@ -3,20 +3,23 @@ import {
   CardActions,
   Link,
   Input,
-  Select,
+  SelectNext,
 } from '@hypothesis/frontend-shared';
+import classnames from 'classnames';
 import { useCallback, useMemo, useState } from 'preact/hooks';
 
 import { downloadJSONFile } from '../../../shared/download-json-file';
 import type { APIAnnotationData } from '../../../types/api';
 import { annotationDisplayName } from '../../helpers/annotation-user';
+import type { UserAnnotations } from '../../helpers/annotations-by-user';
 import { annotationsByUser } from '../../helpers/annotations-by-user';
 import { suggestedFilename } from '../../helpers/export-annotations';
 import { withServices } from '../../service-context';
 import type { AnnotationsExporter } from '../../services/annotations-exporter';
 import type { ToastMessengerService } from '../../services/toast-messenger';
 import { useSidebarStore } from '../../store';
 import LoadingSpinner from './LoadingSpinner';
+import { UserAnnotationsContent } from './UserAnnotationsContent';
 
 export type ExportAnnotationsProps = {
   // injected
@@ -51,9 +54,21 @@ function ExportAnnotations({
     [exportableAnnotations, getDisplayName],
   );
 
-  // User whose annotations are going to be exported. Preselect current user
+  // User whose annotations are going to be exported.
   const currentUser = store.profile().userid;
-  const [selectedUser, setSelectedUser] = useState(currentUser);
+  const allAnnotationsOption: UserAnnotations = useMemo(
+    () => ({
+      annotations: exportableAnnotations,
+      userid: '__all__',
+      displayName: 'All annotations',
+    }),
+    [exportableAnnotations],
+  );
+  const [selectedUser, setSelectedUser] = useState(
+    // Try to preselect current user
+    userList.find(userInfo => userInfo.userid === currentUser) ??
+      allAnnotationsOption,
+  );
 
   const draftCount = store.countDrafts();
 
@@ -76,8 +91,7 @@ function ExportAnnotations({
 
     try {
       const annotationsToExport =
-        userList.find(item => item.userid === selectedUser)?.annotations ??
-        exportableAnnotations;
+        selectedUser?.annotations ?? exportableAnnotations;
       const filename = `${customFilename ?? defaultFilename}.json`;
       const exportData =
         annotationsExporter.buildExportContent(annotationsToExport);
@@ -132,25 +146,29 @@ function ExportAnnotations({
           <label htmlFor="export-user" className="block font-medium">
             Select which user{"'"}s annotations to export:
           </label>
-          <Select
-            id="export-user"
-            onChange={e =>
-              setSelectedUser((e.target as HTMLSelectElement).value || null)
-            }
+          <SelectNext
+            value={selectedUser}
+            onChange={setSelectedUser}
+            label={<UserAnnotationsContent userAnnotations={selectedUser} />}
           >
-            <option value="" selected={!selectedUser}>
-              All annotations ({exportableAnnotations.length})
-            </option>
+            <SelectNext.Option value={allAnnotationsOption}>
+              {() => (
+                <UserAnnotationsContent
+                  userAnnotations={allAnnotationsOption}
+                />
+              )}
+            </SelectNext.Option>
             {userList.map(userInfo => (
-              <option
-                key={userInfo.userid}
-                value={userInfo.userid}
-                selected={userInfo.userid === selectedUser}
-              >
-                {userInfo.displayName} ({userInfo.annotations.length})
-              </option>
+              <SelectNext.Option key={userInfo.userid} value={userInfo}>
+                {({ selected }) => (
+                  <UserAnnotationsContent
+                    userAnnotations={userInfo}
+                    classes={classnames({ 'font-semibold': selected })}
+                  />
+                )}
+              </SelectNext.Option>
             ))}
-          </Select>
+          </SelectNext>
         </>
       ) : (
         <p data-testid="no-annotations-message">

src/sidebar/components/ShareDialog/ImportAnnotations.tsx

@@ -1,15 +1,23 @@
-import { Button, CardActions, Link, Select } from '@hypothesis/frontend-shared';
+import {
+  Button,
+  CardActions,
+  Link,
+  SelectNext,
+} from '@hypothesis/frontend-shared';
+import classnames from 'classnames';
 import { useCallback, useEffect, useId, useMemo, useState } from 'preact/hooks';
 
 import type { APIAnnotationData } from '../../../types/api';
 import { annotationDisplayName } from '../../helpers/annotation-user';
+import type { UserAnnotations } from '../../helpers/annotations-by-user';
 import { annotationsByUser } from '../../helpers/annotations-by-user';
 import { readExportFile } from '../../helpers/import';
 import { withServices } from '../../service-context';
 import type { ImportAnnotationsService } from '../../services/import-annotations';
 import { useSidebarStore } from '../../store';
 import CircularProgress from '../CircularProgress';
 import FileInput from './FileInput';
+import { UserAnnotationsContent } from './UserAnnotationsContent';
 
 export type ImportAnnotationsProps = {
   importAnnotationsService: ImportAnnotationsService;
@@ -33,7 +41,9 @@ function ImportAnnotations({
   const [error, setError] = useState<string | null>(null);
 
   // User whose annotations are going to be imported.
-  const [selectedUser, setSelectedUser] = useState<string | null>(null);
+  const [selectedUser, setSelectedUser] = useState<UserAnnotations | null>(
+    null,
+  );
 
   const store = useSidebarStore();
   const currentUser = store.profile().userid;
@@ -59,6 +69,11 @@ function ImportAnnotations({
     [annotations, getDisplayName],
   );
 
+  // Try to pre-select current user
+  if (userList && !selectedUser) {
+    setSelectedUser(userList.find(user => user.userid === currentUser) ?? null);
+  }
+
   // Parse input file, extract annotations and update the user list.
   useEffect(() => {
     if (!currentUser || !file) {
@@ -68,36 +83,22 @@ function ImportAnnotations({
     setError(null);
     setSelectedUser(null);
     readExportFile(file)
-      .then(annotations => {
-        setAnnotations(annotations);
-
-        // Pre-select the current user in the list, if at least one of the
-        // annotations was authored by them.
-        const userMatch = annotations.some(ann => ann.user === currentUser);
-        setSelectedUser(userMatch ? currentUser : null);
-      })
+      .then(setAnnotations)
       .catch(err => {
         setError(err.message);
       });
   }, [currentUser, file]);
 
   let importAnnotations;
-  if (annotations && selectedUser && userList) {
+  if (selectedUser) {
     importAnnotations = async () => {
-      const userEntry = userList.find(item => item.userid === selectedUser);
-      /* istanbul ignore next - This should never be triggered */
-      if (!userEntry) {
-        return;
-      }
-
       // nb. In the event of an error, `import` will report that directly via
       // a toast message, so we don't do that ourselves.
-      importAnnotationsService.import(userEntry.annotations);
+      importAnnotationsService.import(selectedUser.annotations);
     };
   }
 
   const fileInputId = useId();
-  const userSelectId = useId();
 
   if (!currentUser) {
     // TODO - Make "Log in" a link.
@@ -156,28 +157,33 @@ function ImportAnnotations({
       <FileInput onFileSelected={setFile} disabled={busy} id={fileInputId} />
       {userList && (
         <>
-          <label htmlFor={userSelectId} className="block font-medium">
+          <p className="block font-medium">
             Select which user&apos;s annotations to import:
-          </label>
-          <Select
-            id={userSelectId}
-            data-testid="user-list"
-            disabled={busy}
-            onChange={e =>
-              setSelectedUser((e.target as HTMLSelectElement).value || null)
+          </p>
+          <SelectNext
+            value={selectedUser}
+            onChange={setSelectedUser}
+            label={
+              selectedUser ? (
+                <UserAnnotationsContent userAnnotations={selectedUser} />
+              ) : (
+                // This mimics a placeholder and makes it more accessible by
+                // preventing a button with no text
+                <span className="text-grey-6">Select a user...</span>
+              )
             }
           >
-            <option value="" selected={!selectedUser} />
             {userList.map(userInfo => (
-              <option
-                key={userInfo.userid}
-                value={userInfo.userid}
-                selected={userInfo.userid === selectedUser}
-              >
-                {userInfo.displayName} ({userInfo.annotations.length})
-              </option>
+              <SelectNext.Option key={userInfo.userid} value={userInfo}>
+                {({ selected }) => (
+                  <UserAnnotationsContent
+                    userAnnotations={userInfo}
+                    classes={classnames({ 'font-semibold': selected })}
+                  />
+                )}
+              </SelectNext.Option>
             ))}
-          </Select>
+          </SelectNext>
         </>
       )}
       {error && (

src/sidebar/components/ShareDialog/UserAnnotationsContent.tsx

@@ -0,0 +1,22 @@
+import classnames from 'classnames';
+
+import type { UserAnnotations } from '../../helpers/annotations-by-user';
+
+export type UserAnnotationsContentProps = {
+  userAnnotations: UserAnnotations;
+  classes?: string;
+};
+
+export function UserAnnotationsContent({
+  userAnnotations,
+  classes,
+}: UserAnnotationsContentProps) {
+  return (
+    <div className={classnames('flex gap-x-2', classes)}>
+      {userAnnotations.displayName}
+      <div className="rounded px-1 bg-grey-7 text-white">
+        {userAnnotations.annotations.length}
+      </div>
+    </div>
+  );
+}

src/sidebar/components/ShareDialog/test/ExportAnnotations-test.js

@@ -1,3 +1,4 @@
+import { SelectNext } from '@hypothesis/frontend-shared';
 import { mount } from 'enzyme';
 
 import { checkAccessibility } from '../../../../test-util/accessibility';
@@ -64,9 +65,12 @@ describe('ExportAnnotations', () => {
       '../../store': { useSidebarStore: () => fakeStore },
     });
 
-    // Restore this very simple component to get it test coverage
     $imports.$restore({
+      // Restore this very simple component to get it test coverage
       './LoadingSpinner': true,
+      // Restore UserAnnotationsContent, as it's used as some buttons' content
+      // and can make a11y tests fail
+      './UserAnnotationsContent': true,
     });
   });
 
@@ -124,9 +128,22 @@ describe('ExportAnnotations', () => {
         },
       ],
       userEntries: [
-        { value: '', text: 'All annotations (3)' }, // "No user selected" entry
-        { value: 'acct:brian@example.com', text: 'Brian Smith (2)' },
-        { value: 'acct:john@example.com', text: 'John Smith (1)' },
+        {
+          // "No user selected" entry
+          userid: '__all__',
+          displayName: 'All annotations',
+          annotationsCount: 3,
+        },
+        {
+          userid: 'acct:brian@example.com',
+          displayName: 'Brian Smith',
+          annotationsCount: 2,
+        },
+        {
+          userid: 'acct:john@example.com',
+          displayName: 'John Smith',
+          annotationsCount: 1,
+        },
       ],
     },
 
@@ -144,8 +161,17 @@ describe('ExportAnnotations', () => {
         },
       ],
       userEntries: [
-        { value: '', text: 'All annotations (1)' }, // "No user selected" entry
-        { value: 'acct:brian@example.com', text: 'Brian Smith (1)' },
+        {
+          // "No user selected" entry
+          userid: '__all__',
+          displayName: 'All annotations',
+          annotationsCount: 1,
+        },
+        {
+          userid: 'acct:brian@example.com',
+          displayName: 'Brian Smith',
+          annotationsCount: 1,
+        },
       ],
     },
   ].forEach(({ annotations, userEntries }) => {
@@ -154,13 +180,16 @@ describe('ExportAnnotations', () => {
 
       const wrapper = createComponent();
 
-      const userList = await waitForElement(wrapper, 'Select');
-      const users = userList.find('option');
+      const userList = await waitForElement(wrapper, SelectNext);
+      const users = userList.find(SelectNext.Option);
       assert.equal(users.length, userEntries.length);
 
       for (const [i, entry] of userEntries.entries()) {
-        assert.equal(users.at(i).prop('value'), entry.value);
-        assert.equal(users.at(i).text(), entry.text);
+        const value = users.at(i).prop('value');
+
+        assert.equal(value.userid, entry.userid);
+        assert.equal(value.displayName, entry.displayName);
+        assert.equal(value.annotations.length, entry.annotationsCount);
       }
     });
   });
@@ -235,12 +264,14 @@ describe('ExportAnnotations', () => {
       const wrapper = createComponent();
 
       // Select the user whose annotations we want to export
-      const userList = await waitForElement(wrapper, 'Select');
-      userList.prop('onChange')({
-        target: {
-          value: 'acct:john@example.com',
-        },
-      });
+      const userList = await waitForElement(wrapper, SelectNext);
+      const option = userList
+        .find(SelectNext.Option)
+        .filterWhere(
+          option => option.prop('value').userid === 'acct:john@example.com',
+        )
+        .first();
+      userList.prop('onChange')(option.prop('value'));
       wrapper.update();
 
       submitExportForm(wrapper);