Header Menu aria-expanded fix (#164)

* Add dev warnings for v5 breaking changes (#150)

* Add deprecation warning to footer columns prop

* Shared warning object for v5

* Dev warning for DoDontList.tsx

* Dev warning for TransactionalServiceName.tsx

* Dev warning for ReviewDate.tsx

* Lint

---------

Co-authored-by: Thomas Judd-Cooper <[email protected]>

* Fix Readme CI Build status tag (#149)

Co-authored-by: Thomas Judd-Cooper <[email protected]>

* Only run CI on main and PR's (#148)

Currently ci build will run on any push and any pull_request.
Pushing to a feature branch kicks off CI unintentionally.

Co-authored-by: Thomas Judd-Cooper <[email protected]>

* Bump ansi-regex from 4.1.0 to 4.1.1 (#156)

Bumps [ansi-regex](https://github.com/chalk/ansi-regex) from 4.1.0 to 4.1.1.
- [Release notes](https://github.com/chalk/ansi-regex/releases)
- [Commits](chalk/ansi-regex@v4.1.0...v4.1.1)

---
updated-dependencies:
- dependency-name: ansi-regex
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Thomas Judd-Cooper <[email protected]>

* Bump minimatch from 3.0.4 to 3.1.2 (#155)

Bumps [minimatch](https://github.com/isaacs/minimatch) from 3.0.4 to 3.1.2.
- [Release notes](https://github.com/isaacs/minimatch/releases)
- [Changelog](https://github.com/isaacs/minimatch/blob/main/changelog.md)
- [Commits](isaacs/minimatch@v3.0.4...v3.1.2)

---
updated-dependencies:
- dependency-name: minimatch
  dependency-type: indirect
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Thomas Judd-Cooper <[email protected]>

* Update ErrorSummary component to forward refs (#135)

This allows for things like programmatic keyboard focus in order to
fulfil accessibility requirements.

Co-authored-by: Phil Meier <[email protected]>
Co-authored-by: Thomas Judd-Cooper <[email protected]>

* bug/Checkbox declaration file inconsistency (#133)

* Add "strictNullChecks", fix two cases where typing needs updating in line with checks.

* Revert to .defaultProps

* Throw error on default HeadingLevel.tsx case

* Warn on invalid prop, test to cover

* Better dev warning for heading level

---------

Co-authored-by: Thomas Judd-Cooper <[email protected]>

* Add aria-controls

* Fix tests from main merge

* Remove Dev Warnings

* Final test cleanups

* Remove unused import

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Kai Spencer <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Phil Meier <[email protected]>
Co-authored-by: Phil Meier <[email protected]>

.github/workflows/ci.yml

@@ -1,6 +1,9 @@
 name: CI Build
 
-on: [push, pull_request]
+on: 
+  push:
+    branches: main
+  pull_request: 
 
 jobs:
   build:

README.md

@@ -11,7 +11,7 @@
 
 NHS.UK Frontend ported to React
 
-[![GitHub Actions CI Status](https://github.com/NHSDigital/nhsuk-react-components/workflows/CI/badge.svg)](https://github.com/NHSDigital/nhsuk-react-components/actions?query=workflow%3A%22CI+Build%22+branch%3Amain) [![Bundle Size](https://img.shields.io/bundlephobia/minzip/nhsuk-react-components.svg)](https://bundlephobia.com/result?p=nhsuk-react-components)
+[![GitHub Actions CI Status](https://github.com/NHSDigital/nhsuk-react-components/actions/workflows/ci.yml/badge.svg?branch=main)](https://github.com/NHSDigital/nhsuk-react-components/actions?query=workflow%3A%22CI+Build%22+branch%3Amain) [![Bundle Size](https://img.shields.io/bundlephobia/minzip/nhsuk-react-components.svg)](https://bundlephobia.com/result?p=nhsuk-react-components)
 
 ## Coming from 0.x?
 

src/components/do-dont-list/DoDontList.tsx

@@ -1,4 +1,4 @@
-import React, {HTMLProps, createContext, useContext, ReactNode} from 'react';
+import React, { HTMLProps, createContext, useContext, ReactNode } from 'react';
 import classNames from 'classnames';
 import { Tick, Cross } from '../icons';
 import HeadingLevel, { HeadingLevelType } from '../../util/HeadingLevel';
@@ -43,10 +43,10 @@ const DoDontList: DoDontList = ({
 
 interface DoDontItemProps extends HTMLProps<HTMLLIElement> {
   listItemType?: ListType;
-  prefixText?: ReactNode
+  prefixText?: ReactNode;
 }
 
-const DoDontItem: React.FC<DoDontItemProps> = ({prefixText, listItemType,children, ...rest }) => {
+const DoDontItem: React.FC<DoDontItemProps> = ({ prefixText, listItemType, children, ...rest }) => {
   const listItem = useContext(DoDontListContext);
   const defaultPrefix = (listItemType || listItem) === 'do' ? null : 'do not ';
   const actualPrefix = prefixText === undefined ? defaultPrefix : prefixText;

src/components/do-dont-list/__tests__/DoDontList.test.tsx

@@ -74,7 +74,7 @@ describe('DoDontList', () => {
       dontList.unmount();
     });
 
-    it("items render custom prefix text", () => {
+    it('items render custom prefix text', () => {
       const doList = mount(
         <DoDontList listType="do">
           <DoDontList.Item prefixText="do ">something good 1</DoDontList.Item>
@@ -88,7 +88,9 @@ describe('DoDontList', () => {
         <DoDontList listType="dont">
           <DoDontList.Item prefixText="do not ">do something bad 1</DoDontList.Item>
           <DoDontList.Item>do something bad 2</DoDontList.Item>
-          <DoDontList.Item prefixText={<span>don&apos;t do </span>}>something bad 3</DoDontList.Item>
+          <DoDontList.Item prefixText={<span>don&apos;t do </span>}>
+            something bad 3
+          </DoDontList.Item>
           <DoDontList.Item prefixText={undefined}>something bad 4</DoDontList.Item>
           <DoDontList.Item prefixText={null}>something bad 5</DoDontList.Item>
         </DoDontList>,
@@ -99,14 +101,20 @@ describe('DoDontList', () => {
       expect(doList.find('.nhsuk-list--tick').childAt(3).text()).toBe('something good 4');
       expect(doList.find('.nhsuk-list--tick').childAt(4).text()).toBe('something good 5');
 
-      expect(dontList.find('.nhsuk-list--cross').childAt(0).text()).toBe('do not do something bad 1');
-      expect(dontList.find('.nhsuk-list--cross').childAt(1).text()).toBe('do not do something bad 2');
-      expect(dontList.find('.nhsuk-list--cross').childAt(2).text()).toBe('don\'t do something bad 3');
+      expect(dontList.find('.nhsuk-list--cross').childAt(0).text()).toBe(
+        'do not do something bad 1',
+      );
+      expect(dontList.find('.nhsuk-list--cross').childAt(1).text()).toBe(
+        'do not do something bad 2',
+      );
+      expect(dontList.find('.nhsuk-list--cross').childAt(2).text()).toBe(
+        "don't do something bad 3",
+      );
       expect(dontList.find('.nhsuk-list--cross').childAt(3).text()).toBe('do not something bad 4');
       expect(dontList.find('.nhsuk-list--cross').childAt(4).text()).toBe('something bad 5');
 
       doList.unmount();
       dontList.unmount();
-    })
+    });
   });
 });

src/components/do-dont-list/__tests__/__snapshots__/DoDontList.test.tsx.snap

@@ -13,7 +13,6 @@ exports[`DoDontList matches snapshot: DoDontList-Do 1`] = `
 >
   <HeadingLevel
     className="nhsuk-do-dont-list__label"
-    headingLevel="h3"
   >
     Do
   </HeadingLevel>
@@ -33,7 +32,6 @@ exports[`DoDontList matches snapshot: DoDontList-Dont 1`] = `
 >
   <HeadingLevel
     className="nhsuk-do-dont-list__label"
-    headingLevel="h3"
   >
     Don't
   </HeadingLevel>

src/components/error-summary/ErrorSummary.tsx

@@ -1,4 +1,4 @@
-import React, { HTMLProps } from 'react';
+import React, {forwardRef, HTMLProps, PropsWithoutRef, RefAttributes} from 'react';
 import classNames from 'classnames';
 
 const ErrorSummaryTitle: React.FC<HTMLProps<HTMLHeadingElement>> = ({ className, ...rest }) => (
@@ -19,20 +19,23 @@ const ErrorSummaryListItem: React.FC<HTMLProps<HTMLAnchorElement>> = (props) =>
   </li>
 );
 
-interface ErrorSummary extends React.FC<HTMLProps<HTMLDivElement>> {
+interface ErrorSummary extends React.ForwardRefExoticComponent<PropsWithoutRef<HTMLProps<HTMLDivElement>> & RefAttributes<HTMLDivElement>> {
   Title: React.FC<HTMLProps<HTMLHeadingElement>>;
   Body: React.FC<HTMLProps<HTMLDivElement>>;
   List: React.FC<HTMLProps<HTMLUListElement>>;
   Item: React.FC<HTMLProps<HTMLAnchorElement>>;
 }
 
-const ErrorSummary: ErrorSummary = ({ className, ...rest }) => (
-  <div className={classNames('nhsuk-error-summary', className)} {...rest} />
-);
-
-ErrorSummary.Title = ErrorSummaryTitle;
-ErrorSummary.Body = ErrorSummaryBody;
-ErrorSummary.List = ErrorSummaryList;
-ErrorSummary.Item = ErrorSummaryListItem;
+const ErrorSummaryDiv = forwardRef<HTMLDivElement, HTMLProps<HTMLDivElement>>(({className, ...rest}, ref) =>
+  <div className={classNames('nhsuk-error-summary', className)} ref={ref} {...rest} />
+)
+ErrorSummaryDiv.displayName = "ErrorSummary"
+
+const ErrorSummary: ErrorSummary = Object.assign(ErrorSummaryDiv, {
+  Title: ErrorSummaryTitle,
+  Body: ErrorSummaryBody,
+  List: ErrorSummaryList,
+  Item: ErrorSummaryListItem,
+})
 
 export default ErrorSummary;

src/components/error-summary/__tests__/ErrorSummary.test.tsx

@@ -1,5 +1,5 @@
 import React from 'react';
-import { shallow } from 'enzyme';
+import { mount, shallow } from 'enzyme';
 import ErrorSummary from '..';
 
 describe('ErrorSummary', () => {
@@ -9,6 +9,13 @@ describe('ErrorSummary', () => {
     element.unmount();
   });
 
+  it('forwards refs', () => {
+    const ref = React.createRef<HTMLDivElement>();
+    const element = mount(<ErrorSummary ref={ref}/>);
+    expect(ref.current).not.toBeNull();
+    element.unmount();
+  });
+
   describe('ErrorSummary.Title', () => {
     it('matches snapshot', () => {
       const element = shallow(<ErrorSummary.Title>Title</ErrorSummary.Title>);

src/components/footer/Footer.tsx

@@ -1,7 +1,6 @@
 import React, { HTMLProps } from 'react';
 import classNames from 'classnames';
 import { Container } from '../layout';
-
 type FooterListProps = HTMLProps<HTMLOListElement>;
 
 const FooterList: React.FC<FooterListProps> = ({ className, ...rest }) => (

src/components/header/components/MenuToggle.tsx

@@ -1,6 +1,4 @@
-import React, {
-  HTMLProps, useContext, useEffect, MouseEvent,
-} from 'react';
+import React, { HTMLProps, useContext, useEffect, MouseEvent } from 'react';
 import classNames from 'classnames';
 import HeaderContext, { IHeaderContext } from '../HeaderContext';
 
@@ -28,7 +26,7 @@ const MenuToggle: React.FC<MenuToggleProps> = ({ onClick, ...rest }) => {
     <div className="nhsuk-header__menu">
       <button
         className={classNames('nhsuk-header__menu-toggle', { 'is-active': menuOpen })}
-        aria-label="Open menu"
+        aria-controls="header-navigation"
         aria-expanded={menuOpen ? 'true' : 'false'}
         onClick={onToggleClick}
         {...rest}

src/components/header/components/Nav.tsx

@@ -5,7 +5,11 @@ import { Close as CloseIcon } from '../../icons';
 import HeaderContext, { IHeaderContext } from '../HeaderContext';
 
 const Nav: React.FC<HTMLProps<HTMLDivElement>> = ({
-  className, children, open, ...rest
+  className,
+  children,
+  open,
+  id = 'header-navigation',
+  ...rest
 }) => {
   const { menuOpen, toggleMenu } = useContext<IHeaderContext>(HeaderContext);
 
@@ -16,6 +20,7 @@ const Nav: React.FC<HTMLProps<HTMLDivElement>> = ({
         { 'js-show': open !== undefined ? open : menuOpen },
         className,
       )}
+      id={id}
       {...rest}
     >
       <Container>

src/components/header/components/NavContainer.tsx

@@ -7,6 +7,7 @@ const NavContainer: React.FC<HTMLProps<HTMLDivElement>> = ({
   className,
   children,
   open,
+  id = 'header-navigation',
   ...rest
 }) => {
   const { menuOpen } = useContext<IHeaderContext>(HeaderContext);
@@ -18,11 +19,10 @@ const NavContainer: React.FC<HTMLProps<HTMLDivElement>> = ({
         { 'js-show': open !== undefined ? open : menuOpen },
         className,
       )}
+      id={id}
       {...rest}
     >
-      <Container>
-        {children}
-      </Container>
+      <Container>{children}</Container>
     </nav>
   );
 };

src/components/table/TableHelpers.ts

@@ -6,12 +6,11 @@ export const isTableCell = (child: ReactNode): child is ReactElement => {
 };
 
 export const getHeadingsFromChildren = (children: ReactNode): string[] => {
-  return React.Children
-    .map(children, child => {
-      if (isTableCell(child)) {
-        return child.props.children.toString();
-      }
-      return null;
-    })
-    .filter(Boolean);
+  const headings: string[] = [];
+  React.Children.map(children, (child) => {
+    if (isTableCell(child)) {
+      headings.push(child.props.children.toString());
+    }
+  });
+  return headings;
 };

src/components/warning-callout/__tests__/__snapshots__/WarningCallout.test.tsx.snap

@@ -10,7 +10,6 @@ exports[`WarningCallout matches snapshot 1`] = `
     >
       <HeadingLevel
         className="nhsuk-warning-callout__label"
-        headingLevel="h3"
       >
         <h3
           className="nhsuk-warning-callout__label"

src/deprecated/warnings.ts

@@ -1,3 +1,5 @@
-export const PromoDeprecationWarning = 'The Promo component is deprecated, and will be removed in the next major version of nhsuk-react-components. The Card component is the intended replacement.';
+export const PromoDeprecationWarning =
+  'The Promo component is deprecated, and will be removed in the next major version of nhsuk-react-components. The Card component is the intended replacement.';
 
-export const PanelDeprecationWarning = 'The Promo component is deprecated, and will be removed in the next major version of nhsuk-react-components. The Card component is the intended replacement.';
+export const PanelDeprecationWarning =
+  'The Promo component is deprecated, and will be removed in the next major version of nhsuk-react-components. The Card component is the intended replacement.';

src/util/HeadingLevel.tsx

@@ -18,7 +18,7 @@ export type HeadingLevelType =
   | 'H5'
   | 'H6';
 
-const HeadingLevel: React.FC<HeadingLevelProps> = ({ headingLevel, ...rest }) => {
+const HeadingLevel: React.FC<HeadingLevelProps> = ({ headingLevel='h3', ...rest }) => {
   switch (headingLevel.toLowerCase()) {
     case 'h1':
       return <h1 {...rest} />;
@@ -32,10 +32,10 @@ const HeadingLevel: React.FC<HeadingLevelProps> = ({ headingLevel, ...rest }) =>
       return <h5 {...rest} />;
     case 'h6':
       return <h6 {...rest} />;
+    default:
+      console.error(`HeadingLevel: Invalid headingLevel prop: ${headingLevel}`);
+      return <h3 {...rest} />;
   }
 };
-HeadingLevel.defaultProps = {
-  headingLevel: 'h3',
-};
 
 export default HeadingLevel;

src/util/__tests__/HeadingLevel.test.tsx

@@ -43,4 +43,15 @@ describe('HeadingLevel', () => {
     h6Element.unmount();
     H6Element.unmount();
   });
+
+  it("console.warn when headingLevel is invalid", () => {
+    // eslint-disable-next-line @typescript-eslint/no-empty-function
+    const consoleSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
+    // @ts-expect-error - testing invalid prop
+    shallow(<HeadingLevel headingLevel="h7" />);
+    expect(consoleSpy).toHaveBeenCalledTimes(1);
+    expect(consoleSpy).toHaveBeenCalledWith(
+      'HeadingLevel: Invalid headingLevel prop: h7');
+    consoleSpy.mockRestore();
+  });
 });

tsconfig.json

@@ -12,7 +12,8 @@
         "types": ["react", "jest", "node"],
         "allowSyntheticDefaultImports": true,
         "esModuleInterop": true,
-        "skipLibCheck": true
+        "skipLibCheck": true,
+        "strictNullChecks": true
     },
     "include": ["src/**/*"],
     "exclude": [

yarn.lock

@@ -3530,16 +3530,11 @@ ansi-escapes@^4.2.1:
     type-fest "^0.21.3"
 
 ansi-regex@^4.1.0:
-  version "4.1.0"
-  resolved "https://registry.yarnpkg.com/ansi-regex/-/ansi-regex-4.1.0.tgz#8b9f8f08cf1acb843756a839ca8c7e3168c51997"
-  integrity sha512-1apePfXM1UOSqw0o9IiFAovVz9M5S1Dg+4TrDwfMewQ6p/rmMueb7tWZjQ1rx4Loy1ArBggoqGpfqqdI4rondg==
-
-ansi-regex@^5.0.0:
-  version "5.0.0"
-  resolved "https://registry.yarnpkg.com/ansi-regex/-/ansi-regex-5.0.0.tgz#388539f55179bf39339c81af30a654d69f87cb75"
-  integrity sha512-bY6fj56OUQ0hU1KjFNDQuJFezqKdrAyFdIevADiqrWHwSlbmBNMHp5ak2f40Pm8JTFyM2mqxkG6ngkHO11f/lg==
+  version "4.1.1"
+  resolved "https://registry.yarnpkg.com/ansi-regex/-/ansi-regex-4.1.1.tgz#164daac87ab2d6f6db3a29875e2d1766582dabed"
+  integrity sha512-ILlv4k/3f6vfQ4OoP2AGvirOktlQ98ZEL1k9FaQjxa3L1abBgbuTDAdPOpvbGncC0BTVQrl+OM8xZGK6tWXt7g==
 
-ansi-regex@^5.0.1:
+ansi-regex@^5.0.0, ansi-regex@^5.0.1:
   version "5.0.1"
   resolved "https://registry.yarnpkg.com/ansi-regex/-/ansi-regex-5.0.1.tgz#082cb2c89c9fe8659a311a53bd6a4dc5301db304"
   integrity sha512-quJQXlTSUGL2LH9SUXo8VwsY4soanhgo6LNSm84E1LBcE8s3O0wpdiRzyR9z/ZZJMlMWv37qOOb9pdJlMUEKFQ==
@@ -8165,14 +8160,7 @@ min-indent@^1.0.0:
   resolved "https://registry.yarnpkg.com/min-indent/-/min-indent-1.0.1.tgz#a63f681673b30571fbe8bc25686ae746eefa9869"
   integrity sha512-I9jwMn07Sy/IwOj3zVkVik2JTvgpaykDZEigL6Rx6N9LbMywwUSMtxET+7lVoDLLd3O3IXwJwvuuns8UB/HeAg==
 
-minimatch@^3.0.2, minimatch@^3.0.4:
-  version "3.0.4"
-  resolved "https://registry.yarnpkg.com/minimatch/-/minimatch-3.0.4.tgz#5166e286457f03306064be5497e8dbb0c3d32083"
-  integrity sha512-yJHVQEhyqPLUTgt9B83PXu6W3rx4MvvHvSUvToogpwoGDOUQ+yDrR0HRot+yOCdCO7u4hX3pWft6kWBBcqh0UA==
-  dependencies:
-    brace-expansion "^1.1.7"
-
-minimatch@^3.1.1:
+minimatch@^3.0.2, minimatch@^3.0.4, minimatch@^3.1.1:
   version "3.1.2"
   resolved "https://registry.yarnpkg.com/minimatch/-/minimatch-3.1.2.tgz#19cd194bfd3e428f049a70817c038d89ab4be35b"
   integrity sha512-J7p63hRiAjw1NDEww1W7i37+ByIrOWO5XQQAzZ3VOcL0PNybwpfmV/N05zFAzwQ9USyEcX6t3UO+K5aqBQOIHw==