Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issues with SkipLink and align to NHSUK frontend #248

Merged
merged 1 commit into from
Sep 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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,
},
};
Loading