Update to skiplink

jpmorganchase · Dec 13, 2024 · f56339b · f56339b
1 parent 4bcfb82
commit f56339b
Show file tree

Hide file tree

Showing 7 changed files with 95 additions and 215 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
@@ -1,66 +1,10 @@
-import { SkipLink, SkipLinks } from "@salt-ds/lab";
 import * as skipLinkStories from "@stories/skip-link/skip-link.stories";
 import { composeStories } from "@storybook/react";
 import { checkAccessibility } from "../../../../../../cypress/tests/checkAccessibility";
 
 const composedStories = composeStories(skipLinkStories);
 const { Default, MultipleLinks } = composedStories;
 
-const NoTargetRef = () => {
-  return (
-    <>
-      <div>
-        <span style={{ height: 50, lineHeight: "50px" }} tabIndex={-1}>
-          Click here and press the Tab key to see the Skip Link
-        </span>
-        <div style={{ position: "relative", maxWidth: 500 }}>
-          <SkipLinks>
-            <SkipLink data-testid="skipLink" href="#main">
-              Skip to main content
-            </SkipLink>
-          </SkipLinks>
-          <div
-            style={{
-              borderTop: "2px solid grey",
-              fontSize: 24,
-              lineHeight: 3.5,
-            }}
-          >
-            What we do
-          </div>
-
-          <article id="main">
-            <section>
-              <h1>Salt</h1>
-              <p>
-                Salt provides you with a suite of UI components and a flexible
-                theming system. With no customisation, the default theme offers
-                an attractive and modern look-and-feel, with both light and dark
-                variants and support for a range of UI densities. We have
-                included a theming system which allows you to easily create
-                theme variations, or in fact substitute alternate themes.
-              </p>
-            </section>
-            <section>
-              <h1>Goals</h1>
-              <p>Salt has been developed with the following design goals:</p>
-              <ul>
-                <li>
-                  Providing a comprehensive set of commonly-used UI controls
-                </li>
-                <li>Complying with WCAG 2.1 accessibility guidelines</li>
-                <li> To be lightweight and performant</li>
-                <li> Offering flexible styling and theming support</li>
-                <li> Minimizing dependencies on third-party libraries</li>
-              </ul>
-            </section>
-          </article>
-        </div>
-      </div>
-    </>
-  );
-};
-
 describe("GIVEN a SkipLink", () => {
   checkAccessibility(composedStories);
   describe("WHEN there is a single SkipLink", () => {
@@ -70,22 +14,22 @@ describe("GIVEN a SkipLink", () => {
         "Click here and press the Tab key to see the Skip Link",
       ).click();
       cy.realPress("Tab");
-      cy.findByTestId("skipLink").should("be.visible");
-      cy.findByTestId("skipLink").click();
-
+      cy.findByRole("link", { name: "Skip to main content" }).should(
+        "be.visible",
+      );
+      cy.findByRole("link", { name: "Skip to main content" }).click();
       cy.get("#main").should("be.focused");
     });
 
-    it("THEN it should not move focus if no target ref is given", () => {
-      cy.mount(<NoTargetRef />);
+    it("THEN it should hide the skip link if ref is broken", () => {
+      cy.mount(<Default target="" />);
       cy.findByText(
         "Click here and press the Tab key to see the Skip Link",
       ).click();
       cy.realPress("Tab");
-      cy.findByTestId("skipLink").should("be.visible");
-      cy.findByTestId("skipLink").click();
-      cy.findByTestId("skipLink").should("not.be.focused");
-      cy.get("#main").should("not.be.focused");
+      cy.findByRole("link", { name: "Skip to main content" }).should(
+        "not.exist",
+      );
     });
   });
 
@@ -96,11 +40,14 @@ describe("GIVEN a SkipLink", () => {
         "Click here and press the Tab key to see the Skip Link",
       ).click();
       cy.realPress("Tab");
+      cy.findByRole("link", { name: "Skip to Introduction" }).should(
+        "be.visible",
+      );
       cy.realPress("Tab");
+      cy.findByRole("link", { name: "Skip to Goals" }).should("be.visible");
       cy.realPress("Enter");
 
       cy.get("#goals").should("be.focused");
-      cy.get("#introduction").should("not.be.focused");
     });
   });
 });
diff --git a/packages/lab/src/skip-link/SkipLink.css b/packages/lab/src/skip-link/SkipLink.css
@@ -1,52 +1,35 @@
-/* CSS Variables for the Skip Link */
-.saltSkipLink {
-  --skipLink-padding: var(--saltSkipLink-padding, var(--salt-size-unit));
-  --skipLink-margin: var(--saltSkipLink-margin, var(--salt-size-unit));
-  --skipLink-background: var(--saltSkipLink-background, var(--salt-actionable-primary-background));
-  --skipLink-color: var(--saltSkipLink-color, var(--salt-content-primary-foreground));
-}
-
-/* Overrides */
-.saltSkipLink {
-  --saltLink-color-focus: var(--skipLink-color);
-}
-
-.saltSkipLink-target {
-  --skipLink-target-focus: var(--salt-focused-outline);
-}
-
 /*Styles applied when the link is focused to hide the Skip Link when not in focus*/
