Skip to content

Commit

Permalink
Fix issues with SkipLink and align to NHSUK frontend
Browse files Browse the repository at this point in the history
  • Loading branch information
jakeb-nhs committed Sep 3, 2024
1 parent 16d417f commit a908a79
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 18 deletions.
9 changes: 7 additions & 2 deletions src/components/navigation/skip-link/SkipLink.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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);
Expand All @@ -80,7 +85,7 @@ const SkipLink = ({
<a
className={classNames('nhsuk-skip-link', className)}
onClick={focusTarget}
href={disableDefaultBehaviour ? undefined : href}
href={href}
tabIndex={tabIndex}
{...rest}
>
Expand Down
46 changes: 37 additions & 9 deletions src/components/navigation/skip-link/__tests__/SkipLink.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand All @@ -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 />);

Expand Down
23 changes: 16 additions & 7 deletions stories/Navigation/SkipLink.stories.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>
</>
),
};
Expand All @@ -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,
},
};

0 comments on commit a908a79

Please sign in to comment.