updates from dev feedback

jpmorganchase · Dec 19, 2024 · df8d221 · df8d221
1 parent a157187
commit df8d221
Show file tree

Hide file tree

Showing 3 changed files with 117 additions and 48 deletions.
diff --git a/packages/lab/src/__tests__/__e2e__/skip-link/SkipLink.cy.tsx b/packages/lab/src/__tests__/__e2e__/skip-link/SkipLink.cy.tsx
@@ -8,7 +8,7 @@ const { Default } = composedStories;
 describe("GIVEN a SkipLink", () => {
   checkAccessibility(composedStories);
   describe("WHEN there is a single SkipLink", () => {
-    it("THEN it should move focus to the target element when interacted with", () => {
+    it("THEN it should move focus to the target element when clicked", () => {
       cy.mount(<Default />);
       cy.findByText(
         "Click here and press the Tab key to see the Skip Link",
@@ -20,12 +20,36 @@ describe("GIVEN a SkipLink", () => {
       cy.findByRole("link", { name: "Skip to main content" }).should(
         "be.visible",
       );
+      cy.findByRole("link", { name: "Skip to main content" }).should(
+        "be.focused",
+      );
       cy.findByRole("link", { name: "Skip to main content" }).click();
       cy.get("#main").should("be.focused");
       cy.findByRole("link", { name: "Skip to main content" }).should(
         "not.be.visible",
       );
     });
+    it("THEN it should move focus to the target element when navigating with keyboard", () => {
+      cy.mount(<Default />);
+      cy.findByText(
+        "Click here and press the Tab key to see the Skip Link",
+      ).click();
+      cy.findByRole("link", { name: "Skip to main content" }).should(
+        "not.be.visible",
+      );
+      cy.realPress("Tab");
+      cy.findByRole("link", { name: "Skip to main content" }).should(
+        "be.visible",
+      );
+      cy.findByRole("link", { name: "Skip to main content" }).should(
+        "be.focused",
+      );
+      cy.realPress(" ");
+      cy.get("#main").should("be.focused");
+      cy.findByRole("link", { name: "Skip to main content" }).should(
+        "not.be.visible",
+      );
+    });
 
     it("THEN it should hide the skip link if ref is broken", () => {
       cy.mount(<Default target="" />);

diff --git a/packages/lab/src/skip-link/SkipLink.tsx b/packages/lab/src/skip-link/SkipLink.tsx
@@ -24,7 +24,10 @@ interface SkipLinkProps extends ComponentPropsWithoutRef<"a"> {
 const withBaseName = makePrefixer("saltSkipLink");
 
 export const SkipLink = forwardRef<HTMLAnchorElement, SkipLinkProps>(
-  function SkipLink({ className, target, children, ...rest }, ref) {
+  function SkipLink(
+    { className, target, children, onKeyUp, onBlur, onClick, ...rest },
+    ref,
+  ) {
     const [isTargetAvailable, setIsTargetAvailable] = useState(false);
     const targetWindow = useWindow();
     useComponentCssInjection({
@@ -41,6 +44,9 @@ export const SkipLink = forwardRef<HTMLAnchorElement, SkipLinkProps>(
     }, [target]);
 
     const eventHandlers = useManageFocusOnTarget({
+      onKeyUp,
+      onBlur,
+      onClick,
       targetRef,
       targetClass: withBaseName("target"),
     });

diff --git a/packages/lab/src/skip-link/internal/useManageFocusOnTarget.ts b/packages/lab/src/skip-link/internal/useManageFocusOnTarget.ts
@@ -1,16 +1,37 @@
-import { type RefObject, useEffect, useMemo, useRef, useState } from "react";
+import {
+  type FocusEventHandler,
+  type KeyboardEventHandler,
+  type MouseEventHandler,
+  type RefObject,
+  useEffect,
+  useRef,
+  useState,
+} from "react";
 
 const FOCUS_TIMEOUT = 50;
 
+// Props interface
+interface ManageFocusOnTargetProps {
+  onBlur?: FocusEventHandler;
+  onClick?: MouseEventHandler;
+  onKeyUp?: KeyboardEventHandler;
+  targetRef: RefObject<HTMLElement> | undefined;
+  targetClass: string;
+}
+
+// Result interface
+interface ManageFocusOnTargetResult {
+  onBlur?: FocusEventHandler<HTMLAnchorElement>;
+  onClick?: MouseEventHandler<HTMLAnchorElement>;
+  onKeyUp?: KeyboardEventHandler<HTMLAnchorElement>;
+}
 export const useManageFocusOnTarget = ({
+  onKeyUp,
+  onBlur,
+  onClick,
   targetRef,
   targetClass,
-}: {
-  targetRef: RefObject<HTMLElement> | undefined;
-  targetClass: string;
-}):
-  | { onBlur: () => NodeJS.Timeout; onClick: () => void }
-  | Record<string, undefined> => {
+}: ManageFocusOnTargetProps): ManageFocusOnTargetResult => {
   const [target, setTarget] = useState<HTMLElement>();
 
   const hasTabIndex = useRef<boolean | string>();
@@ -22,49 +43,67 @@ export const useManageFocusOnTarget = ({
     }
   }, [targetRef]);
 
-  return useMemo(() => {
-    if (!target) {
-      return {};
+  if (!target) {
+    return {};
+  }
+
+  const addTabIndex = () => {
+    const tabIndex = target.getAttribute("tabIndex");
+    hasTabIndex.current = tabIndex || tabIndex === "0";
+
+    if (!hasTabIndex.current) {
+      shouldRemoveTabIndex.current = true;
+      target.setAttribute("tabIndex", "-1");
     }
+  };
 
-    const addTabIndex = () => {
-      const tabIndex = target.getAttribute("tabIndex");
-      hasTabIndex.current = tabIndex || tabIndex === "0";
+  const removeTabIndex = () => {
+    if (!hasTabIndex.current && shouldRemoveTabIndex.current) {
+      target.removeAttribute("tabIndex");
+    }
+  };
 
-      if (!hasTabIndex.current) {
-        shouldRemoveTabIndex.current = true;
-        target.setAttribute("tabIndex", "-1");
-      }
-    };
+  const handleFocusOnTarget = () => {
+    shouldRemoveTabIndex.current = false;
+    target.classList.add(targetClass);
+  };
 
-    const removeTabIndex = () => {
-      if (!hasTabIndex.current && shouldRemoveTabIndex.current) {
-        target.removeAttribute("tabIndex");
-      }
-    };
+  const handleBlurFromTarget = () => {
+    shouldRemoveTabIndex.current = true;
+    removeTabIndex();
+    target.classList.remove(targetClass);
+  };
 
-    const handleFocusOnTarget = () => {
-      shouldRemoveTabIndex.current = false;
-      target.classList.add(targetClass);
-    };
+  function moveToTarget() {
+    addTabIndex();
+    setTimeout(() => {
+      target?.focus();
+    }, FOCUS_TIMEOUT);
 
-    const handleBlurFromTarget = () => {
-      shouldRemoveTabIndex.current = true;
-      removeTabIndex();
-      target.classList.remove(targetClass);
-    };
-
-    return {
-      onBlur: () => setTimeout(removeTabIndex, FOCUS_TIMEOUT),
-      onClick: () => {
-        addTabIndex();
-        setTimeout(() => {
-          target.focus();
-        }, FOCUS_TIMEOUT);
-
-        target.addEventListener("focus", handleFocusOnTarget, { once: true });
-        target.addEventListener("blur", handleBlurFromTarget, { once: true });
-      },
-    };
-  }, [target, targetClass]);
+    target?.addEventListener("focus", handleFocusOnTarget, { once: true });
+    target?.addEventListener("blur", handleBlurFromTarget, { once: true });
+  }
+
+  const handleKeyUp: KeyboardEventHandler<HTMLAnchorElement> = (event) => {
+    if (event.key === "Enter" || event.key === " ") {
+      moveToTarget();
+    }
+    onKeyUp?.(event);
+  };
+
+  const handleClick: MouseEventHandler<HTMLAnchorElement> = (event) => {
+    moveToTarget();
+    onClick?.(event);
+  };
+
+  const handleBlur: FocusEventHandler<HTMLAnchorElement> = (event) => {
+    setTimeout(removeTabIndex, FOCUS_TIMEOUT);
+    onBlur?.(event);
+  };
+
+  return {
+    onBlur: handleBlur,
+    onClick: handleClick,
+    onKeyUp: handleKeyUp,
+  };
 };