Merge branch 'main' into lp-layout-2

inferno-framework · Sep 19, 2023 · d34ec25 · d34ec25
2 parents e13f77d + c7ca4bf
commit d34ec25
Show file tree

Hide file tree

Showing 14 changed files with 108 additions and 137 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -1,3 +1,9 @@
+# 0.4.20
+* FI-2077: Improve colors and contrasts for a11y by @AlyssaWang in
+  https://github.com/inferno-framework/inferno-core/pull/392
+* Revert "FI-2035: Improve error handling for validator errors (#379)" by
+  @Jammjammjamm in https://github.com/inferno-framework/inferno-core/pull/393
+
 # 0.4.19
 * FI-2053: Fix inputs dialog overflow by @AlyssaWang in
   https://github.com/inferno-framework/inferno-core/pull/382

diff --git a/Gemfile.lock b/Gemfile.lock
@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    inferno_core (0.4.19)
+    inferno_core (0.4.20)
       activesupport (~> 6.1.7.5)
       base62-rb (= 0.3.1)
       blueprinter (= 0.25.2)
@@ -282,9 +282,9 @@ GEM
       simplecov (~> 0.19)
     simplecov-html (0.12.3)
     simplecov_json_formatter (0.1.3)
-    sqlite3 (1.6.5-arm64-darwin)
-    sqlite3 (1.6.5-x86_64-darwin)
-    sqlite3 (1.6.5-x86_64-linux)
+    sqlite3 (1.6.6-arm64-darwin)
+    sqlite3 (1.6.6-x86_64-darwin)
+    sqlite3 (1.6.6-x86_64-linux)
     strings (0.2.1)
       strings-ansi (~> 0.2)
       unicode-display_width (>= 1.5, < 3.0)

diff --git a/client/src/components/InputsModal/FieldLabel.tsx b/client/src/components/InputsModal/FieldLabel.tsx
@@ -2,12 +2,14 @@ import React, { FC } from 'react';
 import LockIcon from '@mui/icons-material/Lock';
 import { TestInput } from '~/models/testSuiteModels';
 import useStyles from './styles';
+import RequiredInputWarning from './RequiredInputWarning';
 
 export interface FieldLabelProps {
   requirement: TestInput;
+  isMissingInput?: boolean;
 }
 