+
 .saltSkipLink {
   top: 0;
   left: 0;
+  opacity: 0;
   width: 1px;
   height: 1px;
-  display: block;
-  opacity: 0;
+  margin: 0;
+  padding: 0;
   overflow: hidden;
   position: absolute;
+
+  color: var(--salt-content-primary-foreground);
+  letter-spacing: var(--salt-text-letterSpacing);
+  text-decoration: var(--salt-navigable-textDecoration);
+  font-family: var(--salt-text-fontFamily);
+  white-space: nowrap;
+  background: var(--saltSkipLink-background, var(--salt-container-primary-background));
 }
 
 /* Styles applied when the link is focused to display the Skip Link only when in focus*/
 .saltSkipLink:focus {
   opacity: 1;
   width: auto;
-  height: auto;
-  white-space: nowrap;
-  margin: var(--skipLink-margin);
-  padding: calc(var(--skipLink-padding) - 1px) var(--skipLink-padding) var(--skipLink-padding);
-  background: var(--skipLink-background);
-  color: var(--skipLink-color);
-  box-shadow: var(--salt-overlayable-shadow-popout);
-}
-
-.saltSkipLink {
-  font-size: var(--salt-text-fontSize);
-  font-family: var(--saltSkipLink-fontFamily, var(--salt-text-fontFamily));
-  line-height: var(--saltSkipLink-lineHeight, var(--salt-text-lineHeight));
+  height: max(var(--salt-size-base), auto);
+  padding: var(--salt-spacing-100) var(--salt-spacing-300);
+  outline: var(--salt-focused-outline);
+  outline-offset: calc(-1 * var(--salt-focused-outlineWidth));
+  box-shadow: var(--salt-overlayable-shadow);
 }
 
-/*Styles applied to the skip link focus target*/
 .saltSkipLink-target {
-  outline: var(--skipLink-target-focus);
+  outline: var(--salt-focused-outline);
 }
diff --git a/packages/lab/src/skip-link/SkipLink.tsx b/packages/lab/src/skip-link/SkipLink.tsx
@@ -1,8 +1,8 @@
-import { Link, type LinkProps, makePrefixer } from "@salt-ds/core";
+import { type LinkProps, makePrefixer } from "@salt-ds/core";
 import { useComponentCssInjection } from "@salt-ds/styles";
 import { useWindow } from "@salt-ds/window";
 import { clsx } from "clsx";
-import { type RefObject, forwardRef } from "react";
+import { forwardRef, useEffect, useRef } from "react";
 import { useManageFocusOnTarget } from "./internal/useManageFocusOnTarget";
 
 import skipLinkCss from "./SkipLink.css";
@@ -16,33 +16,45 @@ interface SkipLinkProps extends LinkProps {
    * Refs are referentially stable so if this changes it won't be picked up
    * will need to find a better way of passing in the target element to apply the attributes
    */
-  targetRef?: RefObject<HTMLElement>;
+  target: string;
 }
 
 const withBaseName = makePrefixer("saltSkipLink");
 
 export const SkipLink = forwardRef<HTMLAnchorElement, SkipLinkProps>(
-  function SkipLink({ className, targetRef, ...rest }, ref) {
+  function SkipLink({ className, target, children, ...rest }, ref) {
     const targetWindow = useWindow();
     useComponentCssInjection({
       testId: "salt-skip-link",
       css: skipLinkCss,
       window: targetWindow,
     });
 
-    const targetClass = clsx(withBaseName("target"), className);
+    const targetRef = useRef<HTMLElement | null>(null);
 
-    const eventHandlers = useManageFocusOnTarget({ targetRef, targetClass });
+    const targetClass = withBaseName("target");
+    useEffect(() => {
+      targetRef.current = document.getElementById(target);
+    }, [target]);
+
+    const eventHandlers = useManageFocusOnTarget({
+      targetRef,
+      targetClass,
+    });
 
     return (
-      <li>
-        <Link
-          {...eventHandlers}
-          {...rest}
+      targetRef.current && (
+        <a
           className={clsx(withBaseName(), className)}
+          href={`#${target}`}
           ref={ref}
-        />
-      </li>
+          target="_self"
+          {...rest}
+          {...eventHandlers}
+        >
+          {children}
+        </a>
+      )
     );
   },
 );
diff --git a/packages/lab/src/skip-link/SkipLinks.css b/packages/lab/src/skip-link/SkipLinks.css
diff --git a/packages/lab/src/skip-link/SkipLinks.tsx b/packages/lab/src/skip-link/SkipLinks.tsx
diff --git a/packages/lab/src/skip-link/index.ts b/packages/lab/src/skip-link/index.ts
@@ -1,2 +1 @@
 export * from "./SkipLink";
-export * from "./SkipLinks";
Original file line number	Diff line number	Diff line change
		@@ -1,2 +1 @@
		export * from "./SkipLink";
		export * from "./SkipLinks";