updates from KO

jpmorganchase · Dec 12, 2024 · 1d5df7a · 1d5df7a
1 parent c326a4f
commit 1d5df7a
Show file tree

Hide file tree

Showing 7 changed files with 54 additions and 89 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,4 +1,4 @@
-import { SkipLink, SkipLinks } from "@salt-ds/lab";
+import { SkipLink } 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";
@@ -14,11 +14,9 @@ const NoTargetRef = () => {
           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>
+          <SkipLink data-testid="skipLink" target="main">
+            Skip to main content
+          </SkipLink>
           <div
             style={{
               borderTop: "2px solid grey",

diff --git a/packages/lab/src/skip-link/SkipLink.css b/packages/lab/src/skip-link/SkipLink.css
@@ -1,28 +1,35 @@
 /*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: max(var(--salt-size-base), auto);
-  white-space: nowrap;
-  background: var(--saltSkipLink-background, var(--salt-container-primary-background));
   padding: var(--salt-spacing-100) var(--salt-spacing-300);
-  box-shadow: var(--salt-overlayable-shadow);
+  outline: var(--salt-focused-outline);
   outline-offset: calc(-1 * var(--salt-focused-outlineWidth));
+  box-shadow: var(--salt-overlayable-shadow);
 }
 
-.saltSkipLink.saltLink:focus, .saltSkipLink.saltLink:hover {
-  text-decoration: var(--salt-navigable-textDecoration);
-  color:  var(--salt-content-primary-foreground);
+.saltSkipLink-target {
+  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 { forwardRef, type RefObject } from "react";
+import { forwardRef, useEffect, useRef } from "react";
 import { useManageFocusOnTarget } from "./internal/useManageFocusOnTarget";
 
 import skipLinkCss from "./SkipLink.css";
@@ -16,33 +16,43 @@ 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}
-          className={clsx(withBaseName(), className)}
-          ref={ref}
-        />
-      </li>
+      <a
+        className={clsx(withBaseName(), className)}
+        href={`#${target}`}
+        ref={ref}
+        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";
diff --git a/packages/lab/stories/skip-link/skip-link.stories.tsx b/packages/lab/stories/skip-link/skip-link.stories.tsx
@@ -7,33 +7,28 @@ import {
   StackLayout,
   Text,
 } from "@salt-ds/core";
-import { SkipLink, SkipLinks } from "@salt-ds/lab";
+import { SkipLink } from "@salt-ds/lab";
 import type { Meta, StoryFn } from "@storybook/react";
-import { useRef } from "react";
 
 export default {
   title: "Lab/Skip Link",
   component: SkipLink,
 } as Meta<typeof SkipLink>;
 
 export const Default: StoryFn<typeof SkipLink> = () => {
-  const articleRef = useRef<HTMLElement>(null);
-
   return (
     <StackLayout style={{ maxWidth: 500 }}>
       <Text tabIndex={-1}>
         Click here and press the Tab key to see the Skip Link
       </Text>
-      <div>
-        <SkipLinks>
-          <SkipLink data-testid="skipLink" href="#main" targetRef={articleRef}>
-            Skip to main content
-          </SkipLink>
-        </SkipLinks>
+      <div style={{ position: "relative" }}>
+        <SkipLink data-testid="skipLink" target="main">
+          Skip to main content
+        </SkipLink>
         <Divider />
         <H2>What we do</H2>
 
-        <article id="main" ref={articleRef}>
+        <article id="main">
           <section>
             <H3>Salt</H3>
             <p>
@@ -66,28 +61,19 @@ export const Default: StoryFn<typeof SkipLink> = () => {
 };
 
 export const MultipleLinks: StoryFn<typeof SkipLink> = () => {
-  const sectionRef1 = useRef<HTMLElement>(null);
-  const sectionRef2 = useRef<HTMLElement>(null);
-
   return (
     <StackLayout style={{ maxWidth: 500 }}>
       <Text tabIndex={-1}>
         Click here and press the Tab key to see the Skip Link
       </Text>
-      <div>
-        <SkipLinks>
-          <SkipLink href="#introduction" targetRef={sectionRef1}>
-            Skip to Introduction
-          </SkipLink>
-          <SkipLink href="#goals" targetRef={sectionRef2}>
-            Skip to Goals
-          </SkipLink>
-        </SkipLinks>
+      <div style={{ position: "relative" }}>
+        <SkipLink target="intro">Skip to Introduction</SkipLink>
+        <SkipLink target="goals">Skip to Goals</SkipLink>
         <Divider />
         <H2>What we do</H2>
 
         <article>
-          <section id="intro" ref={sectionRef1}>
+          <section id="intro">
             <H3>Salt</H3>
             <p>
               Salt provides you with a suite of UI components and a flexible
@@ -98,7 +84,7 @@ export const MultipleLinks: StoryFn<typeof SkipLink> = () => {
               variations, or in fact substitute alternate themes.
             </p>
           </section>
-          <section id="goals" ref={sectionRef2}>
+          <section id="goals">
             <H3>Goals</H3>
             <p>Salt has been developed with the following design goals:</p>
             <ul className="goalsList">
Original file line number	Diff line number	Diff line change
		@@ -1,2 +1 @@
		export * from "./SkipLink";
		export * from "./SkipLinks";