Improve grade validation accessibility using HTML5 attributes (#5711)

hypothesis · Oct 16, 2023 · 0605845 · 0605845
1 parent f1a66c6
commit 0605845
Show file tree

Hide file tree

Showing 9 changed files with 155 additions and 326 deletions.
diff --git a/lms/static/scripts/frontend_apps/components/GradingCommentButton.tsx b/lms/static/scripts/frontend_apps/components/GradingCommentButton.tsx
@@ -1,4 +1,4 @@
-import type { ButtonProps, TextareaProps } from '@hypothesis/frontend-shared';
+import type { TextareaProps } from '@hypothesis/frontend-shared';
 import {
   Button,
   CancelIcon,
@@ -15,7 +15,8 @@ import { useCallback, useId, useRef, useState } from 'preact/hooks';
 type CommentPopoverProps = {
   comment: string;
   onInput: NonNullable<TextareaProps['onInput']>;
-  onSubmit: NonNullable<ButtonProps['onClick']>;
+  /** @return True if the form is valid */
+  onSubmit: () => boolean;
   closePopover: () => void;
 };
 
@@ -73,9 +74,10 @@ function CommentPopover({
       <div className="flex flex-row-reverse space-x-2 space-x-reverse mt-3">
         <Button
           variant="primary"
-          onClick={e => {
-            onSubmit(e);
-            closePopover();
+          onClick={() => {
+            if (onSubmit()) {
+              closePopover();
+            }
           }}
           data-testid="comment-submit-button"
         >
@@ -98,7 +100,7 @@ export type GradingCommentProps = {
   loading: boolean;
   comment: string;
   onInput: NonNullable<TextareaProps['onInput']>;
-  onSubmit: NonNullable<ButtonProps['onClick']>;
+  onSubmit: () => boolean;
 };
 
 /**

diff --git a/lms/static/scripts/frontend_apps/components/SubmitGradeForm.tsx b/lms/static/scripts/frontend_apps/components/SubmitGradeForm.tsx
@@ -19,12 +19,11 @@ import type { StudentInfo } from '../config';
 import type { ErrorLike } from '../errors';
 import { useService, GradingService } from '../services';
 import { useFetch } from '../utils/fetch';
-import { formatGrade, validateGrade } from '../utils/grade-validation';
+import { formatGrade } from '../utils/grade-validation';
 import { useUniqueId } from '../utils/hooks';
 import { useWarnOnPageUnload } from '../utils/use-warn-on-page-unload';
 import ErrorModal from './ErrorModal';
 import GradingCommentButton from './GradingCommentButton';
-import ValidationMessage from './ValidationMessage';
 
 export type SubmitGradeFormProps = {
   student: StudentInfo | null;
@@ -113,17 +112,12 @@ export default function SubmitGradeForm({
 
   const disabled = !student;
 
-  // The following is state for local validation errors
-  //
-  // Is there a validation error message to show?
-  const [showValidationError, setValidationError] = useState(false);
-  // The actual validation error message.
-  const [validationMessage, setValidationMessageMessage] = useState('');
   // Unique id attribute for <input>
   const gradeId = useUniqueId('SubmitGradeForm__grade:');
 
   // Used to handle keyboard input changes for the grade input field.
   const inputRef = useRef<HTMLInputElement | null>(null);
+  const formRef = useRef<HTMLFormElement | null>(null);
 
   // This is used to track unsaved grades or comments. It is null until user input occurs.
   const [draftGrading, setDraftGrading] = useState<DraftGrading>({
@@ -167,36 +161,46 @@ export default function SubmitGradeForm({
     event.preventDefault();
 
     const newGrade = inputRef.current!.value;
+    const newGradeAsNumber = inputRef.current!.valueAsNumber;
+    if (isNaN(newGradeAsNumber)) {
+      // This branch should not be reached because input type is number, and we validate before submission.
+      throw new Error(`New grade "${newGrade}" is not a number`);
+    }
+
     const newComment = acceptComments
       ? draftGrading.comment ?? grade.data?.comment
       : undefined;
-    const result = validateGrade(newGrade, scoreMaximum);
-
-    if (!result.valid) {
-      setValidationMessageMessage(result.error);
-      setValidationError(true);
-    } else {
-      setGradeSaving(true);
-      try {
-        await gradingService.submitGrade({
-          student: student as StudentInfo,
-          grade: result.grade,
-          comment: newComment ?? undefined,
-        });
-        grade.mutate({ grade: newGrade, comment: newComment });
-        onUnsavedChanges?.(false);
-        setGradeSaved(true);
-      } catch (e) {
-        setSubmitGradeError(e);
-      }
-      setGradeSaving(false);
+
+    setGradeSaving(true);
+    try {
+      await gradingService.submitGrade({
+        student: student as StudentInfo,
+        grade: newGradeAsNumber / scoreMaximum,
+        comment: newComment ?? undefined,
+      });
+      grade.mutate({ grade: newGrade, comment: newComment });
+      onUnsavedChanges?.(false);
+      setGradeSaved(true);
+    } catch (e) {
+      setSubmitGradeError(e);
     }
+    setGradeSaving(false);
   };
 
+  const submitGrade = useCallback(() => {
+    const form = formRef.current;
+
+    // Checks if the form is valid, and display native validation popups if needed
+    if (!form || !form.reportValidity()) {
+      return false;
+    }
+
+    form.requestSubmit();
+    return true;
+  }, []);
+
   const handleInput = useCallback(
     (e: Event, field: keyof DraftGrading) => {
-      // If any input is detected, close the ValidationMessage.
-      setValidationError(false);
       setGradeSaved(false);
 
       const newValue = (e.target as HTMLInputElement).value;
@@ -207,24 +211,16 @@ export default function SubmitGradeForm({
 
   return (
     <>
-      <form autoComplete="off">
+      <form autoComplete="off" onSubmit={onSubmitGrade} ref={formRef}>
         <label htmlFor={gradeId} className="font-semibold text-xs">
           Grade (Out of {scoreMaximum})
         </label>
         <div className="flex">
           <span className="relative w-14">
-            {validationMessage && (
-              <ValidationMessage
-                message={validationMessage}
-                open={showValidationError}
-                onClose={() => {
-                  // Sync up the state when the ValidationMessage is closed
-                  setValidationError(false);
-                }}
-              />
-            )}
             <Input
               classes={classnames(
+                // Hide up/down arrow buttons (https://stackoverflow.com/a/75872055)
+                '[appearance:textfield] [&::-webkit-outer-spin-button]:appearance-none [&::-webkit-inner-spin-button]:appearance-none',
                 'text-center',
                 'disabled:opacity-50',
                 'border border-r-0 rounded-r-none',
@@ -237,9 +233,13 @@ export default function SubmitGradeForm({
               id={gradeId}
               elementRef={inputRef}
               onInput={e => handleInput(e, 'grade')}
-              type="text"
+              type="number"
               value={draftGrading.grade ?? grade.data?.grade ?? ''}
               key={student ? student.LISResultSourcedId : null}
+              min={0}
+              max={scoreMaximum}
+              step="any"
+              required
             />
             {grade.isLoading && (
               <div className="absolute top-1/2 left-1/2 -translate-x-1/2 -translate-y-1/2">
@@ -254,19 +254,19 @@ export default function SubmitGradeForm({
               loading={grade.isLoading}
               comment={draftGrading.comment ?? grade.data?.comment ?? ''}
               onInput={e => handleInput(e, 'comment')}
-              onSubmit={onSubmitGrade}
+              onSubmit={submitGrade}
             />
           )}
 
           <Button
             icon={CheckIcon}
             type="submit"
+            data-testid="submit-button"
             classes={classnames(
               'border rounded-l-none ring-inset',
               'disabled:opacity-50'
             )}
             disabled={disabled}
-            onClick={onSubmitGrade}
           >
             Submit Grade
           </Button>

diff --git a/lms/static/scripts/frontend_apps/components/ValidationMessage.tsx b/lms/static/scripts/frontend_apps/components/ValidationMessage.tsx
diff --git a/lms/static/scripts/frontend_apps/components/test/GradingCommentButton-test.js b/lms/static/scripts/frontend_apps/components/test/GradingCommentButton-test.js
@@ -108,16 +108,31 @@ describe('GradingCommentButton', () => {
     }
   );
 
-  it('submits grade using internal popover submit button', async () => {
-    const onSubmit = sinon.stub();
-    const wrapper = renderComponent({ onSubmit });
+  context('when clicking internal popover submit button', () => {
+    const clickSubmit = wrapper =>
+      wrapper
+        .find('button[data-testid="comment-submit-button"]')
+        .simulate('click');
 
-    togglePopover(wrapper);
+    it('submits grade', async () => {
+      const onSubmit = sinon.stub();
+      const wrapper = renderComponent({ onSubmit });
+
+      togglePopover(wrapper);
+
+      clickSubmit(wrapper);
+      assert.called(onSubmit);
+    });
 
-    wrapper
-      .find('button[data-testid="comment-submit-button"]')
-      .simulate('click');
-    assert.called(onSubmit);
+    it('closes popover after successfully submitting', () => {
+      const onSubmit = sinon.stub().returns(true);
+      const wrapper = renderComponent({ onSubmit });
+
+      togglePopover(wrapper);
+
+      clickSubmit(wrapper);
+      assert.isFalse(commentPopoverExists(wrapper));
+    });
   });
 
   it('updates comment on textarea input', async () => {