From 61e03ccef929ec0767c9b16723129a6da067056c Mon Sep 17 00:00:00 2001 From: Fernanda Castillo Date: Mon, 9 Dec 2024 11:41:13 +0000 Subject: [PATCH 1/6] Updates to SkipLink --- .changeset/gentle-hairs-repair.md | 9 + .../__e2e__/skip-link/SkipLink.cy.tsx | 97 ++----- packages/lab/src/skip-link/SkipLink.css | 56 ++-- packages/lab/src/skip-link/SkipLink.tsx | 50 ++-- packages/lab/src/skip-link/SkipLinks.css | 7 - packages/lab/src/skip-link/SkipLinks.tsx | 29 -- packages/lab/src/skip-link/index.ts | 1 - .../stories/skip-link/skip-link.stories.tsx | 247 ++++++++++-------- 8 files changed, 212 insertions(+), 284 deletions(-) create mode 100644 .changeset/gentle-hairs-repair.md delete mode 100644 packages/lab/src/skip-link/SkipLinks.css delete mode 100644 packages/lab/src/skip-link/SkipLinks.tsx diff --git a/.changeset/gentle-hairs-repair.md b/.changeset/gentle-hairs-repair.md new file mode 100644 index 00000000000..67b06a5f3e8 --- /dev/null +++ b/.changeset/gentle-hairs-repair.md @@ -0,0 +1,9 @@ +--- +"@salt-ds/lab": patch +--- + +Updates to Lab `SkipLink` + +- Deprecated `targetRef` prop, added `target` prop to accept a string representing the ID of the target element. +- Updated styling to adhere with the rest of the library styles for consistency. +- Fixed an issue where the `SkipLink` would render when the ref to the target element was broken. Now, the skip link will not render at all if the target element is not found. diff --git a/packages/lab/src/__tests__/__e2e__/skip-link/SkipLink.cy.tsx b/packages/lab/src/__tests__/__e2e__/skip-link/SkipLink.cy.tsx index 2f9bf628278..0737efdbf77 100644 --- a/packages/lab/src/__tests__/__e2e__/skip-link/SkipLink.cy.tsx +++ b/packages/lab/src/__tests__/__e2e__/skip-link/SkipLink.cy.tsx @@ -1,65 +1,9 @@ -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 ( - <> -
- - Click here and press the Tab key to see the Skip Link - -
- - - Skip to main content - - -
- What we do -
- -
-
-

Salt

-

- 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. -

-
-
-

Goals

-

Salt has been developed with the following design goals:

-
    -
  • - Providing a comprehensive set of commonly-used UI controls -
    • -
  • Complying with WCAG 2.1 accessibility guidelines
    • -
  • To be lightweight and performant
    • -
  • Offering flexible styling and theming support
    • -
  • Minimizing dependencies on third-party libraries
    • -
-
-
-
-
- - ); -}; +const { Default } = composedStories; describe("GIVEN a SkipLink", () => { checkAccessibility(composedStories); @@ -69,38 +13,29 @@ describe("GIVEN a SkipLink", () => { 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.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"); + cy.findByRole("link", { name: "Skip to main content" }).should( + "not.be.visible", + ); }); - it("THEN it should not move focus if no target ref is given", () => { - cy.mount(); - 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"); - }); - }); - - describe("WHEN there are multiple SkipLinks", () => { - it("THEN it should move focus on to correct target when the appropriate link is interacted with", () => { - cy.mount(); + it("THEN it should hide the skip link if ref is broken", () => { + cy.mount(); cy.findByText( "Click here and press the Tab key to see the Skip Link", ).click(); cy.realPress("Tab"); - cy.realPress("Tab"); - cy.realPress("Enter"); - - cy.get("#goals").should("be.focused"); - cy.get("#introduction").should("not.be.focused"); + cy.findByRole("link", { name: "Skip to main content" }).should( + "not.exist", + ); }); }); }); diff --git a/packages/lab/src/skip-link/SkipLink.css b/packages/lab/src/skip-link/SkipLink.css index da55c1c1650..57ac2f1cb29 100644 --- a/packages/lab/src/skip-link/SkipLink.css +++ b/packages/lab/src/skip-link/SkipLink.css @@ -1,52 +1,44 @@ -/* 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); + 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); } -.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)); +@keyframes fade-in-out-outline { + 0% { + outline-color: var(--salt-focused-outlineColor); + } + 100% { + outline-color: transparent; + } } -/*Styles applied to the skip link focus target*/ .saltSkipLink-target { - outline: var(--skipLink-target-focus); + animation: fade-in-out-outline var(--salt-duration-notable) var(--salt-animation-timing-function) both; + outline: var(--salt-focused-outline); } diff --git a/packages/lab/src/skip-link/SkipLink.tsx b/packages/lab/src/skip-link/SkipLink.tsx index 3c8359c2ff5..cab72b388c1 100644 --- a/packages/lab/src/skip-link/SkipLink.tsx +++ b/packages/lab/src/skip-link/SkipLink.tsx @@ -1,28 +1,29 @@ -import { Link, type LinkProps, makePrefixer } from "@salt-ds/core"; +import { 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 { + type ComponentPropsWithoutRef, + forwardRef, + useEffect, + useRef, +} from "react"; import { useManageFocusOnTarget } from "./internal/useManageFocusOnTarget"; import skipLinkCss from "./SkipLink.css"; -interface SkipLinkProps extends LinkProps { +interface SkipLinkProps extends ComponentPropsWithoutRef<"a"> { /** - * This is a ref that has access to the target element. - * - * This will be used to apply focus to that element - * - * 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 + * The ID of the target element to apply focus when the link is clicked. + * If the element with this ID is not found, the SkipLink will not be rendered. */ - targetRef?: RefObject; + target: string; } const withBaseName = makePrefixer("saltSkipLink"); export const SkipLink = forwardRef( - function SkipLink({ className, targetRef, ...rest }, ref) { + function SkipLink({ className, target, children, ...rest }, ref) { const targetWindow = useWindow(); useComponentCssInjection({ testId: "salt-skip-link", @@ -30,19 +31,30 @@ export const SkipLink = forwardRef( window: targetWindow, }); - const targetClass = clsx(withBaseName("target"), className); + const targetRef = useRef(null); - const eventHandlers = useManageFocusOnTarget({ targetRef, targetClass }); + useEffect(() => { + targetRef.current = document.getElementById(target); + }, [target]); + + const eventHandlers = useManageFocusOnTarget({ + targetRef, + targetClass: withBaseName("target"), + }); return ( -
  • - -
    • + target="_self" + {...eventHandlers} + {...rest} + > + {children} + + ) ); }, ); diff --git a/packages/lab/src/skip-link/SkipLinks.css b/packages/lab/src/skip-link/SkipLinks.css deleted file mode 100644 index 763cbb7604a..00000000000 --- a/packages/lab/src/skip-link/SkipLinks.css +++ /dev/null @@ -1,7 +0,0 @@ -.saltSkipLinks { - position: relative; - float: left; - list-style: none; - margin: 0; - padding: 0; -} diff --git a/packages/lab/src/skip-link/SkipLinks.tsx b/packages/lab/src/skip-link/SkipLinks.tsx deleted file mode 100644 index 7a0b8e83804..00000000000 --- a/packages/lab/src/skip-link/SkipLinks.tsx +++ /dev/null @@ -1,29 +0,0 @@ -import { makePrefixer } from "@salt-ds/core"; -import { clsx } from "clsx"; -import { type HTMLAttributes, forwardRef } from "react"; - -import { useComponentCssInjection } from "@salt-ds/styles"; -import { useWindow } from "@salt-ds/window"; - -import skipLinksCss from "./SkipLinks.css"; - -const withBaseName = makePrefixer("saltSkipLinks"); - -export const SkipLinks = forwardRef< - HTMLUListElement, - HTMLAttributes ->(function SkipLinks(props, ref) { - const { className, children, ...restProps } = props; - const targetWindow = useWindow(); - useComponentCssInjection({ - testId: "salt-skip-links", - css: skipLinksCss, - window: targetWindow, - }); - - return ( -
      - {children} -
    - ); -}); diff --git a/packages/lab/src/skip-link/index.ts b/packages/lab/src/skip-link/index.ts index 1de755de460..cb4f10ea24e 100644 --- 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 index 1f264821f8d..48c43046be9 100644 --- a/packages/lab/stories/skip-link/skip-link.stories.tsx +++ b/packages/lab/stories/skip-link/skip-link.stories.tsx @@ -1,132 +1,149 @@ -import { Button } from "@salt-ds/core"; -import { SkipLink, SkipLinks } from "@salt-ds/lab"; +import { + BorderItem, + BorderLayout, + Button, + FlexItem, + FlexLayout, + NavigationItem, + SplitLayout, + StackLayout, +} from "@salt-ds/core"; +import { GithubIcon, StackoverflowIcon, SymphonyIcon } from "@salt-ds/icons"; +import { SkipLink } from "@salt-ds/lab"; import type { Meta, StoryFn } from "@storybook/react"; -import { useRef } from "react"; -import "./skip-link.stories.css"; +import { useEffect, useState } from "react"; export default { title: "Lab/Skip Link", component: SkipLink, } as Meta; -export const Default: StoryFn = () => { - const articleRef = useRef(null); - +const Item = () => { return ( - <> - - Click here and press the Tab key to see the Skip Link - -
    - - - Skip to main content - - - -
    - What we do -
    - -
    -
    -

    Salt

    -

    - 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. -

    -
    -
    -

    Goals

    -

    Salt has been developed with the following design goals:

    -
      -
    • - Providing a comprehensive set of commonly-used UI controls -
      • -
    • Complying with WCAG 2.1 accessibility guidelines
      • -
    • To be lightweight and performant
      • -
    • Offering flexible styling and theming support
      • -
    • Minimizing dependencies on third-party libraries
      • -
    -
    -
    -
    - -
    -
    - +
    ); }; -export const MultipleLinks: StoryFn = () => { - const sectionRef1 = useRef(null); - const sectionRef2 = useRef(null); +const DefaultStory: StoryFn = (args) => { + const headerItems = ["Home", "Transactions", "FX", "Credit Manager"]; - return ( - <> - - Click here and press the Tab key to see the Skip Link - -
    - - - Skip to Introduction - - - Skip to Goals - - + const headerUtilities = [ + { + icon: , + key: "Symphony", + }, + { + icon: , + key: "Stack Overflow", + }, + { + icon: , + key: "GitHub", + }, + ]; + + const [activeHeaderNav, setActiveHeaderNav] = useState(headerItems[0]); + const [offset, setOffset] = useState(0); -
    - What we do -
    + const setScroll = () => { + setOffset(window.scrollY); + }; -
    -
    -

    Salt

    -

    - 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. -

    -
    -
    -

    Goals

    -

    Salt has been developed with the following design goals:

    -
      -
    • - Providing a comprehensive set of commonly-used UI controls -
      • -
    • Complying with WCAG 2.1 accessibility guidelines
      • -
    • To be lightweight and performant
      • -
    • Offering flexible styling and theming support
      • -
    • Minimizing dependencies on third-party libraries
      • -
    -
    + useEffect(() => { + window.addEventListener("scroll", setScroll); + return () => { + window.removeEventListener("scroll", setScroll); + }; + }, []); + + return ( + + +
    + 0 ? "var(--salt-overlayable-shadow-scroll)" : "none", + borderBottom: + "var(--salt-size-border) var(--salt-container-borderStyle) var(--salt-separable-primary-borderColor)", + }} + justify="space-between" + gap={3} + > + + Click here and press the Tab key to see the Skip Link + + + + + {headerUtilities?.map((utility) => ( + + ))} + + + +
    +     + +
    + + +
    -
    - -
    -
    - + Next} /> + + ); }; + +export const Default = DefaultStory.bind({}); +Default.args = { + target: "main", +}; +Default.parameters = { + layout: "fullscreen", +}; From a157187c8aefaa7ce84afeb1f3beeb69566f9743 Mon Sep 17 00:00:00 2001 From: Fernanda Castillo Date: Wed, 18 Dec 2024 13:43:36 +0000 Subject: [PATCH 2/6] updates --- .changeset/gentle-hairs-repair.md | 4 +- packages/lab/src/skip-link/SkipLink.tsx | 26 ++++--- .../stories/skip-link/skip-link.stories.css | 26 +++++-- .../stories/skip-link/skip-link.stories.tsx | 74 ++++--------------- 4 files changed, 51 insertions(+), 79 deletions(-) diff --git a/.changeset/gentle-hairs-repair.md b/.changeset/gentle-hairs-repair.md index 67b06a5f3e8..3a2aa06d41b 100644 --- a/.changeset/gentle-hairs-repair.md +++ b/.changeset/gentle-hairs-repair.md @@ -1,9 +1,9 @@ --- -"@salt-ds/lab": patch +"@salt-ds/lab": minor --- Updates to Lab `SkipLink` -- Deprecated `targetRef` prop, added `target` prop to accept a string representing the ID of the target element. +- Remove `targetRef` prop, added `target` prop to accept a string representing the ID of the target element. - Updated styling to adhere with the rest of the library styles for consistency. - Fixed an issue where the `SkipLink` would render when the ref to the target element was broken. Now, the skip link will not render at all if the target element is not found. diff --git a/packages/lab/src/skip-link/SkipLink.tsx b/packages/lab/src/skip-link/SkipLink.tsx index cab72b388c1..c66325943e8 100644 --- a/packages/lab/src/skip-link/SkipLink.tsx +++ b/packages/lab/src/skip-link/SkipLink.tsx @@ -7,6 +7,7 @@ import { forwardRef, useEffect, useRef, + useState, } from "react"; import { useManageFocusOnTarget } from "./internal/useManageFocusOnTarget"; @@ -24,6 +25,7 @@ const withBaseName = makePrefixer("saltSkipLink"); export const SkipLink = forwardRef( function SkipLink({ className, target, children, ...rest }, ref) { + const [isTargetAvailable, setIsTargetAvailable] = useState(false); const targetWindow = useWindow(); useComponentCssInjection({ testId: "salt-skip-link", @@ -35,6 +37,7 @@ export const SkipLink = forwardRef( useEffect(() => { targetRef.current = document.getElementById(target); + setIsTargetAvailable(!!targetRef.current); }, [target]); const eventHandlers = useManageFocusOnTarget({ @@ -42,19 +45,18 @@ export const SkipLink = forwardRef( targetClass: withBaseName("target"), }); + if (!isTargetAvailable) return null; return ( - targetRef.current && ( - - {children} - - ) + + {children} + ); }, ); diff --git a/packages/lab/stories/skip-link/skip-link.stories.css b/packages/lab/stories/skip-link/skip-link.stories.css index 9283ff6592b..8240e515702 100644 --- a/packages/lab/stories/skip-link/skip-link.stories.css +++ b/packages/lab/stories/skip-link/skip-link.stories.css @@ -1,9 +1,21 @@ -.saltTargetBlankNoIcon-Link .saltLink-icon { - display: none; -} +.header { + padding-left: var(--salt-spacing-300); + padding-right: var(--salt-spacing-300); + background-color: var(--salt-container-primary-background); + position: fixed; + width: 100%; -.goalsList { - font-size: var(--salt-text-fontSize); - font-family: var(--salt-text-fontFamily); - line-height: var(--salt-text-lineHeight); + border-bottom: var(--salt-size-border) var(--salt-container-borderStyle) var(--salt-separable-primary-borderColor); +} +.navigation { + display: flex; + list-style: none; + padding: 0; + margin: 0; +} +.center { + margin: calc(var(--salt-spacing-300) * 2); +} +.article { + max-width: var(--max-content-width); } diff --git a/packages/lab/stories/skip-link/skip-link.stories.tsx b/packages/lab/stories/skip-link/skip-link.stories.tsx index 48c43046be9..be1d9bc5dbb 100644 --- a/packages/lab/stories/skip-link/skip-link.stories.tsx +++ b/packages/lab/stories/skip-link/skip-link.stories.tsx @@ -4,6 +4,7 @@ import { Button, FlexItem, FlexLayout, + H1, NavigationItem, SplitLayout, StackLayout, @@ -11,25 +12,14 @@ import { import { GithubIcon, StackoverflowIcon, SymphonyIcon } from "@salt-ds/icons"; import { SkipLink } from "@salt-ds/lab"; import type { Meta, StoryFn } from "@storybook/react"; -import { useEffect, useState } from "react"; +import { useState } from "react"; +import "./skip-link.stories.css"; export default { title: "Lab/Skip Link", component: SkipLink, } as Meta; -const Item = () => { - return ( -
    - ); -}; - const DefaultStory: StoryFn = (args) => { const headerItems = ["Home", "Transactions", "FX", "Credit Manager"]; @@ -49,51 +39,18 @@ const DefaultStory: StoryFn = (args) => { ]; const [activeHeaderNav, setActiveHeaderNav] = useState(headerItems[0]); - const [offset, setOffset] = useState(0); - - const setScroll = () => { - setOffset(window.scrollY); - }; - - useEffect(() => { - window.addEventListener("scroll", setScroll); - return () => { - window.removeEventListener("scroll", setScroll); - }; - }, []); return (
    - 0 ? "var(--salt-overlayable-shadow-scroll)" : "none", - borderBottom: - "var(--salt-size-border) var(--salt-container-borderStyle) var(--salt-separable-primary-borderColor)", - }} - justify="space-between" - gap={3} - > + Click here and press the Tab key to see the Skip Link
    - -
    - - - + +
    +

    Explore our offering

    +
    +

    + Lorem ipsum dolor sit amet, consectetur adipisicing elit. Aliquam, + consequuntur culpa dolor excepturi fugit in ipsa iusto laudantium + magnam minima necessitatibus odio qui quia repellendus sit tempore + veniam. At, veritatis. +

    +
    Next} />     From df8d221b050eacfa6c1d5dea58cce693758bd3c5 Mon Sep 17 00:00:00 2001 From: Fernanda Castillo Date: Thu, 19 Dec 2024 11:18:27 +0000 Subject: [PATCH 3/6] updates from dev feedback --- .../__e2e__/skip-link/SkipLink.cy.tsx | 26 +++- packages/lab/src/skip-link/SkipLink.tsx | 8 +- .../internal/useManageFocusOnTarget.ts | 131 ++++++++++++------ 3 files changed, 117 insertions(+), 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 index 0737efdbf77..5ff3e9e72f1 100644 --- 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(); 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(); + 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(); diff --git a/packages/lab/src/skip-link/SkipLink.tsx b/packages/lab/src/skip-link/SkipLink.tsx index c66325943e8..59552a14b0e 100644 --- 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( - 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( }, [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 index 8b6314d6992..e633987a874 100644 --- 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 | undefined; + targetClass: string; +} + +// Result interface +interface ManageFocusOnTargetResult { + onBlur?: FocusEventHandler; + onClick?: MouseEventHandler; + onKeyUp?: KeyboardEventHandler; +} export const useManageFocusOnTarget = ({ + onKeyUp, + onBlur, + onClick, targetRef, targetClass, -}: { - targetRef: RefObject | undefined; - targetClass: string; -}): - | { onBlur: () => NodeJS.Timeout; onClick: () => void } - | Record => { +}: ManageFocusOnTargetProps): ManageFocusOnTargetResult => { const [target, setTarget] = useState(); const hasTabIndex = useRef(); @@ -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 = (event) => { + if (event.key === "Enter" || event.key === " ") { + moveToTarget(); + } + onKeyUp?.(event); + }; + + const handleClick: MouseEventHandler = (event) => { + moveToTarget(); + onClick?.(event); + }; + + const handleBlur: FocusEventHandler = (event) => { + setTimeout(removeTabIndex, FOCUS_TIMEOUT); + onBlur?.(event); + }; + + return { + onBlur: handleBlur, + onClick: handleClick, + onKeyUp: handleKeyUp, + }; }; From 1bea029a9997a06053314297b89ff875b4417ee4 Mon Sep 17 00:00:00 2001 From: Fernanda Castillo Date: Thu, 19 Dec 2024 15:49:12 +0000 Subject: [PATCH 4/6] updates --- packages/lab/src/skip-link/SkipLink.css | 10 +- .../stories/skip-link/skip-link.stories.css | 13 +- .../stories/skip-link/skip-link.stories.tsx | 116 +++++++++++------- 3 files changed, 90 insertions(+), 49 deletions(-) diff --git a/packages/lab/src/skip-link/SkipLink.css b/packages/lab/src/skip-link/SkipLink.css index 57ac2f1cb29..00269ee1746 100644 --- a/packages/lab/src/skip-link/SkipLink.css +++ b/packages/lab/src/skip-link/SkipLink.css @@ -29,7 +29,8 @@ box-shadow: var(--salt-overlayable-shadow); } -@keyframes fade-in-out-outline { +@keyframes fade-out-back-outline { + /* required animation to apply an opacity fade-out-back to outline */ 0% { outline-color: var(--salt-focused-outlineColor); } @@ -39,6 +40,11 @@ } .saltSkipLink-target { - animation: fade-in-out-outline var(--salt-duration-notable) var(--salt-animation-timing-function) both; + animation: fade-out-back-outline var(--salt-duration-notable) var(--salt-animation-timing-function) both; outline: var(--salt-focused-outline); } +@media (prefers-reduced-motion: reduce) { + .saltSkipLink-target { + animation: none; + } +} diff --git a/packages/lab/stories/skip-link/skip-link.stories.css b/packages/lab/stories/skip-link/skip-link.stories.css index 8240e515702..d2e870a4502 100644 --- a/packages/lab/stories/skip-link/skip-link.stories.css +++ b/packages/lab/stories/skip-link/skip-link.stories.css @@ -1,10 +1,10 @@ +.app-header { + z-index: var(--salt-zIndex-appHeader); +} .header { padding-left: var(--salt-spacing-300); padding-right: var(--salt-spacing-300); background-color: var(--salt-container-primary-background); - position: fixed; - width: 100%; - border-bottom: var(--salt-size-border) var(--salt-container-borderStyle) var(--salt-separable-primary-borderColor); } .navigation { @@ -14,8 +14,9 @@ margin: 0; } .center { - margin: calc(var(--salt-spacing-300) * 2); + margin: calc(var(--salt-spacing-300) * 2) auto; + max-width: 700px; } -.article { - max-width: var(--max-content-width); +.content-header { + padding-top: calc(var(--salt-spacing-300) * 2); } diff --git a/packages/lab/stories/skip-link/skip-link.stories.tsx b/packages/lab/stories/skip-link/skip-link.stories.tsx index be1d9bc5dbb..9438fd56593 100644 --- a/packages/lab/stories/skip-link/skip-link.stories.tsx +++ b/packages/lab/stories/skip-link/skip-link.stories.tsx @@ -2,9 +2,12 @@ import { BorderItem, BorderLayout, Button, + Card, FlexItem, FlexLayout, + GridLayout, H1, + H2, NavigationItem, SplitLayout, StackLayout, @@ -37,62 +40,93 @@ const DefaultStory: StoryFn = (args) => { key: "GitHub", }, ]; + const cardHeaders = [ + "Economic trends", + "Machine learning", + "Financial conditions", + "Balance sheets", + "Outlook hub", + "Trading strategies", + "Emerging markets", + "Support tools", + "Index", + ]; const [activeHeaderNav, setActiveHeaderNav] = useState(headerItems[0]); return ( - -
    - - - Click here and press the Tab key to see the Skip Link - - - - - {headerUtilities?.map((utility) => ( - - ))} - - - -
    + + + + Click here and press the Tab key to see the Skip Link + + + + + {headerUtilities?.map((utility) => ( + + ))} + + + -
    -

    Explore our offering

    +
    +

    + Explore our offering +

    Lorem ipsum dolor sit amet, consectetur adipisicing elit. Aliquam, consequuntur culpa dolor excepturi fugit in ipsa iusto laudantium magnam minima necessitatibus odio qui quia repellendus sit tempore veniam. At, veritatis.

    + + {cardHeaders.map((title) => { + return ( + +

    {title}

    +

    + Lorem ipsum dolor sit amet, consectetur adipisicing elit. + Aliquam, consequuntur culpa dolor excepturi. +

    +

    + Fugit in ipsa iusto laudantium magnam minima + necessitatibus odio qui quia repellendus sit tempore + veniam. At, veritatis. +

    +     + ); + })} +
    -
    - Next} /> + See all themes} + /> +     ); From 394ac358a0de46ac5e301a463a9d992439b593e4 Mon Sep 17 00:00:00 2001 From: Fernanda Castillo Date: Thu, 19 Dec 2024 16:45:17 +0000 Subject: [PATCH 5/6] targetid --- .../src/__tests__/__e2e__/skip-link/SkipLink.cy.tsx | 2 +- packages/lab/src/skip-link/SkipLink.tsx | 10 +++++----- packages/lab/stories/skip-link/skip-link.stories.tsx | 2 +- 3 files changed, 7 insertions(+), 7 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 index 5ff3e9e72f1..f5ed90d0252 100644 --- a/packages/lab/src/__tests__/__e2e__/skip-link/SkipLink.cy.tsx +++ b/packages/lab/src/__tests__/__e2e__/skip-link/SkipLink.cy.tsx @@ -52,7 +52,7 @@ describe("GIVEN a SkipLink", () => { }); it("THEN it should hide the skip link if ref is broken", () => { - cy.mount(); + cy.mount(); cy.findByText( "Click here and press the Tab key to see the Skip Link", ).click(); diff --git a/packages/lab/src/skip-link/SkipLink.tsx b/packages/lab/src/skip-link/SkipLink.tsx index 59552a14b0e..cf3dd157052 100644 --- a/packages/lab/src/skip-link/SkipLink.tsx +++ b/packages/lab/src/skip-link/SkipLink.tsx @@ -18,14 +18,14 @@ interface SkipLinkProps extends ComponentPropsWithoutRef<"a"> { * The ID of the target element to apply focus when the link is clicked. * If the element with this ID is not found, the SkipLink will not be rendered. */ - target: string; + targetId: string; } const withBaseName = makePrefixer("saltSkipLink"); export const SkipLink = forwardRef( function SkipLink( - { className, target, children, onKeyUp, onBlur, onClick, ...rest }, + { className, targetId, children, onKeyUp, onBlur, onClick, ...rest }, ref, ) { const [isTargetAvailable, setIsTargetAvailable] = useState(false); @@ -39,9 +39,9 @@ export const SkipLink = forwardRef( const targetRef = useRef(null); useEffect(() => { - targetRef.current = document.getElementById(target); + targetRef.current = document.getElementById(targetId); setIsTargetAvailable(!!targetRef.current); - }, [target]); + }, [targetId]); const eventHandlers = useManageFocusOnTarget({ onKeyUp, @@ -55,7 +55,7 @@ export const SkipLink = forwardRef( return ( = (args) => { export const Default = DefaultStory.bind({}); Default.args = { - target: "main", + targetId: "main", }; Default.parameters = { layout: "fullscreen", From 27cfbcd0a6b20ec45ef1bf5f3375d395e28fba59 Mon Sep 17 00:00:00 2001 From: Fernanda Castillo Date: Tue, 24 Dec 2024 14:54:16 +0000 Subject: [PATCH 6/6] move skiplink up --- packages/lab/src/skip-link/SkipLink.css | 1 + packages/lab/stories/skip-link/skip-link.stories.tsx | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/lab/src/skip-link/SkipLink.css b/packages/lab/src/skip-link/SkipLink.css index 00269ee1746..1193fd89a4b 100644 --- a/packages/lab/src/skip-link/SkipLink.css +++ b/packages/lab/src/skip-link/SkipLink.css @@ -16,6 +16,7 @@ font-family: var(--salt-text-fontFamily); white-space: nowrap; background: var(--saltSkipLink-background, var(--salt-container-primary-background)); + z-index: calc(var(--salt-zIndex-appHeader) + 1); } /* Styles applied when the link is focused to display the Skip Link only when in focus*/ diff --git a/packages/lab/stories/skip-link/skip-link.stories.tsx b/packages/lab/stories/skip-link/skip-link.stories.tsx index eb67f8691c7..6c085126645 100644 --- a/packages/lab/stories/skip-link/skip-link.stories.tsx +++ b/packages/lab/stories/skip-link/skip-link.stories.tsx @@ -57,12 +57,12 @@ const DefaultStory: StoryFn = (args) => { return ( + Skip to main content Click here and press the Tab key to see the Skip Link