[EuiSkipLink] Improve overrideLinkBehavior UX

src/components/accessibility/skip_link/skip_link.spec.tsx

@@ -0,0 +1,148 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0 and the Server Side Public License, v 1; you may not use this file except
+ * in compliance with, at your election, the Elastic License 2.0 or the Server
+ * Side Public License, v 1.
+ */
+
+/// <reference types="../../../../cypress/support"/>
+
+import React from 'react';
+import { EuiSkipLink } from './';
+
+describe('EuiSkipLink', () => {
+  describe('overrideLinkBehavior', () => {
+    describe('focus/tab behavior', () => {
+      it('manually focuses the target element', () => {
+        cy.realMount(
+          <div>
+            <EuiSkipLink
+              destinationId="start-of-content"
+              overrideLinkBehavior
+            />
+            <main id="start-of-content">Hello world</main>
+          </div>
+        );
+
+        cy.realPress('Tab');
+        cy.focused().should('have.class', 'euiSkipLink');
+
+        cy.realPress('Enter');
+        cy.focused().should('have.id', 'start-of-content');
+      });
+
+      it('removes the manual tabindex on the target element after the user moves away from it', () => {
+        cy.realMount(
+          <div>
+            <EuiSkipLink
+              destinationId="start-of-content"
+              overrideLinkBehavior
+            />
+            <main id="start-of-content">
+              Hello world
+              <button id="button">Next focus</button>
+            </main>
+          </div>
+        );
+        cy.realPress('Tab');
+        cy.realPress('Enter');
+
+        cy.focused()
+          .should('have.id', 'start-of-content')
+          .should('have.attr', 'tabindex', '-1');
+
+        cy.realPress('Tab');
+
+        cy.focused().should('have.id', 'button');
+        cy.get('#start-of-content').should('not.have.attr', 'tabindex');
+      });
+
+      it('does not add tabindex -1 to target elements that are already tabbable', () => {
+        cy.realMount(
+          <div>
+            <EuiSkipLink destinationId="button" overrideLinkBehavior />
+            <button id="button">Button</button>
+
+            <EuiSkipLink destinationId="negativeOne" overrideLinkBehavior />
+            <div id="negativeOne" tabIndex={0}>
+              tabIndex -1
+            </div>
+          </div>
+        );
+
+        cy.realPress('Tab');
+        cy.realPress('Enter');
+        cy.focused()
+          .should('have.id', 'button')
+          .should('not.have.attr', 'tabindex', '-1');
+
+        cy.realPress('Tab');
+        cy.realPress('Enter');
+        cy.focused()
+          .should('have.id', 'negativeOne')
+          .should('have.attr', 'tabindex', '0');
+      });
+    });
+
+    describe('scroll behavior', () => {
+      it('should scroll down to the target element if it is out of the viewport', () => {
+        cy.realMount(
+          <div style={{ height: '1500px', paddingTop: '1400px' }}>
+            <EuiSkipLink
+              position="fixed"
+              destinationId="start-of-content"
+              overrideLinkBehavior
+            />
+            <main id="start-of-content">Hello world</main>
+          </div>
+        );
+        cy.window().its('scrollY').should('equal', 0);
+
+        cy.realPress('Tab');
+        cy.realPress('Enter');
+
+        cy.window().its('scrollY').should('not.equal', 0);
+      });
+
+      it('should scroll back up to the top of the target element if the user scrolled down past it', () => {
+        cy.realMount(
+          <div style={{ height: '1500px' }}>
+            <EuiSkipLink
+              position="fixed"
+              destinationId="start-of-content"
+              overrideLinkBehavior
+            />
+            <main id="start-of-content">Hello world</main>
+          </div>
+        );
+        cy.scrollTo('bottom');
+        cy.window().its('scrollY').should('not.equal', 0);
+
+        cy.realPress('Tab');
+        cy.realPress('Enter');
+
+        cy.window().its('scrollY').should('equal', 1); // Not sure why this isn't 0 - might be a Chrome thing
+      });
+
+      it('should not scroll if the element is already visible in the viewport', () => {
+        cy.realMount(
+          <div style={{ height: '1500px', paddingTop: '100px' }}>
+            <EuiSkipLink
+              position="fixed"
+              destinationId="start-of-content"
+              overrideLinkBehavior
+            />
+            <main id="start-of-content">Hello world</main>
+          </div>
+        );
+        cy.window().its('scrollY').should('equal', 0);
+
+        cy.realPress('Tab');
+        cy.realPress('Enter');
+
+        cy.window().its('scrollY').should('equal', 0);
+      });
+    });
+  });
+});