-const FieldLabel: FC<FieldLabelProps> = ({ requirement }) => {
+const FieldLabel: FC<FieldLabelProps> = ({ requirement, isMissingInput = false }) => {
   const { classes } = useStyles();
 
   const fieldLabelText = requirement.title || requirement.name;
@@ -20,6 +22,7 @@ const FieldLabel: FC<FieldLabelProps> = ({ requirement }) => {
 
   return (
     <>
+      {isMissingInput && <RequiredInputWarning />}
       {fieldLabelText}
       {requiredLabel}
       {lockedIcon}

diff --git a/client/src/components/InputsModal/InputCheckboxGroup.tsx b/client/src/components/InputsModal/InputCheckboxGroup.tsx
@@ -26,6 +26,7 @@ const InputCheckboxGroup: FC<InputCheckboxGroupProps> = ({
   setInputsMap,
 }) => {
   const { classes } = useStyles();
+  const [hasBeenModified, setHasBeenModified] = React.useState(false);
 
   const [values, setValues] = React.useState<CheckboxValues>(() => {
     // Default values should be in form ['value'] where all values are checked
@@ -61,6 +62,9 @@ const InputCheckboxGroup: FC<InputCheckboxGroupProps> = ({
     return startingValues as CheckboxValues;
   });
 
+  const isMissingInput =
+    hasBeenModified && !requirement.optional && inputsMap.get(requirement.name) === '[]';
+
   useEffect(() => {
     // Make sure starting values get set in inputsMap
     inputsMap.set(requirement.name, transformValuesToJSONArray(values));
@@ -75,6 +79,7 @@ const InputCheckboxGroup: FC<InputCheckboxGroupProps> = ({
     inputsMap.set(requirement.name, transformValuesToJSONArray(newValues));
     setInputsMap(new Map(inputsMap));
     setValues(newValues);
+    setHasBeenModified(true);
   };
 
   // Convert map from item name to checked status back to array of checked values
@@ -93,11 +98,13 @@ const InputCheckboxGroup: FC<InputCheckboxGroupProps> = ({
         component="fieldset"
         id={`requirement${index}_input`}
         disabled={requirement.locked}
+        required={!requirement.optional}
+        error={isMissingInput}
         fullWidth
         className={classes.inputField}
       >
-        <FormLabel required={!requirement.optional} className={classes.inputLabel}>
-          <FieldLabel requirement={requirement} />
+        <FormLabel className={classes.inputLabel}>
+          <FieldLabel requirement={requirement} isMissingInput={isMissingInput} />
         </FormLabel>
         {requirement.description && (
           <FormHelperText sx={{ mx: 0 }}>{requirement.description}</FormHelperText>
@@ -111,6 +118,11 @@ const InputCheckboxGroup: FC<InputCheckboxGroupProps> = ({
                   color="secondary"
                   name={option.value}
                   checked={values[option.value as keyof CheckboxValues] || false}
+                  onBlur={(e) => {
+                    if (e.currentTarget === e.target) {
+                      setHasBeenModified(true);
+                    }
+                  }}
                   onChange={handleChange}
                 />
               }

diff --git a/client/src/components/InputsModal/InputOAuthCredentials.tsx b/client/src/components/InputsModal/InputOAuthCredentials.tsx
@@ -12,6 +12,7 @@ import {
 import { OAuthCredentials, TestInput } from '~/models/testSuiteModels';
 import FieldLabel from './FieldLabel';
 import useStyles from './styles';
+import RequiredInputWarning from './RequiredInputWarning';
 
 export interface InputOAuthCredentialsProps {
   requirement: TestInput;
@@ -35,22 +36,20 @@ const InputOAuthCredentials: FC<InputOAuthCredentialsProps> = ({
   setInputsMap,
 }) => {
   const { classes } = useStyles();
+  const [hasBeenModified, setHasBeenModified] = React.useState({});
 
   // Convert OAuth string to Object
   // OAuth should be an Object while in this component but should be converted to a string
   // before being updated in the inputs map
-  const oAuthCredentials = (
-    inputsMap.get(requirement.name)
-      ? JSON.parse(inputsMap.get(requirement.name) as string)
-      : {
-          access_token: '',
-          refresh_token: '',
-          expires_in: '',
-          client_id: '',
-          client_secret: '',
-          token_url: '',
-        }
-  ) as OAuthCredentials;
+  const oAuthCredentials = {
+    access_token: '',
+    refresh_token: '',
+    expires_in: '',
+    client_id: '',
+    client_secret: '',
+    token_url: '',
+    ...JSON.parse((inputsMap.get(requirement.name) as string) || '{}'),
+  } as OAuthCredentials;
 
   const showRefreshDetails = !!oAuthCredentials.refresh_token;
 
@@ -91,16 +90,32 @@ const InputOAuthCredentials: FC<InputOAuthCredentialsProps> = ({
     },
   ];
 
+  const getIsMissingInput = (field: InputOAuthField) => {
+    return (
+      hasBeenModified[field.name as keyof typeof hasBeenModified] &&
+      field.required &&
+      !oAuthCredentials[field.name as keyof OAuthCredentials]
+    );
+  };
+
   const oAuthField = (field: InputOAuthField) => {
-    const fieldLabel = field.required
+    const fieldName = field.required
       ? `${(field.label || field.name) as string} (required)`
       : field.label || field.name;
+
+    const fieldLabel = (
+      <>
+        {getIsMissingInput(field) && <RequiredInputWarning />}
+        {fieldName}
+      </>
+    );
+
     return (
       <ListItem disabled={field.locked} key={field.name}>
         <TextField
           disabled={requirement.locked}
           required={field.required}
-          error={field.required && !oAuthCredentials[field.name as keyof OAuthCredentials]}
+          error={getIsMissingInput(field)}
           id={`requirement${index}_${field.name}`}
           label={fieldLabel}
           helperText={requirement.description}
@@ -109,6 +124,11 @@ const InputOAuthCredentials: FC<InputOAuthCredentialsProps> = ({
           variant="standard"
           color="secondary"
           fullWidth
+          onBlur={(e) => {
+            if (e.currentTarget === e.target) {
+              setHasBeenModified({ ...hasBeenModified, [field.name]: true });
+            }
+          }}
           onChange={(event) => {
             const value = event.target.value;
             oAuthCredentials[field.name as keyof OAuthCredentials] = value;
@@ -129,7 +149,6 @@ const InputOAuthCredentials: FC<InputOAuthCredentialsProps> = ({
             required={!requirement.optional}
             disabled={requirement.locked}
             className={classes.inputLabel}
-            shrink
           >
             <FieldLabel requirement={requirement} />
           </InputLabel>

diff --git a/client/src/components/InputsModal/InputTextArea.tsx b/client/src/components/InputsModal/InputTextArea.tsx
@@ -16,27 +16,34 @@ const InputTextArea: FC<InputTextAreaProps> = ({ requirement, index, inputsMap,
   const { classes } = useStyles();
   const [hasBeenModified, setHasBeenModified] = React.useState(false);
 
+  const isMissingInput =
+    hasBeenModified && !requirement.optional && !inputsMap.get(requirement.name);
+
   return (
     <ListItem>
       <TextField
         disabled={requirement.locked}
         required={!requirement.optional}
-        error={hasBeenModified && !requirement.optional && !inputsMap.get(requirement.name)}
+        error={isMissingInput}
         id={`requirement${index}_input`}
         className={classes.inputField}
         variant="standard"
         color="secondary"
         fullWidth
-        label={<FieldLabel requirement={requirement} />}
+        label={<FieldLabel requirement={requirement} isMissingInput={isMissingInput} />}
         helperText={requirement.description}
         value={inputsMap.get(requirement.name)}
         multiline
         rows={4}
+        onBlur={(e) => {
+          if (e.currentTarget === e.target) {
+            setHasBeenModified(true);
+          }
+        }}
         onChange={(event) => {
           const value = event.target.value;
           inputsMap.set(requirement.name, value);
           setInputsMap(new Map(inputsMap));
-          setHasBeenModified(true);
         }}
         FormHelperTextProps={{
           sx: { '&.Mui-disabled': { color: lightTheme.palette.common.grayDark } },

diff --git a/client/src/components/InputsModal/InputTextField.tsx b/client/src/components/InputsModal/InputTextField.tsx
@@ -21,25 +21,32 @@ const InputTextField: FC<InputTextFieldProps> = ({
   const { classes } = useStyles();
   const [hasBeenModified, setHasBeenModified] = React.useState(false);
 
+  const isMissingInput =
+    hasBeenModified && !requirement.optional && !inputsMap.get(requirement.name);
+
   return (
     <ListItem>
       <TextField
         disabled={requirement.locked}
         required={!requirement.optional}
-        error={hasBeenModified && !requirement.optional && !inputsMap.get(requirement.name)}
+        error={isMissingInput}
         id={`requirement${index}_input`}
         className={classes.inputField}
         variant="standard"
         color="secondary"
         fullWidth
-        label={<FieldLabel requirement={requirement} />}
+        label={<FieldLabel requirement={requirement} isMissingInput={isMissingInput} />}
         helperText={requirement.description}
         value={inputsMap.get(requirement.name)}
+        onBlur={(e) => {
+          if (e.currentTarget === e.target) {
+            setHasBeenModified(true);
+          }
+        }}
         onChange={(event) => {
           const value = event.target.value;
           inputsMap.set(requirement.name, value);
           setInputsMap(new Map(inputsMap));
-          setHasBeenModified(true);
         }}
         InputLabelProps={{ shrink: true }}
         FormHelperTextProps={{

diff --git a/client/src/components/InputsModal/InputsModal.tsx b/client/src/components/InputsModal/InputsModal.tsx
@@ -98,8 +98,7 @@ const InputsModal: FC<InputsModalProps> = ({
         ) as OAuthCredentials;
         const accessTokenIsEmpty = oAuthJSON.access_token === '';
         const refreshIsEmpty =
-          oAuthJSON.refresh_token !== '' &&
-          (oAuthJSON.token_url === '' || oAuthJSON.client_id === '');
+          oAuthJSON.refresh_token !== '' && (!oAuthJSON.token_url || !oAuthJSON.client_id);
         oAuthMissingRequiredInput = (accessTokenIsEmpty && !input.optional) || refreshIsEmpty;
       } catch (e: unknown) {
         const errorMessage = e instanceof Error ? e.message : String(e);
@@ -281,8 +280,9 @@ const InputsModal: FC<InputsModalProps> = ({
     const parsedChanges = parseSerialChanges(serialChanges);
     if (parsedChanges !== undefined && parsedChanges.keys !== undefined) {
       parsedChanges.forEach((change: TestInput) => {
-        if (!change.locked && change.value !== undefined)
+        if (!change.locked && change.value !== undefined) {
           inputsMap.set(change.name, change.value || '');
+        }
       });
     }
     handleSetInputsMap(new Map(inputsMap), true);

diff --git a/client/src/components/InputsModal/RequiredInputWarning.tsx b/client/src/components/InputsModal/RequiredInputWarning.tsx
@@ -1,11 +1,19 @@
 import React, { FC } from 'react';
-import WarningIcon from '@mui/icons-material/Warning';
+import { Report } from '@mui/icons-material';
 import CustomTooltip from '~/components/_common/CustomTooltip';
 
 const RequiredInputWarning: FC = () => {
   return (
     <CustomTooltip title="Missing value for required input">
-      <WarningIcon fontSize="small" color="error" />
+      <Report
+        color="error"
+        aria-hidden={false}
+        tabIndex={0}
+        sx={{
+          marginRight: '4px',
+          verticalAlign: 'bottom',
+        }}
+      />
     </CustomTooltip>
   );
 };

diff --git a/client/src/components/InputsModal/styles.tsx b/client/src/components/InputsModal/styles.tsx
@@ -23,9 +23,8 @@ export default makeStyles()((theme: Theme) => ({
     },
   },
   inputLabel: {
-    color: theme.palette.common.grayDarkest,
+    color: theme.palette.common.grayDarker,
     fontWeight: 600,
-    fontSize: '.75rem',
   },
   lockedIcon: {
     marginLeft: '5px',