Fix issues with SkipLink and align to NHSUK frontend (#248)

Author: jakeb-nhs

src/components/navigation/skip-link/SkipLink.tsx

@@ -2,14 +2,19 @@ import React, { HTMLProps, MouseEvent, useEffect } from 'react';
 import classNames from 'classnames';
 
 interface SkipLinkProps extends HTMLProps<HTMLAnchorElement> {
+  /** The reference to the element to set focus to when the link is clicked */
   focusTargetRef?: React.RefObject<HTMLElement>;
+  /** Disables the default anchor click behaviour, avoiding navigation entries being added to the browser history */
   disableDefaultBehaviour?: boolean;
+  /** Disables focusing the first h1 level heading when {@link focusTargetRef} is not set */
+  disableHeadingFocus?: boolean;
 }
 
 const SkipLink = ({
   children = 'Skip to main content',
   className,
   disableDefaultBehaviour,
+  disableHeadingFocus,
   focusTargetRef,
   href = '#maincontent',
   tabIndex = 0,
@@ -65,7 +70,7 @@ const SkipLink = ({
     if (disableDefaultBehaviour) event.preventDefault();
     if (focusTargetRef && focusTargetRef.current) {
       focusElement(focusTargetRef.current);
-    } else if (!disableDefaultBehaviour) {
+    } else if (!disableHeadingFocus) {
       // Follow the default NHSUK Frontend behaviour, but go about it in a safer way.
       // https://github.com/nhsuk/nhsuk-frontend/blob/master/packages/components/skip-link/skip-link.js
       if (firstHeadingElement) focusElement(firstHeadingElement);
@@ -80,7 +85,7 @@ const SkipLink = ({
     <a
       className={classNames('nhsuk-skip-link', className)}
       onClick={focusTarget}
-      href={disableDefaultBehaviour ? undefined : href}
+      href={href}
       tabIndex={tabIndex}
       {...rest}
     >

src/components/navigation/skip-link/__tests__/SkipLink.test.tsx

@@ -22,10 +22,44 @@ describe('SkipLink', () => {
     expect(container).toMatchSnapshot('SkipLink');
   });
 
-  it('sets the href to #maincontent by default', () => {
-    const { container } = render(<SkipLink />);
+  it('sets the href to #maincontent by default and focuses the first heading', () => {
+    const { container } = render(
+      <>
+        <SkipLink />
+        <h1 id="heading">Heading</h1>
+      </>,
+    );
+
+    const headingEl = container.querySelector('#heading') as HTMLElement;
+    const focusSpy = jest.spyOn(headingEl, 'focus');
+
+    const skipLinkEl = container.querySelector('.nhsuk-skip-link')!;
+
+    expect(skipLinkEl.getAttribute('href')).toBe('#maincontent');
 
-    expect(container.querySelector('.nhsuk-skip-link')?.getAttribute('href')).toBe('#maincontent');
+    fireEvent.click(skipLinkEl);
+
+    expect(focusSpy).toHaveBeenCalled();
+  });
+
+  it('Does not focus the first heading if disableHeadingFocus is set', () => {
+    const { container } = render(
+      <>
+        <SkipLink disableHeadingFocus />
+        <h1 id="heading">Heading</h1>
+      </>,
+    );
+
+    const headingEl = container.querySelector('#heading') as HTMLElement;
+    const focusSpy = jest.spyOn(headingEl, 'focus');
+
+    const skipLinkEl = container.querySelector('.nhsuk-skip-link')!;
+
+    expect(skipLinkEl.getAttribute('href')).toBe('#maincontent');
+
+    fireEvent.click(skipLinkEl);
+
+    expect(focusSpy).not.toHaveBeenCalled();
   });
 
   it('calls onClick callback when clicked', () => {
@@ -38,12 +72,6 @@ describe('SkipLink', () => {
     expect(onClick).toHaveBeenCalled();
   });
 
-  it('does not set the href when disableDefaultBehaviour is set', () => {
-    const { container } = render(<SkipLink disableDefaultBehaviour />);
-
-    expect(container.querySelector('.nhsuk-skip-link')?.getAttribute('href')).toBeFalsy();
-  });
-
   it('Focuses the main content when clicked', () => {
     const { container } = render(<SkipLinkTestApp />);
 

stories/Navigation/SkipLink.stories.tsx

@@ -25,17 +25,20 @@ const CodeText: React.FC<{ children: React.ReactNode }> = ({
 const meta: Meta<typeof SkipLink> = {
   title: 'Navigation/SkipLink',
   component: SkipLink,
-  argTypes: {
-    focusTargetRef: { table: { disable: true } },
-  },
+
   render: (args) => (
     <>
       <HintText>
         Press
         <CodeText>tab</CodeText>
         to show the SkipLink
       </HintText>
-      <SkipLink disableDefaultBehaviour={args.disableDefaultBehaviour} />
+      <SkipLink
+        disableDefaultBehaviour={args.disableDefaultBehaviour}
+        disableHeadingFocus={args.disableHeadingFocus}
+      />
+      <h1>Page heading</h1>
+      <div id="#maincontent">This is the main content</div>
     </>
   ),
 };
@@ -46,14 +49,20 @@ type Story = StoryObj<typeof SkipLink>;
 export const Standard: Story = {
   args: {
     disableDefaultBehaviour: false,
-  },
-  argTypes: {
-    disableDefaultBehaviour: { table: { disable: true } },
+    disableHeadingFocus: false,
   },
 };
 
 export const SkipLinkWithDefaultBehaviourDisabled: Story = {
   args: {
     disableDefaultBehaviour: true,
+    disableHeadingFocus: false,
+  },
+};
+
+export const SkipLinkWithHeadingFocusDisabled: Story = {
+  args: {
+    disableDefaultBehaviour: false,
+    disableHeadingFocus: true,
   },
 };