Use SelectNext for StudentSelector

hypothesis · Oct 20, 2023 · c12dca4 · c12dca4
1 parent 1a26d56
commit c12dca4
Show file tree

Hide file tree

Showing 5 changed files with 47 additions and 50 deletions.
diff --git a/lms/static/scripts/frontend_apps/components/StudentSelector.tsx b/lms/static/scripts/frontend_apps/components/StudentSelector.tsx
@@ -3,7 +3,7 @@ import {
   ArrowRightIcon,
   IconButton,
   InputGroup,
-  Select,
+  SelectNext,
 } from '@hypothesis/frontend-shared';
 
 import type { StudentInfo } from '../config';
@@ -47,15 +47,6 @@ export default function StudentSelector<Student extends StudentOption>({
     onSelectStudent(selectedIndex === 0 ? null : students[selectedIndex - 1]);
   };
 
-  const handleSelectStudent = (e: Event) => {
-    const studentIndex = parseInt((e.target as HTMLInputElement).value);
-    if (studentIndex === -1) {
-      onSelectStudent(null);
-    } else {
-      onSelectStudent(students[studentIndex]);
-    }
-  };
-
   return (
     <>
       <label
@@ -86,29 +77,23 @@ export default function StudentSelector<Student extends StudentOption>({
             title="Previous student"
             variant="dark"
           />
-          <Select
-            classes="min-w-[12rem] xl:w-[20rem]"
-            aria-label="Select student"
-            id={selectId}
-            onChange={handleSelectStudent}
+          <label className="sr-only" htmlFor={selectId}>
+            {selectedStudent?.displayName} Select student
+          </label>
+          <SelectNext
+            classes="md:!w-[12rem] lg:!w-[16rem] xl:!w-[20rem]"
+            value={selectedStudent}
+            onChange={onSelectStudent}
+            buttonId={selectId}
+            buttonContent={selectedStudent?.displayName ?? 'All Students'}
           >
-            <option
-              key={'student-all'}
-              selected={!hasSelectedStudent}
-              value={-1}
-            >
-              All Students
-            </option>
+            <SelectNext.Option value={null}>All Students</SelectNext.Option>
             {students.map((studentOption, idx) => (
-              <option
-                key={`student-${idx}`}
-                selected={selectedIndex === idx}
-                value={idx}
-              >
-                {studentOption.displayName}
-              </option>
+              <SelectNext.Option value={studentOption} key={`student-${idx}`}>
+                studentOption.displayName
+              </SelectNext.Option>
             ))}
-          </Select>
+          </SelectNext>
           <IconButton
             data-testid="next-student-button"
             disabled={selectedIndex >= students.length - 1}

diff --git a/lms/static/scripts/frontend_apps/components/test/StudentSelector-test.js b/lms/static/scripts/frontend_apps/components/test/StudentSelector-test.js
@@ -41,7 +41,10 @@ describe('StudentSelector', () => {
 
   it('shall have "All Students" as the default option', () => {
     const wrapper = renderSelector({ selectedStudent: null });
-    assert.equal(wrapper.find('select [selected=true]').text(), 'All Students');
+    assert.equal(
+      wrapper.find('SelectNext').prop('buttonContent'),
+      'All Students'
+    );
     assert.equal(
       wrapper.find('[data-testid="student-selector-label"]').text(),
       '2 Students'
@@ -50,7 +53,7 @@ describe('StudentSelector', () => {
 
   it('sets the selected option to the reflect the selected student', () => {
     const wrapper = renderSelector({ selectedStudent: fakeStudents[1] });
-    assert.equal(wrapper.find('select [selected=true]').text(), 'Student 2');
+    assert.equal(wrapper.find('SelectNext').prop('buttonContent'), 'Student 2');
     assert.equal(
       wrapper.find('[data-testid="student-selector-label"]').text(),
       'Student 2 of 2'
@@ -63,17 +66,18 @@ describe('StudentSelector', () => {
       onSelectStudent: onChange,
       selectedStudent: fakeStudents[0],
     });
-    wrapper.find('select').simulate('change');
-    assert.isTrue(onChange.called);
+    wrapper.find('SelectNext').props().onChange(fakeStudents[1]);
+    assert.calledWith(onChange, fakeStudents[1]);
   });
 
   it('unsets the selected user when the "All Students" option is selected', () => {
     const onChange = sinon.spy();
     const wrapper = renderSelector({
-      // No student is selected
       onSelectStudent: onChange,
+      selectedStudent: fakeStudents[0],
     });
-    wrapper.find('select').simulate('change');
+    // No student is selected
+    wrapper.find('SelectNext').props().onChange(null);
     assert.calledWith(onChange, null);
   });
 

diff --git a/lms/static/scripts/ui-playground/components/ToolbarPage.tsx b/lms/static/scripts/ui-playground/components/ToolbarPage.tsx
@@ -9,7 +9,7 @@ import {
   InputGroup,
   LinkButton,
   ModalDialog,
-  Select,
+  SelectNext,
 } from '@hypothesis/frontend-shared';
 import type { ModalProps } from '@hypothesis/frontend-shared/lib/components/feedback/Modal';
 import Library from '@hypothesis/frontend-shared/lib/pattern-library/components/Library';
@@ -125,15 +125,23 @@ export default function ToolbarPage() {
                               title="previous student"
                               variant="dark"
                             />
-                            <Select
-                              aria-label="Select student"
-                              classes="xl:w-80"
-                              id="student-selector"
+                            <label
+                              className="sr-only"
+                              htmlFor="student-selector"
                             >
-                              <option key={'all-students'} value={-1}>
+                              Select student
+                            </label>
+                            <SelectNext
+                              classes="xl:!w-80"
+                              value={-1}
+                              onChange={() => {}}
+                              buttonId="student-selector"
+                              buttonContent="All Students"
+                            >
+                              <SelectNext.Option value={-1}>
                                 All Students
-                              </option>
-                            </Select>
+                              </SelectNext.Option>
+                            </SelectNext>
                             <IconButton
                               icon={ArrowRightIcon}
                               title="next student"

diff --git a/package.json b/package.json
@@ -25,7 +25,7 @@
     "@babel/preset-react": "^7.22.15",
     "@babel/preset-typescript": "^7.23.0",
     "@hypothesis/frontend-build": "^2.2.0",
-    "@hypothesis/frontend-shared": "^6.8.0",
+    "@hypothesis/frontend-shared": "^6.9.0",
     "@rollup/plugin-babel": "^6.0.3",
     "@rollup/plugin-commonjs": "^25.0.4",
     "@rollup/plugin-node-resolve": "^15.2.1",

diff --git a/yarn.lock b/yarn.lock
@@ -1861,15 +1861,15 @@ __metadata:
   languageName: node
   linkType: hard
 
-"@hypothesis/frontend-shared@npm:^6.8.0":
-  version: 6.8.0
-  resolution: "@hypothesis/frontend-shared@npm:6.8.0"
+"@hypothesis/frontend-shared@npm:^6.9.0":
+  version: 6.9.0
+  resolution: "@hypothesis/frontend-shared@npm:6.9.0"
   dependencies:
     highlight.js: ^11.6.0
     wouter-preact: ^2.10.0-alpha.1
   peerDependencies:
     preact: ^10.4.0
-  checksum: e2981d395929d9ddd5b6b78f35b4aebe5422ebbdaa48f6e65684cb1514534186e98559b2aadbaad77f3dc02652b73128c5bf3714d86b14f4104d5162d9d79466
+  checksum: e8fe4a40a7502476f7eb16957803158084e140b8dc95b0ff54aae25cf02a2c780953e42c776715f7eddf8bc5e1119223b5aeaa070ae65c194ba38b06dfff650a
   languageName: node
   linkType: hard
 
@@ -7386,7 +7386,7 @@ __metadata:
     "@babel/preset-react": ^7.22.15
     "@babel/preset-typescript": ^7.23.0
     "@hypothesis/frontend-build": ^2.2.0
-    "@hypothesis/frontend-shared": ^6.8.0
+    "@hypothesis/frontend-shared": ^6.9.0
     "@hypothesis/frontend-testing": ^1.0.1
     "@rollup/plugin-babel": ^6.0.3
     "@rollup/plugin-commonjs": ^25.0.4