ErrorSummary: Add fallback for heading + better focus handling (#3140)

Co-authored-by: Ken <[email protected]> Co-authored-by: Ken <[email protected]>
navikt · Sep 12, 2024 · 431d4de · 431d4de
1 parent 4e5e7dc
commit 431d4de
Show file tree

Hide file tree

Showing 9 changed files with 129 additions and 51 deletions.
diff --git a/.changeset/great-crews-hang.md b/.changeset/great-crews-hang.md
@@ -0,0 +1,5 @@
+---
+"@navikt/ds-react": major
+---
+
+ErrorSummary: Added fallback text for `heading`.
diff --git a/.changeset/thick-roses-sneeze.md b/.changeset/thick-roses-sneeze.md
@@ -0,0 +1,6 @@
+---
+"@navikt/ds-react": patch
+"@navikt/ds-css": patch
+---
+
+ErrorSummary: Focus heading instead of container for improved experience with screen reader.
diff --git a/@navikt/core/css/form/error-summary.css b/@navikt/core/css/form/error-summary.css
@@ -9,6 +9,14 @@
   padding: var(--a-spacing-3);
 }
 
+.navds-error-summary__heading {
+  scroll-margin-top: var(--a-spacing-6);
+}
+
+.navds-error-summary--small .navds-error-summary__heading {
+  scroll-margin-top: var(--a-spacing-4);
+}
+
 .navds-error-summary__heading:focus {
   outline: none;
 }

diff --git a/@navikt/core/react/src/form/error-summary/ErrorSummary.tsx b/@navikt/core/react/src/form/error-summary/ErrorSummary.tsx
@@ -1,8 +1,8 @@
 import cl from "clsx";
-import React, { HTMLAttributes, forwardRef, isValidElement } from "react";
+import React, { HTMLAttributes, forwardRef, useRef } from "react";
 import { BodyShort, Heading } from "../../typography";
