[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
elastic · Sep 23, 2022 · b4ef328 · b4ef328
1 parent 08c4ac4
commit b4ef328
Show file tree

Hide file tree

Showing 6 changed files with 144 additions and 87 deletions.
diff --git 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 <EuiCode>destinationId</EuiCode>{' '}
             which should match the <EuiCode>id</EuiCode> of your main content.
-            You can also change the <EuiCode>position</EuiCode> to{' '}
-            <EuiCode>fixed</EuiCode>.
+            If your ID does not correspond to a valid element, the skip link
+            will fall back to focusing the <EuiCode>{'<main>'}</EuiCode> tag on
+            your page, if it exists.
           </p>
           <p>
             <em>

diff --git 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 (
-    <>
-      <EuiSwitch
-        label="Fix link to top of screen"
-        checked={isFixed}
-        onChange={(e) => setFixed(e.target.checked)}
-      />
-      <EuiSpacer />
-      <EuiSkipLink
-        destinationId="/utilities/accessibility"
-        position={isFixed ? 'fixed' : 'static'}
-        data-test-subj="skip-link-demo-subj"
-      >
-        Skip to {isFixed && 'main '}content
+    <EuiText id="skip-link-example">
+      <p>The following skip links are only visible on focus:</p>
+      <EuiSkipLink destinationId="skip-link-example" overrideLinkBehavior>
+        Skips to this example container
+      </EuiSkipLink>
+      <EuiSkipLink destinationId="" overrideLinkBehavior>
+        Falls back to main container
       </EuiSkipLink>
-      {isFixed && (
-        <>
-          <EuiCallOut
-            size="s"
-            title="A functional &lsquo;Skip to main content&rsquo; link will be added to the EUI docs site once our URL format is updated."
-            iconType="iInCircle"
-          />
-        </>
-      )}
-    </>
+    </EuiText>
   );
 };
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
@@ -20,18 +20,6 @@ exports[`EuiSkipLink is rendered 1`] = `
 </a>
 `;
 
-exports[`EuiSkipLink props onClick is rendered 1`] = `
-<a
-  class="euiSkipLink emotion-euiButtonDisplay-s-s-fill-primary-euiSkipLink-euiScreenReaderOnly"
-  href="#somewhere"
-  rel="noreferrer"
->
-  <span
-    class="emotion-euiButtonDisplayContent"
-  />
-</a>
-`;
-
 exports[`EuiSkipLink props position absolute is rendered 1`] = `
 <a
   class="euiSkipLink emotion-euiButtonDisplay-s-s-fill-primary-euiSkipLink-absolute-euiScreenReaderOnly"

diff --git a/src/components/accessibility/skip_link/skip_link.test.tsx b/src/components/accessibility/skip_link/skip_link.test.tsx
@@ -7,20 +7,22 @@
  */
 
 import React from 'react';
-import { render, mount } from 'enzyme';
+import { mount } from 'enzyme';
+import { fireEvent } from '@testing-library/dom';
+import { render } from '../../../test/rtl';
 import { requiredProps } from '../../../test';
 
 import { EuiSkipLink, POSITIONS } from './skip_link';
 
 describe('EuiSkipLink', () => {
   test('is rendered', () => {
-    const component = render(
+    const { container } = render(
       <EuiSkipLink destinationId="somewhere" {...requiredProps}>
         Skip
       </EuiSkipLink>
     );
 
-    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(
+          <>
+            <EuiSkipLink destinationId="">Skip to content</EuiSkipLink>
+            <main>I am content</main>
+          </>
+        );
+        fireEvent.click(getByText('Skip to content'));
+
+        const expectedFocus = document.querySelector('main');
+        expect(document.activeElement).toEqual(expectedFocus);
+      });
+
+      it('supports multiple query selectors', () => {
+        const { getByText } = render(
+          <>
+            <EuiSkipLink
+              destinationId=""
+              fallbackDestination="main, [role=main]"
+            >
+              Skip to content
+            </EuiSkipLink>
+            <div role="main">I am content</div>
+          </>
+        );
+        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(
         <EuiSkipLink destinationId="somewhere" tabIndex={-1} />
       );
 
-      expect(component).toMatchSnapshot();
+      expect(container.firstChild).toMatchSnapshot();
     });
 
-    test('onClick is rendered', () => {
-      const component = render(
-        <EuiSkipLink destinationId="somewhere" onClick={() => {}} />
-      );
+    describe('onClick', () => {
+      test('is always called', () => {
+        const onClick = jest.fn();
+        const { getByText } = render(
+          <>
+            <EuiSkipLink destinationId="somewhere" onClick={onClick}>
+              Test
+            </EuiSkipLink>
+            <div id="somewhere" />
+          </>
+        );
+        fireEvent.click(getByText('Test'));
+
+        expect(onClick).toHaveBeenCalled();
+      });
+
+      test('does not override overrideLinkBehavior', () => {
+        const onClick = jest.fn();
+        const { getByText } = render(
+          <>
+            <EuiSkipLink
+              destinationId="somewhere"
+              overrideLinkBehavior
+              onClick={onClick}
+            >
+              Test
+            </EuiSkipLink>
+            <div id="somewhere" />
+          </>
+        );
+        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(
             <EuiSkipLink destinationId="somewhere" position={position} />
           );
 
-          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
@@ -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<HTMLAnchorElement>;
   }
 >;
 
-type propsForButton = PropsForButton<
-  EuiSkipLinkInterface,
-  {
-    buttonRef?: Ref<HTMLButtonElement>;
-  }
->;
-
-export type EuiSkipLinkProps = ExclusiveUnion<propsForAnchor, propsForButton>;
-
 export const EuiSkipLink: FunctionComponent<EuiSkipLinkProps> = ({
   destinationId,
+  fallbackDestination = 'main',
   overrideLinkBehavior,
   tabIndex,
   position = 'static',
   children,
   className,
+  onClick: _onClick,
   ...rest
 }) => {
   const euiTheme = useEuiTheme();
@@ -76,21 +81,30 @@ export const EuiSkipLink: FunctionComponent<EuiSkipLinkProps> = ({
     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<HTMLElement | null>(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<HTMLElement>(fallbackDestination);
+    if (fallbackEl) {
+      setDestinationEl(fallbackEl);
+    }
+  }, [destinationId, fallbackDestination]);
 
-        const destinationEl = document.getElementById(destinationId);
+  const onClick = useCallback(
+    (e: React.MouseEvent<HTMLAnchorElement>) => {
+      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<EuiSkipLinkProps> = ({
         }
 
         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 (
     <EuiScreenReaderOnly showOnFocus>
@@ -125,7 +142,8 @@ export const EuiSkipLink: FunctionComponent<EuiSkipLinkProps> = ({
         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
@@ -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`