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.1",
     "@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,22 @@ import {
   CardActions,
   Link,
   Input,
-  Select,
+  SelectNext,
 } from '@hypothesis/frontend-shared';
 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 { UserAnnotationsListItem } from './UserAnnotationsListItem';
 
 export type ExportAnnotationsProps = {
   // injected
@@ -51,9 +53,20 @@ 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: Omit<UserAnnotations, 'userid'> = useMemo(
+    () => ({
+      annotations: exportableAnnotations,
+      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 +89,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 +144,27 @@ 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}
+            buttonId="export-user"
+            buttonContent={
+              <UserAnnotationsListItem userAnnotations={selectedUser} />
             }
           >
-            <option value="" selected={!selectedUser}>
-              All annotations ({exportableAnnotations.length})
-            </option>
+            <SelectNext.Option value={allAnnotationsOption}>
+              {() => (
+                <UserAnnotationsListItem
+                  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}>
+                {() => <UserAnnotationsListItem userAnnotations={userInfo} />}
+              </SelectNext.Option>
             ))}
-          </Select>
+          </SelectNext>
         </>
       ) : (
         <p data-testid="no-annotations-message">

src/sidebar/components/ShareDialog/ImportAnnotations.tsx

@@ -1,4 +1,9 @@
-import { Button, CardActions, Link, Select } from '@hypothesis/frontend-shared';
+import {
+  Button,
+  CardActions,
+  Link,
+  SelectNext,
+} from '@hypothesis/frontend-shared';
 import { useCallback, useEffect, useId, useMemo, useState } from 'preact/hooks';
 
 import type { APIAnnotationData } from '../../../types/api';
@@ -10,6 +15,7 @@ import type { ImportAnnotationsService } from '../../services/import-annotations
 import { useSidebarStore } from '../../store';
 import CircularProgress from '../CircularProgress';
 import FileInput from './FileInput';
+import { UserAnnotationsListItem } from './UserAnnotationsListItem';
 
 export type ImportAnnotationsProps = {
   importAnnotationsService: ImportAnnotationsService;
@@ -32,9 +38,6 @@ 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 store = useSidebarStore();
   const currentUser = store.profile().userid;
 
@@ -59,45 +62,45 @@ function ImportAnnotations({
     [annotations, getDisplayName],
   );
 
+  // User whose annotations are going to be imported.
+  const [selectedUserId, setSelectedUserId] = useState<string | null>(null);
+  const selectedUser = useMemo(
+    () => userList?.find(user => user.userid === selectedUserId) ?? null,
+    [selectedUserId, userList],
+  );
+
   // Parse input file, extract annotations and update the user list.
   useEffect(() => {
     if (!currentUser || !file) {
       return;
     }
     setAnnotations(null);
     setError(null);
-    setSelectedUser(null);
+    setSelectedUserId(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);
+        setSelectedUserId(userMatch ? currentUser : null);
       })
       .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 +159,27 @@ function ImportAnnotations({
       <FileInput onFileSelected={setFile} disabled={busy} id={fileInputId} />
       {userList && (
         <>
-          <label htmlFor={userSelectId} className="block font-medium">
+          <label htmlFor="import-annotations" 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)
+          <SelectNext
+            value={selectedUser}
+            onChange={newValue => setSelectedUserId(newValue?.userid ?? null)}
+            buttonId="import-annotations"
+            buttonContent={
+              selectedUser ? (
+                <UserAnnotationsListItem userAnnotations={selectedUser} />
+              ) : (
+                <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}>
+                {() => <UserAnnotationsListItem userAnnotations={userInfo} />}
+              </SelectNext.Option>
             ))}
-          </Select>
+          </SelectNext>
         </>
       )}
       {error && (

src/sidebar/components/ShareDialog/UserAnnotationsListItem.tsx

@@ -0,0 +1,21 @@
+import type { UserAnnotations } from '../../helpers/annotations-by-user';
+
+export type UserAnnotationsListItemProps = {
+  userAnnotations: Omit<UserAnnotations, 'userid'>;
+};
+
+/**
+ * UserAnnotations representation to use inside `SelectNext.Option`.
+ */
+export function UserAnnotationsListItem({
+  userAnnotations,
+}: UserAnnotationsListItemProps) {
+  return (
+    <div className="flex gap-x-2">
+      {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,4 +1,6 @@
+import { SelectNext } from '@hypothesis/frontend-shared';
 import { mount } from 'enzyme';
+import { act } from 'preact/test-utils';
 
 import { checkAccessibility } from '../../../../test-util/accessibility';
 import { mockImportedComponents } from '../../../../test-util/mock-imported-components';
@@ -64,9 +66,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 UserAnnotationsListItem, as it's used as some buttons' content
+      // and can make a11y tests fail
+      './UserAnnotationsListItem': true,
     });
   });
 
@@ -124,9 +129,21 @@ 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
+          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,16 @@ 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
+          displayName: 'All annotations',
+          annotationsCount: 1,
+        },
+        {
+          userid: 'acct:brian@example.com',
+          displayName: 'Brian Smith',
+          annotationsCount: 1,
+        },
       ],
     },
   ].forEach(({ annotations, userEntries }) => {
@@ -154,13 +179,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,11 +263,15 @@ 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();
+      act(() => {
+        userList.prop('onChange')(option.prop('value'));
       });
       wrapper.update();