Moving ErrorReport component to the bottom of the form (#346)

Co-authored-by: Håkon <[email protected]> Co-authored-by: Ole Martin Handeland <[email protected]>
Altinn · Jul 28, 2022 · cb830f2 · cb830f2
1 parent a5f1f8f
commit cb830f2
Show file tree

Hide file tree

Showing 46 changed files with 910 additions and 526 deletions.
diff --git a/src/.eslintrc.json b/src/.eslintrc.json
@@ -59,7 +59,7 @@
     "import/no-unresolved": ["off"],
     "@typescript-eslint/no-unused-vars": [
       "error",
-      { "ignoreRestSiblings": true }
+      { "ignoreRestSiblings": true, "argsIgnorePattern": "^_" }
     ],
     "@typescript-eslint/consistent-type-imports": ["warn"],
     "react-hooks/exhaustive-deps": ["error"],

diff --git a/src/altinn-app-frontend/package.json b/src/altinn-app-frontend/package.json
@@ -18,7 +18,7 @@
   "author": "Altinn",
   "license": "3-Clause BSD",
   "dependencies": {
-    "@altinn/altinn-design-system": "0.1.4",
+    "@altinn/altinn-design-system": "^0.2.0",
     "@babel/polyfill": "^7.12.1",
     "@date-io/moment": "1.3.13",
     "@material-ui/core": "^4.12.4",

diff --git a/src/altinn-app-frontend/src/components/GenericComponent.tsx b/src/altinn-app-frontend/src/components/GenericComponent.tsx
@@ -101,6 +101,7 @@ export function GenericComponent<Type extends ComponentExceptGroup>(
   const { id, ...passThroughProps } = props;
   const dispatch = useAppDispatch();
   const classes = useStyles(props);
+  const gridRef = React.useRef<HTMLDivElement>();
   const GetHiddenSelector = makeGetHidden();
   const GetFocusSelector = makeGetFocus();
   const [hasValidationMessages, setHasValidationMessages] =
@@ -154,6 +155,20 @@ export function GenericComponent<Type extends ComponentExceptGroup>(
     );
   }, [componentValidations]);
 
+  React.useLayoutEffect(() => {
+    if (!hidden && shouldFocus && gridRef.current) {
+      gridRef.current.scrollIntoView();
+
+      const maybeInput = gridRef.current.querySelector(
+        'input,textarea,select',
+      ) as HTMLSelectElement | HTMLInputElement | HTMLTextAreaElement;
+      if (maybeInput) {
+        maybeInput.focus();
+      }
+      dispatch(FormLayoutActions.updateFocus({ focusComponentId: null }));
+    }
+  }, [shouldFocus, hidden, dispatch]);
+
   if (hidden) {
     return null;
   }
@@ -199,15 +214,6 @@ export function GenericComponent<Type extends ComponentExceptGroup>(
     );
   };
 