-import { useId } from "../../util/hooks";
-import ErrorSummaryItem, { ErrorSummaryItemType } from "./ErrorSummaryItem";
+import { useId, useMergeRefs } from "../../util/hooks";
+import ErrorSummaryItem from "./ErrorSummaryItem";
 
 export interface ErrorSummaryProps extends HTMLAttributes<HTMLDivElement> {
   /**
@@ -16,6 +16,7 @@ export interface ErrorSummaryProps extends HTMLAttributes<HTMLDivElement> {
   size?: "medium" | "small";
   /**
    * Heading above links.
+   * @default "Du må rette disse feilene før du kan fortsette:"
    */
   heading?: React.ReactNode;
   /**
@@ -41,7 +42,7 @@ interface ErrorSummaryComponent
    * </ErrorSummary.Item>
    * ```
    */
-  Item: ErrorSummaryItemType;
+  Item: typeof ErrorSummaryItem;
 }
 
 /**
@@ -69,16 +70,21 @@ export const ErrorSummary = forwardRef<HTMLDivElement, ErrorSummaryProps>(
       className,
       size = "medium",
       headingTag = "h2",
-      heading,
+      heading = "Du må rette disse feilene før du kan fortsette:",
       ...rest
     },
     ref,
   ) => {
     const headingId = useId();
 
+    const sectionRef = useRef<HTMLDivElement>(null);
+    const headingRef = useRef<HTMLHeadingElement>(null);
+
+    const mergedRef = useMergeRefs(ref, sectionRef);
+
     return (
       <section
-        ref={ref}
+        ref={mergedRef}
         {...rest}
         className={cl(
           className,
@@ -89,22 +95,24 @@ export const ErrorSummary = forwardRef<HTMLDivElement, ErrorSummaryProps>(
         aria-live="polite"
         aria-relevant="all"
         aria-labelledby={headingId}
+        onFocus={(event) => {
+          if (event.target === sectionRef.current) {
+            headingRef?.current?.focus();
+          }
+        }}
       >
         <Heading
           className="navds-error-summary__heading"
           as={headingTag}
           size="small"
           id={headingId}
+          ref={headingRef}
+          tabIndex={-1}
         >
           {heading}
         </Heading>
         <BodyShort as="ul" size={size} className="navds-error-summary__list">
-          {React.Children.map(children, (child) => {
-            if (!isValidElement(child)) {
-              return null;
-            }
-            return <li key={child.toString()}>{child}</li>;
-          })}
+          {children}
         </BodyShort>
       </section>
     );

diff --git a/@navikt/core/react/src/form/error-summary/ErrorSummaryItem.tsx b/@navikt/core/react/src/form/error-summary/ErrorSummaryItem.tsx
@@ -14,21 +14,23 @@ export interface ErrorSummaryItemProps
   href?: string;
 }
 
-export type ErrorSummaryItemType = OverridableComponent<
+type ErrorSummaryItemType = OverridableComponent<
   ErrorSummaryItemProps,
   HTMLAnchorElement
 >;
 
 export const ErrorSummaryItem: ErrorSummaryItemType = forwardRef(
   ({ children, as: Component = "a", className, ...rest }, ref) => {
     return (
-      <Component
-        {...rest}
-        ref={ref}
-        className={cl(className, "navds-error-summary__item", "navds-link")}
-      >
-        {children}
-      </Component>
+      <li>
+        <Component
+          {...rest}
+          ref={ref}
+          className={cl(className, "navds-error-summary__item", "navds-link")}
+        >
+          {children}
+        </Component>
+      </li>
     );
   },
 );

diff --git a/@navikt/core/react/src/form/error-summary/error-summary.stories.tsx b/@navikt/core/react/src/form/error-summary/error-summary.stories.tsx
@@ -1,24 +1,12 @@
 import { Meta, StoryObj } from "@storybook/react";
-import React from "react";
+import { expect, userEvent, within } from "@storybook/test";
+import React, { useRef } from "react";
 import { VStack } from "../../layout/stack";
 import { ErrorSummary } from "./ErrorSummary";
 
 export default {
-  title: "ds-react/Errorsummary",
+  title: "ds-react/ErrorSummary",
   component: ErrorSummary,
-  argTypes: {
-    headingTag: {
-      control: {
-        type: "text",
-      },
-    },
-    size: {
-      control: {
-        type: "radio",
-      },
-      options: ["medium", "small"],
-    },
-  },
   parameters: {
     chromatic: { disable: true },
   },
@@ -27,18 +15,32 @@ export default {
 type Story = StoryObj<typeof ErrorSummary>;
 
 export const Default: Story = {
-  render: (props) => (
-    <ErrorSummary
-      heading="Feiloppsummering komponent"
-      headingTag={props.headingTag || "h2"}
-      size={props.size ?? undefined}
-    >
+  render: ({ headingTag, ...rest }) => (
+    <ErrorSummary headingTag={headingTag || undefined} {...rest}>
       <ErrorSummary.Item href="#1">Checkbox må fylles ut</ErrorSummary.Item>
       <ErrorSummary.Item href="#2">
         Tekstfeltet må ha en godkjent e-mail
       </ErrorSummary.Item>
     </ErrorSummary>
   ),
+  argTypes: {
+    heading: {
+      control: {
+        type: "text",
+      },
+    },
+    headingTag: {
+      control: {
+        type: "text",
+      },
+    },
+    size: {
+      control: {
+        type: "radio",
+      },
+      options: ["medium", "small"],
+    },
+  },
 };
 
 export const Small: Story = {
@@ -52,6 +54,59 @@ export const Small: Story = {
   ),
 };
 
+export const A11yDemo: Story = {
+  name: "A11y Demo",
+  render: () => {
+    const ref = useRef<HTMLHeadingElement>(null);
+    return (
+      <div>
+        <button
+          onClick={() => {
+            ref.current?.focus();
+          }}
+        >
+          Fokuser ErrorSummary
+        </button>
+        <ErrorSummary heading="Feiloppsummering tittel" ref={ref}>
+          <ErrorSummary.Item href="#1">Checkbox må fylles ut</ErrorSummary.Item>
+          <ErrorSummary.Item href="#2">
+            Tekstfeltet må ha en godkjent e-mail
+          </ErrorSummary.Item>
+        </ErrorSummary>
+      </div>
+    );
+  },
+};
+
+export const FocusDemo: Story = {
+  render: () => {
+    const ref = useRef<HTMLHeadingElement>(null);
+    return (
+      <div>
+        <button onClick={() => ref.current?.focus()}>Focus summary</button>
+        <ErrorSummary heading="Feiloppsummering tittel" ref={ref}>
+          <ErrorSummary.Item href="#1">Checkbox må fylles ut</ErrorSummary.Item>
+          <ErrorSummary.Item href="#2">
+            Tekstfeltet må ha en godkjent e-mail
+          </ErrorSummary.Item>
+        </ErrorSummary>
+      </div>
+    );
+  },
+  play: async ({ canvasElement, step }) => {
+    const canvas = within(canvasElement);
+
+    const button = canvas.getByText("Focus summary");
+    const heading = canvas.getByText("Feiloppsummering tittel");
+
+    await step("click button", async () => {
+      await userEvent.click(button);
+    });
+
+    expect(heading).toHaveFocus();
+  },
+};
+
 export const Chromatic: Story = {
   render: () => (
     <VStack gap="4">

diff --git a/aksel.nav.no/website/pages/eksempler/errorsummary/demo.tsx b/aksel.nav.no/website/pages/eksempler/errorsummary/demo.tsx
@@ -3,7 +3,7 @@ import { withDsExample } from "@/web/examples/withDsExample";
 
 const Example = () => {
   return (
-    <ErrorSummary heading="Du må rette disse feilene før du kan sende inn søknaden:">
+    <ErrorSummary>
       <ErrorSummary.Item href="#1">
         Felt må fylles ut med alder
       </ErrorSummary.Item>

diff --git a/aksel.nav.no/website/pages/eksempler/errorsummary/setting-focus.tsx b/aksel.nav.no/website/pages/eksempler/errorsummary/setting-focus.tsx
@@ -13,10 +13,7 @@ const Example = () => {
   return (
     <VStack gap="12" align="start">
       {hasError && (
-        <ErrorSummary
-          ref={errorRef}
-          heading="Du må rette disse feilene før du kan sende inn søknaden:"
-        >
+        <ErrorSummary ref={errorRef}>
           <ErrorSummary.Item href="#1">
             Felt må fylles ut med alder
           </ErrorSummary.Item>
@@ -40,5 +37,5 @@ export const Demo = {
 
 export const args = {
   index: 3,
-  desc: "Sett fokus på ErrorSummary ved submit. Hvis du gjør en ny sidelasting kan du også sette en ID på ErrorSummary og referere til den i URLens hash.",
+  desc: "Sett fokus på ErrorSummary ved submit. Hvis du gjør en ny sidelasting kan du også sette en `id` og referere til den i URLens hash.",
 };
diff --git a/aksel.nav.no/website/pages/eksempler/errorsummary/small.tsx b/aksel.nav.no/website/pages/eksempler/errorsummary/small.tsx
@@ -3,10 +3,7 @@ import { withDsExample } from "@/web/examples/withDsExample";
 
 const Example = () => {
   return (
-    <ErrorSummary
-      heading="Du må rette disse feilene før du kan sende inn søknaden:"
-      size="small"
-    >
+    <ErrorSummary size="small">
       <ErrorSummary.Item href="#1">
         Felt må fylles ut med alder
       </ErrorSummary.Item>