From b4ef32811a0a150e6f927f96bac49a167eb6193a Mon Sep 17 00:00:00 2001
From: Constance
Date: Fri, 23 Sep 2022 14:43:48 -0700
Subject: [PATCH] [EuiSkipLink] Add `fallbackDestination` support, defaulting
to `main` tag (#6261)
* Solidify EuiSkipLink as always being an `a` tag
- remove PropsForButton/ExclusiveUnion typing
- always pass a `href`, even if it's just `href="#"`
- simplify optionalProps to just being an optional onClick
* Add support for `fallbackDestination` query selectors, defaulting to the `main` tag
* [setup] Convert all render tests to RTL
* Add unit tests for fallbackDestination
* [docs] improve skip link docs
- remove callout about skip link not working on our doc
- remove isFixed toggle, since our docs now have a skip link
- make example work even in codesandbox, and add example with `main` fallback behavior
* Fix custom `onClick`s overriding ours instead of being called after
* [perf] optimize onClick to a memoized callback, and fix consumer onClick to always be called
* changelog
---
.../accessibility/accessibility_example.js | 5 +-
.../src/views/accessibility/skip_link.tsx | 42 +++------
.../__snapshots__/skip_link.test.tsx.snap | 12 ---
.../skip_link/skip_link.test.tsx | 90 ++++++++++++++++---
.../accessibility/skip_link/skip_link.tsx | 76 ++++++++++------
upcoming_changelogs/6261.md | 6 ++
6 files changed, 144 insertions(+), 87 deletions(-)
create mode 100644 upcoming_changelogs/6261.md
diff --git a/src-docs/src/views/accessibility/accessibility_example.js b/src-docs/src/views/accessibility/accessibility_example.js
index 202d6805b16..60ee25bfede 100644
--- a/src-docs/src/views/accessibility/accessibility_example.js
+++ b/src-docs/src/views/accessibility/accessibility_example.js
@@ -215,8 +215,9 @@ export const AccessibilityExample = {
navigation, or ornamental elements, and quickly reach the main
content of the page. It requires a destinationId{' '}
which should match the id of your main content.
- You can also change the position to{' '}
- fixed.
+ If your ID does not correspond to a valid element, the skip link
+ will fall back to focusing the {''} tag on
+ your page, if it exists.
diff --git a/src-docs/src/views/accessibility/skip_link.tsx b/src-docs/src/views/accessibility/skip_link.tsx
index 44bce0567b8..0bce49d434e 100644
--- a/src-docs/src/views/accessibility/skip_link.tsx
+++ b/src-docs/src/views/accessibility/skip_link.tsx
@@ -1,39 +1,17 @@
-import React, { useState } from 'react';
+import React from 'react';
-import {
- EuiSkipLink,
- EuiCallOut,
- EuiSpacer,
- EuiSwitch,
-} from '../../../../src/components';
+import { EuiSkipLink, EuiText } from '../../../../src/components';
export default () => {
- const [isFixed, setFixed] = useState(false);
-
return (
- <>
- setFixed(e.target.checked)}
- />
-
-
- Skip to {isFixed && 'main '}content
+
+ The following skip links are only visible on focus:
+
+ Skips to this example container
+
+
+ Falls back to main container
- {isFixed && (
- <>
-
- >
- )}
- >
+
);
};
diff --git a/src/components/accessibility/skip_link/__snapshots__/skip_link.test.tsx.snap b/src/components/accessibility/skip_link/__snapshots__/skip_link.test.tsx.snap
index 10f6e9e4a4e..9f81dada84c 100644
--- a/src/components/accessibility/skip_link/__snapshots__/skip_link.test.tsx.snap
+++ b/src/components/accessibility/skip_link/__snapshots__/skip_link.test.tsx.snap
@@ -20,18 +20,6 @@ exports[`EuiSkipLink is rendered 1`] = `
`;
-exports[`EuiSkipLink props onClick is rendered 1`] = `
-
-
-
-`;
-
exports[`EuiSkipLink props position absolute is rendered 1`] = `
{
test('is rendered', () => {
- const component = render(
+ const { container } = render(
Skip
);
- expect(component).toMatchSnapshot();
+ expect(container.firstChild).toMatchSnapshot();
});
describe('props', () => {
@@ -57,32 +59,96 @@ describe('EuiSkipLink', () => {
component.find('a').simulate('click');
expect(scrollSpy).toHaveBeenCalled();
});
+
+ afterAll(() => jest.restoreAllMocks());
+ });
+
+ describe('fallbackDestination', () => {
+ it('falls back to focusing the main tag if destinationId is invalid', () => {
+ const { getByText } = render(
+ <>
+ Skip to content
+ I am content
+ >
+ );
+ fireEvent.click(getByText('Skip to content'));
+
+ const expectedFocus = document.querySelector('main');
+ expect(document.activeElement).toEqual(expectedFocus);
+ });
+
+ it('supports multiple query selectors', () => {
+ const { getByText } = render(
+ <>
+
+ Skip to content
+
+ I am content
+ >
+ );
+ fireEvent.click(getByText('Skip to content'));
+
+ const expectedFocus = document.querySelector('[role=main]');
+ expect(document.activeElement).toEqual(expectedFocus);
+ });
});
test('tabIndex is rendered', () => {
- const component = render(
+ const { container } = render(
);
- expect(component).toMatchSnapshot();
+ expect(container.firstChild).toMatchSnapshot();
});
- test('onClick is rendered', () => {
- const component = render(
- {}} />
- );
+ describe('onClick', () => {
+ test('is always called', () => {
+ const onClick = jest.fn();
+ const { getByText } = render(
+ <>
+
+ Test
+
+
+ >
+ );
+ fireEvent.click(getByText('Test'));
+
+ expect(onClick).toHaveBeenCalled();
+ });
+
+ test('does not override overrideLinkBehavior', () => {
+ const onClick = jest.fn();
+ const { getByText } = render(
+ <>
+
+ Test
+
+
+ >
+ );
+ fireEvent.click(getByText('Test'));
- expect(component).toMatchSnapshot();
+ expect(document.activeElement?.id).toEqual('somewhere');
+ expect(onClick).toHaveBeenCalled();
+ });
});
describe('position', () => {
POSITIONS.forEach((position) => {
test(`${position} is rendered`, () => {
- const component = render(
+ const { container } = render(
);
- expect(component).toMatchSnapshot();
+ expect(container.firstChild).toMatchSnapshot();
});
});
});
diff --git a/src/components/accessibility/skip_link/skip_link.tsx b/src/components/accessibility/skip_link/skip_link.tsx
index a11591f2a82..88a487917f5 100644
--- a/src/components/accessibility/skip_link/skip_link.tsx
+++ b/src/components/accessibility/skip_link/skip_link.tsx
@@ -6,12 +6,18 @@
* Side Public License, v 1.
*/
-import React, { FunctionComponent, Ref } from 'react';
+import React, {
+ FunctionComponent,
+ Ref,
+ useState,
+ useEffect,
+ useCallback,
+} from 'react';
import classNames from 'classnames';
import { isTabbable } from 'tabbable';
import { useEuiTheme } from '../../../services';
import { EuiButton, EuiButtonProps } from '../../button/button';
-import { PropsForAnchor, PropsForButton, ExclusiveUnion } from '../../common';
+import { PropsForAnchor } from '../../common';
import { EuiScreenReaderOnly } from '../screen_reader_only';
import { euiSkipLinkStyles } from './skip_link.styles';
@@ -29,6 +35,12 @@ interface EuiSkipLinkInterface extends EuiButtonProps {
* will be prepended with a hash `#` and used as the link `href`
*/
destinationId: string;
+ /**
+ * If no destination ID element exists or can be found, you may provide a string of
+ * query selectors to fall back to (e.g. a `main` or `role="main"` element)
+ * @default main
+ */
+ fallbackDestination?: string;
/**
* If default HTML anchor link behavior is not desired (e.g. for SPAs with hash routing),
* setting this flag to true will manually scroll to and focus the destination element
@@ -41,29 +53,22 @@ interface EuiSkipLinkInterface extends EuiButtonProps {
tabIndex?: number;
}
-type propsForAnchor = PropsForAnchor<
+export type EuiSkipLinkProps = PropsForAnchor<
EuiSkipLinkInterface,
{
buttonRef?: Ref;
}
>;
-type propsForButton = PropsForButton<
- EuiSkipLinkInterface,
- {
- buttonRef?: Ref;
- }
->;
-
-export type EuiSkipLinkProps = ExclusiveUnion;
-
export const EuiSkipLink: FunctionComponent = ({
destinationId,
+ fallbackDestination = 'main',
overrideLinkBehavior,
tabIndex,
position = 'static',
children,
className,
+ onClick: _onClick,
...rest
}) => {
const euiTheme = useEuiTheme();
@@ -76,21 +81,30 @@ export const EuiSkipLink: FunctionComponent = ({
position !== 'static' ? styles[position] : undefined,
];
- // Create the `href` from `destinationId`
- let optionalProps = {};
- if (destinationId) {
- optionalProps = {
- href: `#${destinationId}`,
- };
- }
- if (overrideLinkBehavior) {
- optionalProps = {
- ...optionalProps,
- onClick: (e: React.MouseEvent) => {
- e.preventDefault();
+ const [destinationEl, setDestinationEl] = useState(null);
+ const [hasValidId, setHasValidId] = useState(true);
+
+ useEffect(() => {
+ const idEl = document.getElementById(destinationId);
+ if (idEl) {
+ setHasValidId(true);
+ setDestinationEl(idEl);
+ return;
+ }
+ setHasValidId(false);
+
+ // If no valid element via ID is available, use the fallback query selectors
+ const fallbackEl = document.querySelector(fallbackDestination);
+ if (fallbackEl) {
+ setDestinationEl(fallbackEl);
+ }
+ }, [destinationId, fallbackDestination]);
- const destinationEl = document.getElementById(destinationId);
+ const onClick = useCallback(
+ (e: React.MouseEvent) => {
+ if (overrideLinkBehavior || !hasValidId) {
if (!destinationEl) return;
+ e.preventDefault();
// Scroll to the top of the destination content only if it's ~mostly out of view
const destinationY = destinationEl.getBoundingClientRect().top;
@@ -113,9 +127,12 @@ export const EuiSkipLink: FunctionComponent = ({
}
destinationEl.focus({ preventScroll: true }); // Scrolling is already handled above, and focus autoscroll behaves oddly on Chrome around fixed headers
- },
- };
- }
+ }
+
+ _onClick?.(e);
+ },
+ [overrideLinkBehavior, hasValidId, destinationEl, _onClick]
+ );
return (
@@ -125,7 +142,8 @@ export const EuiSkipLink: FunctionComponent = ({
tabIndex={position === 'fixed' ? 0 : tabIndex}
size="s"
fill
- {...optionalProps}
+ href={`#${destinationId}`}
+ onClick={onClick}
{...rest}
>
{children}
diff --git a/upcoming_changelogs/6261.md b/upcoming_changelogs/6261.md
new file mode 100644
index 00000000000..5b18f92cbfa
--- /dev/null
+++ b/upcoming_changelogs/6261.md
@@ -0,0 +1,6 @@
+- Added the `fallbackDestination` prop to `EuiSkipLink`, which accepts a string of query selectors to fall back to if the `destinationId` does not have a valid target. Defaults to `main`
+- `EuiSkipLink` is now always an `a` tag to ensure that it is always placed within screen reader link menus.
+
+**Bug fixes**
+
+- Fixed custom `onClick`s passed to `EuiSkipLink` overriding `overrideLinkBehavior`