src/components/accessibility/skip_link/skip_link.test.tsx

@@ -24,24 +24,39 @@ describe('EuiSkipLink', () => {
   });
 
   describe('props', () => {
-    test('overrideLinkBehavior prevents default link behavior and manually scrolls and focuses the destination', () => {
-      const scrollSpy = jest.fn();
-      const focusSpy = jest.fn();
-      jest.spyOn(document, 'getElementById').mockReturnValue({
-        scrollIntoView: scrollSpy,
-        focus: focusSpy,
-      } as any);
-
-      const component = mount(
-        <EuiSkipLink destinationId="somewhere" overrideLinkBehavior />
-      );
+    describe('overrideLinkBehavior', () => {
+      const mockElement = document.createElement('main');
+      jest.spyOn(document, 'getElementById').mockReturnValue(mockElement);
+
+      it('prevents default link behavior and manually focuses the destination', () => {
+        const focusSpy = jest.fn();
+        mockElement.focus = focusSpy;
+
+        const component = mount(
+          <EuiSkipLink destinationId="somewhere" overrideLinkBehavior />
+        );
+
+        const preventDefault = jest.fn();
+        component.find('EuiButton').simulate('click', { preventDefault });
 
-      const preventDefault = jest.fn();
-      component.find('EuiButton').simulate('click', { preventDefault });
+        expect(preventDefault).toHaveBeenCalled();
+        expect(focusSpy).toHaveBeenCalled();
+      });
+
+      it('only scrolls to the destination if out of view', () => {
+        const scrollSpy = jest.fn();
+        mockElement.scrollIntoView = scrollSpy;
 
-      expect(preventDefault).toHaveBeenCalled();
-      expect(scrollSpy).toHaveBeenCalled();
-      expect(focusSpy).toHaveBeenCalled();
+        const component = mount(
+          <EuiSkipLink destinationId="somewhere" overrideLinkBehavior />
+        );
+        component.find('EuiButton').simulate('click');
+        expect(scrollSpy).not.toHaveBeenCalled();
+
+        mockElement.getBoundingClientRect = () => ({ top: 1000 } as any);
+        component.find('EuiButton').simulate('click');
+        expect(scrollSpy).toHaveBeenCalled();
+      });
     });
 
     test('tabIndex is rendered', () => {

src/components/accessibility/skip_link/skip_link.tsx

@@ -8,6 +8,7 @@
 
 import React, { FunctionComponent, Ref } 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';
@@ -91,9 +92,27 @@ export const EuiSkipLink: FunctionComponent<EuiSkipLinkProps> = ({
         const destinationEl = document.getElementById(destinationId);
         if (!destinationEl) return;
 
-        destinationEl.scrollIntoView();
-        destinationEl.tabIndex = -1; // Ensure the destination content is focusable
-        destinationEl.focus({ preventScroll: true }); // Scrolling is already handled above, and focus's autoscroll behaves oddly around fixed headers
+        // Scroll to the top of the destination content only if it's ~mostly out of view
+        const destinationY = destinationEl.getBoundingClientRect().top;
+        const halfOfViewportHeight = window.innerHeight / 2;
+        if (
+          destinationY >= halfOfViewportHeight ||
+          window.scrollY >= destinationY + halfOfViewportHeight
+        ) {
+          destinationEl.scrollIntoView();
+        }
+
+        // Ensure the destination content is focusable
+        if (!isTabbable(destinationEl)) {
+          destinationEl.tabIndex = -1;
+          destinationEl.addEventListener(
+            'blur',
+            () => destinationEl.removeAttribute('tabindex'),
+            { once: true }
+          );
+        }
+
+        destinationEl.focus({ preventScroll: true }); // Scrolling is already handled above, and focus autoscroll behaves oddly on Chrome around fixed headers
       },
     };
   }

upcoming_changelogs/5996.md

@@ -0,0 +1 @@
+- Enhanced `EuiSkipLink`'s `overrideLinkBehavior` scroll and focus UX