-  const handleFocusUpdate = (componentId: string, step?: number) => {
-    dispatch(
-      FormLayoutActions.updateFocus({
-        currentComponentId: componentId,
-        step: step || 0,
-      }),
-    );
-  };
-
   const getValidationsForInternalHandling = () => {
     if (
       props.type === 'AddressComponent' ||
@@ -288,7 +294,6 @@ export function GenericComponent<Type extends ComponentExceptGroup>(
 
   const componentProps = {
     handleDataChange,
-    handleFocusUpdate,
     getTextResource: getTextResourceWrapper,
     getTextResourceAsString,
     formData,
@@ -332,6 +337,7 @@ export function GenericComponent<Type extends ComponentExceptGroup>(
   return (
     <FormComponentContext.Provider value={formComponentContext}>
       <Grid
+        ref={gridRef}
         item={true}
         container={true}
         {...gridBreakpoints(props.grid)}

diff --git a/src/altinn-app-frontend/src/components/base/CheckboxesContainerComponent.md b/src/altinn-app-frontend/src/components/base/CheckboxesContainerComponent.md
@@ -1,10 +1,12 @@
 Simple checkbox with 3 options
 
 ```tsx
-import {CheckboxContainerComponent} from './CheckboxesContainerComponent';
+import { CheckboxContainerComponent } from './CheckboxesContainerComponent';
 import Legend from '../../features/form/components/Legend';
-import {nb} from '../../shared/resources/language/texts/nb';
-const dummyFunc = () => {return;}
+import { nb } from '../../shared/resources/language/texts/nb';
+const dummyFunc = () => {
+  return;
+};
 
 const legend = () => {
   return (
@@ -15,13 +17,12 @@ const legend = () => {
       helpTextProps={{}}
     />
   );
-}
+};
 
 const props = {
   id: 'simpleCheckbox',
   formData: '',
   handleDataChange: dummyFunc,
-  handleFocusUpdate: dummyFunc,
   isValid: true,
   validationMessages: {},
   options: [
@@ -44,9 +45,5 @@ const props = {
   legend,
 };
 
-<CheckboxContainerComponent
-  {...props}
-/>
-
+<CheckboxContainerComponent {...props} />;
 ```
-
diff --git a/src/altinn-app-frontend/src/components/base/CheckboxesContainerComponent.test.tsx b/src/altinn-app-frontend/src/components/base/CheckboxesContainerComponent.test.tsx
@@ -42,7 +42,6 @@ const render = (
     validationMessages: {},
     legend: 'legend',
     handleDataChange: jest.fn(),
-    handleFocusUpdate: jest.fn(),
     getTextResource: (value) => value,
     getTextResourceAsString: (value) => value,
     ...({} as IComponentProps),

diff --git a/src/altinn-app-frontend/src/components/base/CheckboxesContainerComponent.tsx b/src/altinn-app-frontend/src/components/base/CheckboxesContainerComponent.tsx
@@ -85,7 +85,6 @@ export const CheckboxContainerComponent = ({
   formData,
   preselectedOptionIndex,
   handleDataChange,
-  handleFocusUpdate,
   layout,
   legend,
   getTextResourceAsString,
@@ -146,7 +145,6 @@ export const CheckboxContainerComponent = ({
     } else {
       handleDataChange(selected.concat(clickedItem).join(','));
     }
-    handleFocusUpdate(id);
   };
 
   const handleBlur = () => {

diff --git a/src/altinn-app-frontend/src/components/base/RadioButtons/ControlledRadioGroup.tsx b/src/altinn-app-frontend/src/components/base/RadioButtons/ControlledRadioGroup.tsx
@@ -28,7 +28,6 @@ export const ControlledRadioGroup = ({
   id,
   layout,
   legend,
-  shouldFocus,
   getTextResource,
   validationMessages,
   fetchingOptions,
@@ -66,11 +65,7 @@ export const ControlledRadioGroup = ({
           {calculatedOptions.map((option: any, index: number) => (
             <React.Fragment key={index}>
               <FormControlLabel
-                control={
-                  <StyledRadio
-                    autoFocus={shouldFocus && selected === option.value}
-                  />
-                }
+                control={<StyledRadio />}
                 label={getTextResource(option.label)}
                 value={option.value}
                 classes={{ root: cn(classes.margin) }}

diff --git a/...inn-app-frontend/src/components/base/RadioButtons/RadioButtonsContainerComponent.test.tsx b/...inn-app-frontend/src/components/base/RadioButtons/RadioButtonsContainerComponent.test.tsx
@@ -41,7 +41,6 @@ const render = (
     preselectedOptionIndex: undefined,
     legend: 'legend',
     handleDataChange: jest.fn(),
-    handleFocusUpdate: jest.fn(),
     getTextResource: (value) => value,
     ...({} as IComponentProps),
     ...props,

diff --git a/src/altinn-app-frontend/src/components/base/RadioButtons/radioButtonsUtils.ts b/src/altinn-app-frontend/src/components/base/RadioButtons/radioButtonsUtils.ts
@@ -54,10 +54,8 @@ export const useRadioStyles = makeStyles((theme) => ({
 }));
 
 export const useRadioButtons = ({
-  id,
   optionsId,
   options,
-  handleFocusUpdate,
   handleDataChange,
   preselectedOptionIndex,
   formData,
@@ -103,7 +101,6 @@ export const useRadioButtons = ({
   }, [handleDataChange, optionsHasChanged, formData]);
 
   const handleChange = (event: React.ChangeEvent<HTMLInputElement>) => {
-    handleFocusUpdate(id);
     handleDataChange(event.target.value);
   };
 

diff --git a/src/altinn-app-frontend/src/components/custom/CustomWebComponent.test.tsx b/src/altinn-app-frontend/src/components/custom/CustomWebComponent.test.tsx
@@ -39,7 +39,6 @@ describe('CustomWebComponent', () => {
         title: 'Title',
       },
       handleDataChange: (value: string) => value,
-      handleFocusUpdate: jest.fn(),
       getTextResource: (key: string) => {
         return key;
       },

diff --git a/src/altinn-app-frontend/src/components/index.ts b/src/altinn-app-frontend/src/components/index.ts
@@ -63,7 +63,6 @@ export interface IComponentProps extends IGenericComponentProps {
     skipValidation?: boolean,
     checkIfRequired?: boolean,
   ) => void;
-  handleFocusUpdate: (componentId: string, step?: number) => void;
   getTextResource: (key: string) => React.ReactNode;
   getTextResourceAsString: (key: string) => string;
   language: ILanguage;

diff --git a/src/altinn-app-frontend/src/components/message/ErrorReport.test.tsx b/src/altinn-app-frontend/src/components/message/ErrorReport.test.tsx
@@ -11,11 +11,6 @@ import type { IValidations } from 'src/types';
 import { getParsedLanguageFromText } from 'altinn-shared/utils';
 
 describe('ErrorReport', () => {
-  const genericErrorText =
-    getInitialStateMock().language.language.form_filler[
-      'error_report_description'
-    ];
-
   const render = (validations: Partial<IValidations>) => {
     const mockValidationState: IValidationState = {
       validations: {
@@ -29,27 +24,17 @@ describe('ErrorReport', () => {
       formValidations: mockValidationState,
     });
 
-    return renderWithProviders(<ErrorReport />, {
+    return renderWithProviders(<ErrorReport components={[]} />, {
       preloadedState: initialState,
     });
   };
 
-  it('should render generic error message by default', () => {
-    const validations = {
-      page1: {
-        someComponent: {
-          simpleBinding: {
-            errors: [getParsedLanguageFromText('some error')],
-          },
-        },
-      },
-    };
-    render(validations);
-
-    expect(screen.getByText(genericErrorText)).toBeInTheDocument();
+  it('should not render when there are no errors', () => {
+    render({});
+    expect(screen.queryByTestId('ErrorReport')).not.toBeInTheDocument();
   });
 
-  it('should list unmapped errors if present and hide generic error message', () => {
+  it('should list unmapped errors as unclickable', () => {
     const validations = {
       unmapped: {
         // unmapped layout
@@ -64,8 +49,31 @@ describe('ErrorReport', () => {
     };
 
     render(validations);
+    expect(screen.getByTestId('ErrorReport')).toBeInTheDocument();
+
+    // Unmapped errors should not be clickable
+    const errorNode = screen.getByText('some unmapped error');
+    expect(errorNode).toBeInTheDocument();
+    expect(errorNode.parentElement.tagName).toEqual('LI');
+  });
+
+  it('should list mapped error as clickable', () => {
+    const validations = {
+      page1: {
+        someComponent: {
+          simpleBinding: {
+            errors: [getParsedLanguageFromText('some mapped error')],
+          },
+        },
+      },
+    };
+
+    render(validations);
+    expect(screen.getByTestId('ErrorReport')).toBeInTheDocument();
 
-    expect(screen.queryByText(genericErrorText)).not.toBeInTheDocument();
-    expect(screen.getByText('some unmapped error')).toBeInTheDocument();
+    const errorNode = screen.getByText('some mapped error');
+    expect(errorNode).toBeInTheDocument();
+    expect(errorNode.parentElement.parentElement.tagName).toEqual('LI');
+    expect(errorNode.parentElement.tagName).toEqual('BUTTON');
   });
 });