From bfa80a4f4adbdea4856e23a7d6898e904a898491 Mon Sep 17 00:00:00 2001
From: Robbert Broersma <git@robbertbroersma.nl>
Date: Sun, 19 May 2024 19:12:43 +0200
Subject: [PATCH 1/4] test: small tweaks to React component tests

---
 .../src/Article.test.tsx                        | 12 ++++++++++++
 .../src/BadgeCounter.test.tsx                   |  3 ++-
 .../src/Checkbox.test.tsx                       | 17 +++++++++++++----
 .../component-library-react/src/IBANData.tsx    |  2 +-
 .../src/Listbox.test.tsx                        | 10 +++++++---
 5 files changed, 35 insertions(+), 9 deletions(-)

diff --git a/packages/component-library-react/src/Article.test.tsx b/packages/component-library-react/src/Article.test.tsx
index edb97961b3c..30c35b2ad31 100644
--- a/packages/component-library-react/src/Article.test.tsx
+++ b/packages/component-library-react/src/Article.test.tsx
@@ -13,6 +13,18 @@ describe('Article', () => {
     expect(article).toBeVisible();
   });
 
+  it('can render an article role with a name', () => {
+    render(
+      <Article aria-labelledby="heading">
+        <h1 id="heading">Heading</h1>
+      </Article>,
+    );
+
+    const article = screen.getByRole('article', { name: 'Heading' });
+
+    expect(article).toBeInTheDocument();
+  });
+
   it('renders an article HTML element', () => {
     const { container } = render(<Article />);
 
diff --git a/packages/component-library-react/src/BadgeCounter.test.tsx b/packages/component-library-react/src/BadgeCounter.test.tsx
index d071c83e1f0..56f17fcc6ad 100644
--- a/packages/component-library-react/src/BadgeCounter.test.tsx
+++ b/packages/component-library-react/src/BadgeCounter.test.tsx
@@ -3,7 +3,7 @@ import { createRef } from 'react';
 import { BadgeCounter } from './BadgeCounter';
 import '@testing-library/jest-dom';
 
-describe('Data badge', () => {
+describe('Badge counter', () => {
   it('renders an HTML span element', () => {
     const { container } = render(<BadgeCounter>{'42'}</BadgeCounter>);
 
@@ -29,6 +29,7 @@ describe('Data badge', () => {
 
     expect(badge).toHaveClass('utrecht-badge-counter');
   });
+
   it('can have a additional class name', () => {
     const { container } = render(<BadgeCounter className="large" />);
 
diff --git a/packages/component-library-react/src/Checkbox.test.tsx b/packages/component-library-react/src/Checkbox.test.tsx
index 5b0dde3baa1..3ff0dc3ad60 100644
--- a/packages/component-library-react/src/Checkbox.test.tsx
+++ b/packages/component-library-react/src/Checkbox.test.tsx
@@ -36,6 +36,7 @@ describe('Checkbox', () => {
 
     expect(link).toHaveClass('utrecht-checkbox');
   });
+
   it('can have a additional class name', () => {
     const { container } = render(<Checkbox className="large" />);
 
@@ -47,9 +48,9 @@ describe('Checkbox', () => {
   });
   describe('checked variant', () => {
     it('is not checked by default', () => {
-      const { container } = render(<Checkbox />);
+      render(<Checkbox />);
 
-      const checkbox = container.querySelector(':only-child');
+      const checkbox = screen.getByRole('checkbox');
 
       expect(checkbox).not.toBeChecked();
     });
@@ -67,9 +68,17 @@ describe('Checkbox', () => {
 
     it('can have a checked state', () => {
       const handleChange = () => {};
-      const { container } = render(<Checkbox checked onChange={handleChange} />);
+      render(<Checkbox checked onChange={handleChange} />);
 
-      const checkbox = container.querySelector(':only-child');
+      const checkbox = screen.getByRole('checkbox');
+
+      expect(checkbox).toBeChecked();
+    });
+
+    it('can have a defaultChecked state (in React)', () => {
+      render(<Checkbox defaultChecked />);
+
+      const checkbox = screen.getByRole('checkbox');
 
       expect(checkbox).toBeChecked();
     });
diff --git a/packages/component-library-react/src/IBANData.tsx b/packages/component-library-react/src/IBANData.tsx
index 63ed55a0bab..762ac3190dc 100644
--- a/packages/component-library-react/src/IBANData.tsx
+++ b/packages/component-library-react/src/IBANData.tsx
@@ -27,7 +27,7 @@ export const IBANData = forwardRef(
   ({ children, value, className, ...restProps }: IBANDataProps, ref: ForwardedRef<HTMLDataElement>) => {
     const normalized = normalizeIBAN(value);
     const formatted = formatIBAN(normalized);
-    console.log({ value, normalized, formatted });
+
     return (
       <data {...restProps} ref={ref} value={normalized} className={clsx('utrecht-iban-data', className)} translate="no">
         {children || formatted}
diff --git a/packages/component-library-react/src/Listbox.test.tsx b/packages/component-library-react/src/Listbox.test.tsx
index 12cf7afbaae..3d9c1e69b5b 100644
--- a/packages/component-library-react/src/Listbox.test.tsx
+++ b/packages/component-library-react/src/Listbox.test.tsx
@@ -39,13 +39,17 @@ describe('Listbox', () => {
   });
 
   describe('disabled state', () => {
-    // `aria-disabled` is somehow not recognized as disabled state
-    it.skip('has a disabled listbox', () => {
+    // `aria-disabled` is somehow not recognized as disabled state on a listbox by Testing Library
+    it.todo('has a disabled listbox in the accessibility tree');
+
+    // Temporary alternative to the accessibility tree test
+    it('has a disabled listbox in the DOM', () => {
       render(<Listbox disabled />);
 
       const listbox = screen.getByRole('listbox');
 
-      expect(listbox).toBeDisabled();
+      // Look at the DOM instead of the accessibility tree
+      expect(listbox).toHaveAttribute('aria-disabled', 'true');
     });
   });
 

From b0431221a25fd2ba21b36892cd5907eab56041f8 Mon Sep 17 00:00:00 2001
From: Robbert Broersma <git@robbertbroersma.nl>
Date: Sun, 19 May 2024 19:13:19 +0200
Subject: [PATCH 2/4] docs: guidance on writing complete React component unit
 tests

---
 packages/component-library-react/TESTING.md | 207 ++++++++++++++++++++
 1 file changed, 207 insertions(+)
 create mode 100644 packages/component-library-react/TESTING.md

diff --git a/packages/component-library-react/TESTING.md b/packages/component-library-react/TESTING.md
new file mode 100644
index 00000000000..3496aa06d1f
--- /dev/null
+++ b/packages/component-library-react/TESTING.md
@@ -0,0 +1,207 @@
+<!-- @license CC0-1.0 -->
+
+# Testing components
+
+## Test for extensibility
+
+### Class names
+
+Front-end developers rely on the BEM class names to add their own CSS. When the component renames or removes a class name, there is a breaking change. Unit tests must check each class name, so they are reliable APIs.
+
+You will find many tests like this:
+
+```jsx
+it("renders a design system BEM class name: my-component", () => {
+  const { container } = render(<MyComponent />);
+
+  const field = container.querySelector("div");
+
+  expect(field).toHaveClass(".my-component");
+});
+```
+
+### So I put some HTML in your HTML
+
+Text in components can sometimes be improved with markup: language metadata, code, emphasis or images. Each property that ends up in the HTML should be tested to be extensible with rich text content.
+
+```jsx
+it("renders rich text content", () => {
+  const { container } = render(
+    <Heading1 {...defaultProps}>
+      The French national motto: <span lang="fr">Liberté, égalité, fraternité</span>
+    </Heading1>
+  );
+
+  const richText = container.querySelector("span");
+
+  expect(richText).toBeInTheDocument();
+});
+```
+
+Testing properties is perhaps even more important, because `children` usually already allows HTML content:
+
+```jsx
+it('renders rich text content', () => {
+  const { container } = render(
+    <FormFieldTextbox label={
+      <EmailIcon/> E-mail address
+    }></FormFieldTextbox>,
+  );
+
+  const richText = container.querySelector('svg');
+
+  expect(richText).toBeInTheDocument();
+});
+```
+
+## Don't break native HTML
+
+### Global attributes
+
+[Global attributes](https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes) can be used on all HTML elements, so components that render HTML must support them too. In React this is easy to support using `...restProps`. The following code examples use global attributes:
+
+- `<MyComponent id="main" />`
+- `<MyComponent style={{ '--my-component-color': 'currentColor' }} />`
+- `<MyComponent hidden />`
+- `<MyComponent tabIndex={-1} />`
+- `<MyComponent lang="en" />`
+- `<MyComponent className="custom" />`
+- `<MyComponent data-test-id="component" />`
+- `<MyComponent role="group" />`
+
+### The `hidden` property
+
+The CSS for a component frequently break the `hidden` attribute, because code like `display: flex` overrides the default styles. Test that the `hidden` attribute still makes the invisible.
+
+```jsx
+it("can be hidden", () => {
+  const { container } = render(<MyComponent hidden />);
+
+  const component = container.querySelector("div");
+
+  expect(component).not.toBeVisible();
+});
+```
+
+### The `className` property
+
+Components render BEM class names, but front-end developers need to by able to use their own class names as well. Additional class names must extend the class list, not overwrite the component class names.
+
+```jsx
+it("can have a additional class name", () => {
+  const { container } = render(<MyComponent className="large" />);
+
+  const component = container.querySelector(":only-child");
+
+  expect(component).toHaveClass("large");
+  expect(component).toHaveClass("my-component");
+});
+```
+
+## Test the accessibility tree
+
+### Landmarks
+
+```jsx
+it("renders an complementary role element", () => {
+  render(<Aside />);
+
+  const aside = screen.getByRole("complementary");
+
+  expect(aside).toBeInTheDocument();
+});
+```
+
+### Label for landmarks
+
+Some components have an API to configure the label:
+
+```jsx
+it("renders an complementary role element with a name", () => {
+  render(<BreadcrumbNav label="Breadcrumbs" />);
+
+  const nav = screen.getByRole("navigation", { name: "Breadcrumbs" });
+
+  expect(nav).toBeInTheDocument();
+});
+```
+
+Other components need to rely on `aria-labelledby` or `aria-label`.
+
+```jsx
+it('renders an complementary role element with a name', () => {
+  render(
+    <Aside aria-labelledby="heading">
+      <h2 id="heading">See also</h1>
+    </Aside>
+  );
+
+  const aside = screen.getByRole('complementary', { name: 'See also' });
+
+  expect(aside).toBeInTheDocument();
+});
+```
+
+### States
+
+Voor [WCAG 4.1.2](https://nldesignsystem.nl/wcag/4.1.2) is het belangrijk dat de state van componenten beschikbaar is in de accessibility tree. [Testing Library heeft APIs](https://testing-library.com/docs/queries/byrole) om de informatie uit de accessibility tree op te vragen, in plaats van via de DOM.
+
+Voorbeelden van state zijn:
+
+- Een `checkbox` die `checked` is.
+- Een `textbox` die `disabled` is.
+- Een `textarea` die `required` is.
+- Een `button` die `expanded` is.
+
+```jsx
+describe("checked variant", () => {
+  it("is not checked by default", () => {
+    const { container } = render(<Checkbox />);
+
+    const checkbox = screen.getByRole("checkbox");
+
+    expect(checkbox).not.toBeChecked();
+  });
+
+  it("can have a checked state", () => {
+    const handleChange = () => {};
+    render(<Checkbox checked onChange={handleChange} />);
+
+    const checkbox = screen.getByRole("checkbox");
+
+    expect(checkbox).toBeChecked();
+  });
+
+  it("can have a defaultChecked state (in React)", () => {
+    render(<Checkbox defaultChecked />);
+
+    const checkbox = screen.getByRole("checkbox");
+
+    expect(checkbox).toBeChecked();
+  });
+});
+```
+
+Helaas ondersteunt Testing Library nog niet elke state in de accessibility tree. Maak alvast wel de test, maar sla de test over `skip`. Gebruik de DOM om de test op een alternative manier te doen.
+
+```jsx
+// `aria-disabled` is somehow not recognized as disabled state
+it.skip("has a disabled listbox", () => {
+  render(<Listbox disabled />);
+
+  const listbox = screen.getByRole("listbox");
+
+  expect(listbox).toBeDisabled();
+});
+
+// Temporary alternative to the accessibility tree test
+it("has a disabled listbox", () => {
+  render(<Listbox disabled />);
+
+  const listbox = screen.getByRole("listbox");
+
+  expect(listbox).toHaveAttribute("aria-disabled", "true");
+});
+```
+
+Controleer periodiek of een nieuwe versie van Testing Library de state wel ondersteunt.

From 198f8060b18fddf3ea0428892f8326286fb42747 Mon Sep 17 00:00:00 2001
From: Robbert Broersma <git@robbertbroersma.nl>
Date: Mon, 20 May 2024 18:40:17 +0200
Subject: [PATCH 3/4] docs: guidance on writing effective React components

---
 .../component-library-react/CONTRIBUTING.md   | 169 ++++++++++++++++++
 packages/component-library-react/TESTING.md   |  14 +-
 2 files changed, 174 insertions(+), 9 deletions(-)
 create mode 100644 packages/component-library-react/CONTRIBUTING.md

diff --git a/packages/component-library-react/CONTRIBUTING.md b/packages/component-library-react/CONTRIBUTING.md
new file mode 100644
index 00000000000..ebf07263036
--- /dev/null
+++ b/packages/component-library-react/CONTRIBUTING.md
@@ -0,0 +1,169 @@
+<!-- @license CC0-1.0 -->
+
+# Developing components
+
+Warning: the code examples are deliberately simplified to show one concept at a time. Do not use the code snippets as-is.
+
+For more complete examples, look at the source code of existing components.
+
+## Develop for extensibility
+
+### Class names
+
+Enable front-end developers to add their own class names to the outermost HTML element of your component. Since all components have BEM class names, you must combine your BEM class names with any class names from the parameters. We typically use `clsx` to format the `class` attribute.
+
+```jsx
+import clsx from "clsx";
+
+export const MyComponent = ({ children, className }) => (
+  <div className={clsx("my-component", className)}>{children}</div>
+);
+```
+
+### Allow rich text content
+
+Allow `ReactNode` contents for text parameters, so front-end developers can use accessible and meaningful markup. Using `PropsWithChildren` is recommended for non-empty components, because it allows `ReactNode` for children.
+
+```tsx
+import clsx from "clsx";
+import type { PropsWithChildren } from "react";
+
+export interface MyComponentProps {
+  // ...
+}
+
+export const MyComponent = ({ children }: PropsWithChildren<MyComponentProps>) => <div>{children}</div>;
+```
+
+For other parameters using `ReactNode` might not be as obvious, since you might feel like starting with `string`. For example:
+
+```tsx
+import clsx from "clsx";
+import type { PropsWithChildren } from "react";
+
+export interface MyLandmarkComponentProps {
+  label: ReactNode;
+}
+
+export const MyLandmarkComponent = ({ children, label }: PropsWithChildren<MyComponentProps>) => {
+  const headingId = useId();
+  return (
+    <div role="region" aria-labelledby={label ? labelId : undefined}>
+      {label && <div id={labelId}>{label}</div>}
+      {children}
+    </div>
+  );
+};
+```
+
+This allows front-end developers to use any markup:
+
+```jsx
+<MyLandmarkComponent
+  label={
+    <>
+      <LandmarkIcon /> Landmark label
+    </>
+  }
+>
+  <p>Landmark content</p>
+</MyLandmarkComponent>
+```
+
+Allowing rich text is one more reason to use `aria-labelledby` instead of `aria-label`.
+
+## Export interfaces and types
+
+Export the type definitions for parameters, so other developers can easily use those to develop wrapper components.
+
+```tsx
+export type TextboxTypes = "password" | "text";
+
+export interface TextboxProps extends InputHTMLAttributes<HTMLInputElement> {
+  type?: TextboxTypes;
+}
+
+export const Textbox = ({ type }: TextboxProps) => <input {...restProps} type={type} />;
+```
+
+This way another developer could extend your component:
+
+```tsx
+import type { TextboxProps, TextboxTypes } from "@my/textbox";
+
+export interface AdvancedTextboxProps extends TextboxProps {
+  type?: TextboxTypes | "date";
+}
+
+export const AdvancedTextbox = ({ type }: AdvancedTextboxProps) => <input {...restProps} type={type} />;
+```
+
+## Use `forwardRef`
+
+Use [`forwardRef`](https://react.dev/reference/react/forwardRef) to expose the DOM node with a [ref](https://react.dev/learn/manipulating-the-dom-with-refs).
+
+```tsx
+import clsx from "clsx";
+import type { ForwardedRef, HTMLAttributes, PropsWithChildren } from "react";
+
+export interface MyComponentProps extends HTMLAttributes<HTMLDivElement> {}
+
+export const MyComponent = forwardRef(
+  ({ children, ...restProps }: PropsWithChildren<MyComponentProps>, ref: ForwardedRef<HTMLDivElement>) => (
+    <div ref={ref} {...restProps}>
+      {children}
+    </div>
+  )
+);
+```
+
+This allows front-end developers to perform actions that need access to the DOM, such as focusing an element:
+
+```tsx
+const ref = useRef<HTMLDivElement>(null);
+
+render(<MyComponent tabIndex={0} ref={ref} />);
+
+ref.current?.focus();
+```
+
+## Don't break native HTML
+
+### Global attributes
+
+Use `restProps` to allow front-end developers to global attributes as well as specific attributes to an HTML element:
+
+```jsx
+import clsx from "clsx";
+
+export const MyComponent = ({ children, ...restProps }) => <div {...restProps}>{children}</div>;
+```
+
+With TypeScript you will need to extend the interface with the `HTMLAttributes` of the outermost element, and it will look like this:
+
+```tsx
+import clsx from "clsx";
+import type { HTMLAttributes, PropsWithChildren } from "react";
+
+export interface MyComponentProps extends HTMLAttributes<HTMLDivElement> {}
+
+export const MyComponent = ({ children, ...restProps }) => <div {...restProps}>{children}</div>;
+```
+
+For different elements you need to import different types `HTMLAttributes`, but I don't think React offers documentation on this subject. The most effective approach might be using an IDE like Visual Code with a TypeScript plugin, to find out which interface you need — or simply check existing components that use the same HTML element.
+
+### Prevent duplicate IDs
+
+Generate `id` attributes with [`useId()` from React](https://react.dev/reference/react/useId). Do not use hardcoded `id` values, because that could break accessibility.
+
+```jsx
+export const MyLandmarkComponent = ({ children, label }) => {
+  const headingId = useId();
+  return (
+    <div role="region" aria-labelledby={label ? labelId : undefined}>
+      {label && <div id={labelId}>{label}</div>}
+      {children}
+    </div>
+  );
+};
+```
diff --git a/packages/component-library-react/TESTING.md b/packages/component-library-react/TESTING.md
index 3496aa06d1f..7bccb8152e2 100644
--- a/packages/component-library-react/TESTING.md
+++ b/packages/component-library-react/TESTING.md
@@ -182,17 +182,12 @@ describe("checked variant", () => {
 });
 ```
 
-Helaas ondersteunt Testing Library nog niet elke state in de accessibility tree. Maak alvast wel de test, maar sla de test over `skip`. Gebruik de DOM om de test op een alternative manier te doen.
+Helaas ondersteunt Testing Library nog niet elke state in de accessibility tree. Maak alvast wel de test, maar sla de test over `todo`. Gebruik de DOM om de test op een alternative manier te doen.
 
 ```jsx
-// `aria-disabled` is somehow not recognized as disabled state
-it.skip("has a disabled listbox", () => {
-  render(<Listbox disabled />);
-
-  const listbox = screen.getByRole("listbox");
-
-  expect(listbox).toBeDisabled();
-});
+// `aria-disabled` is somehow not recognized as disabled state on a listbox by Testing Library
+// https://github.com/testing-library/jest-dom/issues/144
+it.todo("has a disabled listbox in the accessibility tree", () => {});
 
 // Temporary alternative to the accessibility tree test
 it("has a disabled listbox", () => {
@@ -200,6 +195,7 @@ it("has a disabled listbox", () => {
 
   const listbox = screen.getByRole("listbox");
 
+  // Look at the DOM instead of the accessibility tree
   expect(listbox).toHaveAttribute("aria-disabled", "true");
 });
 ```

From 7fedef315dd1b81f547279415702dec481018994 Mon Sep 17 00:00:00 2001
From: Robbert Broersma <git@robbertbroersma.nl>
Date: Mon, 20 May 2024 18:51:29 +0200
Subject: [PATCH 4/4] chore: add developing/testing component docs to Storybook

---
 packages/storybook-react/config/preview.tsx               | 7 ++++++-
 .../storybook-react/src/stories/Introduction.stories.mdx  | 6 +++---
 .../storybook-react/src/stories/contributing.stories.mdx  | 8 ++++++++
 packages/storybook-react/src/stories/testing.stories.mdx  | 8 ++++++++
 4 files changed, 25 insertions(+), 4 deletions(-)
 create mode 100644 packages/storybook-react/src/stories/contributing.stories.mdx
 create mode 100644 packages/storybook-react/src/stories/testing.stories.mdx

diff --git a/packages/storybook-react/config/preview.tsx b/packages/storybook-react/config/preview.tsx
index 96a32982898..ff7f28d0512 100644
--- a/packages/storybook-react/config/preview.tsx
+++ b/packages/storybook-react/config/preview.tsx
@@ -11,7 +11,7 @@ import '@utrecht/storybook-helpers/src/storybook-docs.scss';
 import '@nl-design-system-unstable/amsterdam-design-tokens/dist/index.css';
 import '@nl-design-system-unstable/rotterdam-design-tokens/dist/index.css';
 import '@gemeente-denhaag/design-tokens-components/dist/theme/index.css';
-console.log(results);
+
 const preview: Preview = {
   decorators: [
     (Story: any) => <div className="utrecht-document">{Story()}</div>,
@@ -44,6 +44,11 @@ const preview: Preview = {
         );
       },
     },
+    options: {
+      storySort: {
+        order: ['React Component', ['Component Introduction', 'Developing components', 'Testing components']],
+      },
+    },
     ...addonStatus,
     ...addonThemes,
     ...addonViewport,
diff --git a/packages/storybook-react/src/stories/Introduction.stories.mdx b/packages/storybook-react/src/stories/Introduction.stories.mdx
index 28297032fdf..66cd03b375b 100644
--- a/packages/storybook-react/src/stories/Introduction.stories.mdx
+++ b/packages/storybook-react/src/stories/Introduction.stories.mdx
@@ -1,8 +1,8 @@
-import { Description, Meta } from "@storybook/addon-docs";
-import README from "@utrecht/component-library-react/README.md?raw";
+import { Markdown, Meta } from "@storybook/blocks";
+import document from "@utrecht/component-library-react/README.md?raw";
 
 <Meta id="react--readme" title="React Component/Component Introduction" />
 
 {/* @license CC0-1.0 */}
 
-<Description markdown={README} />
+<Markdown>{document}</Markdown>
diff --git a/packages/storybook-react/src/stories/contributing.stories.mdx b/packages/storybook-react/src/stories/contributing.stories.mdx
new file mode 100644
index 00000000000..69f973a19b1
--- /dev/null
+++ b/packages/storybook-react/src/stories/contributing.stories.mdx
@@ -0,0 +1,8 @@
+import { Markdown, Meta } from "@storybook/blocks";
+import document from "@utrecht/component-library-react/CONTRIBUTING.md?raw";
+
+<Meta id="react--contributing" title="React Component/Developing components" />
+
+{/* @license CC0-1.0 */}
+
+<Markdown>{document}</Markdown>
diff --git a/packages/storybook-react/src/stories/testing.stories.mdx b/packages/storybook-react/src/stories/testing.stories.mdx
new file mode 100644
index 00000000000..ee246495072
--- /dev/null
+++ b/packages/storybook-react/src/stories/testing.stories.mdx
@@ -0,0 +1,8 @@
+import { Markdown, Meta } from "@storybook/blocks";
+import document from "@utrecht/component-library-react/TESTING.md?raw";
+
+<Meta id="react--testing" title="React Component/Testing components" />
+
+{/* @license CC0-1.0 */}
+
+<Markdown>{document}</Markdown>