[V6] ErrorSummary magic (#2642)

* ♻️ Flyttet li-tag fra ErrorSummary-parent til child * 🏷️ Bedre typer * 🔒 Heading er nå required i ErrorSummary * 🎨 Egen prop for ErrorSummary fokus * 📝 Changeset * 🎨 Bedre fokushåndtering i ErrorSummary * ♻️ focusTarget -> focusTargetRef * 📝 Fikset jsdoc * 📝 Migreringstekst * 📝 changeset * 📝 Migration-text * 📝 JSDoc-staving
navikt · Jan 18, 2024 · aa8cc46 · aa8cc46
1 parent 40f688f
commit aa8cc46
Show file tree

Hide file tree

Showing 7 changed files with 101 additions and 24 deletions.
diff --git a/.changeset/gold-pumas-cheat.md b/.changeset/gold-pumas-cheat.md
@@ -0,0 +1,5 @@
+---
+"@navikt/ds-react": major
+---
+
+ErrorSummary: Heading-prop er satt til required. Lagt til ny prop `focusTargetRef` for å kunne bedre støtte skjermlesere når man manuelt flytter fokus til `ErrorSummary` og `li`-tag er flyttet inn i ErrorSummary.Item.
diff --git a/@navikt/core/css/form/error-summary.css b/@navikt/core/css/form/error-summary.css
@@ -9,11 +9,17 @@
   padding: var(--a-spacing-3);
 }
 
-.navds-error-summary:focus-visible {
-  box-shadow: var(--a-shadow-focus);
+.navds-error-summary__heading:focus {
   outline: none;
 }
 
+.navds-error-summary:focus-visible,
+.navds-error-summary:has(.navds-error-summary__heading:focus-visible) {
+  box-shadow:
+    0 0 0 1px var(--a-border-on-inverted),
+    0 0 0 4px var(--a-border-focus);
+}
+
 @supports not selector(:focus-visible) {
   .navds-error-summary:focus {
     box-shadow: var(--a-shadow-focus);

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 } from "react";
 import { BodyShort, Heading } from "../../typography";
 import { useId } from "../../util/hooks";
-import ErrorSummaryItem, { ErrorSummaryItemType } from "./ErrorSummaryItem";
+import ErrorSummaryItem from "./ErrorSummaryItem";
 
 export interface ErrorSummaryProps extends HTMLAttributes<HTMLDivElement> {
   /**
@@ -17,12 +17,18 @@ export interface ErrorSummaryProps extends HTMLAttributes<HTMLDivElement> {
   /**
    * Heading above links
    */
-  heading?: React.ReactNode;
+  heading: React.ReactNode;
   /**
    * Allows setting a different HTML h-tag
    * @default "h2"
    */
   headingTag?: React.ElementType<any>;
+  /**
+   * When manually setting focus to `<ErrorSummary />` use the
+   * `focusTargetRef`-prop and not ref.
+   * This directs focus to heading, improving screen reader experience
+   */
+  focusTargetRef?: React.RefObject<HTMLElement>;
 }
 
 interface ErrorSummaryComponent
@@ -41,7 +47,7 @@ interface ErrorSummaryComponent
    * </ErrorSummary.Item>
    * ```
    */
-  Item: ErrorSummaryItemType;
+  Item: typeof ErrorSummaryItem;
 }
 
 /**
@@ -70,6 +76,7 @@ export const ErrorSummary = forwardRef<HTMLDivElement, ErrorSummaryProps>(
       size = "medium",
       headingTag = "h2",
       heading,
+      focusTargetRef,
       ...rest
     },
     ref,
@@ -85,7 +92,7 @@ export const ErrorSummary = forwardRef<HTMLDivElement, ErrorSummaryProps>(
           "navds-error-summary",
           `navds-error-summary--${size}`,
         )}
-        tabIndex={-1}
+        tabIndex={ref ? -1 : undefined}
         aria-live="polite"
         aria-relevant="all"
         aria-labelledby={headingId}
@@ -95,16 +102,13 @@ export const ErrorSummary = forwardRef<HTMLDivElement, ErrorSummaryProps>(
           as={headingTag}
           size="small"
           id={headingId}
+          ref={focusTargetRef}
+          tabIndex={focusTargetRef ? -1 : undefined}
         >
           {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,5 +1,6 @@
 import { Meta } from "@storybook/react";
-import React from "react";
+import { expect, userEvent, within } from "@storybook/test";
+import React, { useRef } from "react";
 import { ErrorSummary } from "..";
 
 export default {
@@ -43,3 +44,56 @@ export const Small = () => (
     </ErrorSummary.Item>
   </ErrorSummary>
 );
+
+export const A11yDemo = {
+  render: () => {
+    // eslint-disable-next-line react-hooks/rules-of-hooks
+    const ref = useRef<HTMLHeadingElement>(null);
+    return (
+      <div>
+        <button onClick={() => ref.current?.focus()}>
+          Fokuser Errorsummary på klikk
+        </button>
+        <ErrorSummary heading="Feiloppsummering tittel" focusTargetRef={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 = {
+  render: () => {
+    // eslint-disable-next-line react-hooks/rules-of-hooks
+    const ref = useRef<HTMLHeadingElement>(null);
+    return (
+      <div>
+        <button onClick={() => ref.current?.focus()}>Focus summary</button>
+        <ErrorSummary heading="Feiloppsummering tittel" focusTargetRef={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();
+  },
+  parameters: {
+    chromatic: { disableSnapshot: true },
+  },
+};
diff --git a/aksel.nav.no/website/pages/eksempler/errorsummary/setting-focus.tsx b/aksel.nav.no/website/pages/eksempler/errorsummary/setting-focus.tsx
@@ -3,7 +3,7 @@ import { Button, ErrorSummary } from "@navikt/ds-react";
 import { withDsExample } from "@/web/examples/withDsExample";
 
 const Example = () => {
-  const errorRef = useRef<HTMLDivElement>(null);
+  const errorRef = useRef<HTMLElement>(null);
   const [hasError, setHasError] = useState(false);
 
   useEffect(() => {
@@ -14,8 +14,8 @@ const Example = () => {
     <div className="flex flex-col items-start gap-12">
       {hasError && (
         <ErrorSummary
-          ref={errorRef}
           heading="Du må fikse disse feilene før du kan sende inn søknad."
+          focusTargetRef={errorRef}
         >
           <ErrorSummary.Item href="#1">
             Felt må fylles ut med alder

diff --git a/v6-migration.md b/v6-migration.md
@@ -1,5 +1,11 @@
 # Aksel v6 migrations
 
+## ErrorSummary
+
+- Heading-prop er satt som required
+- Ny prop `focusTargetRef` for å bedre håndtere fokushåntering
+- Hinte om at neste major-versjon kommer til å potensielt endre hvordan ref settes på section (tabIndex fjernes) og at de bør bruke `focusTargetRef`.
+
 ## Chat
 
 ### Props