Skip to content

Commit

Permalink
Update render method for tooltips accessible description (#3806)
Browse files Browse the repository at this point in the history
* add test story to validate SR experience

* update tooltip renderChild method and somantic element validation

* update tooltip tests to check valid accessible properties

* increase test coverage for aria-describedby scenarios

* move animation duration tests into own test file

* remove now redundant test story

* add changeset

* split semantic role into util and add unit test
  • Loading branch information
mcwinter07 authored Jul 20, 2023
1 parent 52e3d1f commit c8cf582
Show file tree
Hide file tree
Showing 7 changed files with 261 additions and 32 deletions.
11 changes: 11 additions & 0 deletions .changeset/dull-pens-grow.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
"@kaizen/draft-tooltip": patch
---

Updates tooltip to pass accessible descriptions only into native or custom semantic components and only when in the DOM
- aria-describedby should not be attach to non semantic elements, ie: div and spans
- id for aria-describedby is undefined until the tooltip is in the dom
- This will stop the a11y error of pointing to an id of an element that does exists
- Increases test coverage to validate semantic elements of the tooltip with receive aria-describedby
- Moves tooltip animation tests to separate file so mock does not interfere with return value

99 changes: 71 additions & 28 deletions draft-packages/tooltip/KaizenDraft/Tooltip/Tooltip.spec.tsx
Original file line number Diff line number Diff line change
@@ -1,42 +1,85 @@
import React from "react"
import { render } from "@testing-library/react"
import * as AppearanceAnim from "./AppearanceAnim"
import { Tooltip } from "./index"
import "@testing-library/jest-dom"
jest.mock("./AppearanceAnim")
const AnimationProvider = AppearanceAnim.AnimationProvider as jest.Mock
import { render, screen, waitFor } from "@testing-library/react"
import { Button } from "@kaizen/button"

import { Tooltip } from "./index"
describe("<Tooltip />", () => {
beforeEach(() => {
AnimationProvider.mockReturnValue(<div />)
})
describe("When no animationDuration prop is given", () => {
it("animationDuration prop is passed to AnimationProvider as undefined", () => {
describe("Linking the tooltip to the inner element with aria-describedby", () => {
it("adds an accessible description when wrapping Kaizen Button", async () => {
render(
<Tooltip text="Example">
<div />
<Tooltip
text="Tooltip popup description for Kaizen Button"
display="inline"
isInitiallyVisible
position="below"
>
<Button label="More info" />
</Tooltip>
)
expect(AnimationProvider).toHaveBeenCalledWith(
expect.objectContaining({
animationDuration: undefined,
}),
{}

await waitFor(() => {
expect(
screen.getByRole("button", { name: "More info" })
).toHaveAccessibleDescription(
"Tooltip popup description for Kaizen Button"
)
})
})

it("doesn't add an accessible description if the tooltip is inactive", async () => {
render(
<Tooltip
text="Tooltip popup description for button"
display="inline"
position="below"
>
<Button label="More info" />
</Tooltip>
)

await waitFor(() => {
expect(screen.queryByRole("tooltip")).toBe(null)
expect(
screen.getByRole("button", { name: "More info" })
).not.toHaveAttribute("aria-describedby")
})
})
})
describe("When animationDuration prop is given", () => {
it("animationDuration prop is passed to AnimationProvider", () => {

// Non-semantic elements without roles should not have aria-description on them.
// They won't read to all screen readers as expected and may be reported in Storybook's accessibility tab (which uses Axe under the hood)
it("doesn't add an accessible description when wrapping a non-semantic element", async () => {
render(
<Tooltip text="Example" animationDuration={1000}>
<div />
<Tooltip
text="Tooltip popup description for div"
display="inline"
isInitiallyVisible
position="below"
>
<div>Non semantic element</div>
</Tooltip>
)
expect(AnimationProvider).toHaveBeenCalledWith(
expect.objectContaining({
animationDuration: 1000,
}),
{}
await waitFor(() => {
expect(screen.getByText("Non semantic element")).not.toHaveAttribute(
"aria-describedby"
)
})
})
})

it("adds an accessible description when wrapping a semantic element", async () => {
render(
<Tooltip
text="Tooltip popup description for div"
display="inline"
isInitiallyVisible
position="below"
>
<div role="textbox" contentEditable="true" aria-multiline="true"></div>
</Tooltip>
)
await waitFor(() => {
expect(screen.getByRole("textbox")).toHaveAccessibleDescription(
"Tooltip popup description for div"
)
})
})
Expand Down
36 changes: 32 additions & 4 deletions draft-packages/tooltip/KaizenDraft/Tooltip/Tooltip.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,17 @@
import React, { useEffect, useRef, useState } from "react"
import React, {
ReactNode,
cloneElement,
useEffect,
useRef,
useState,
} from "react"
import ReactDOM from "react-dom"
import { Placement } from "@popperjs/core"
import classnames from "classnames"
import { usePopper } from "react-popper"
import { AnimationProvider, useAnimation } from "./AppearanceAnim"
import { useUuid } from "./useUuid"
import { isSemanticElement } from "./utils/isSemanticElement"
import styles from "./Tooltip.module.scss"

type Position = "above" | "below" | "left" | "right"
Expand All @@ -30,6 +37,9 @@ export type TooltipProps = {
*/
position?: Position
text: React.ReactNode
/**
* This is the interactable element that is being described by the tooltip `text`
*/
children?: React.ReactNode
classNameOverride?: string
mood?: Mood
Expand Down Expand Up @@ -145,6 +155,24 @@ const TooltipContent = ({
) : null
}

const renderChildren = (
content: ReactNode,
tooltipId: string,
hasActiveTooltip: boolean
): ReactNode => {
if (isSemanticElement(content)) {
return cloneElement(content, {
"aria-describedby": hasActiveTooltip ? tooltipId : undefined,
})
}
// We don't want to block them from this but just provide context for better a11y guidance
// eslint-disable-next-line no-console
console.warn(
"<Tooltip /> is not directly wrapping a semantic element, screen reader users will not be able to access the tooltip info. To ensure accessibility, Tooltip should be wrapping a semantic and focusable element directly."
)
return content
}

/**
* {@link https://cultureamp.design/components/tooltip/ Guidance} |
* {@link https://cultureamp.design/storybook/?path=/docs/components-tooltip--default-kaizen-site-demo Storybook}
Expand All @@ -166,6 +194,7 @@ export const Tooltip = ({
const [referenceElement, setReferenceElement] =
useState<HTMLDivElement | null>(null)
const tooltipId = useUuid()
const hasActiveTooltip = isHover || isFocus

const tooltip = (
<TooltipContent
Expand Down Expand Up @@ -216,7 +245,7 @@ export const Tooltip = ({

return (
<AnimationProvider
isVisible={isHover || isFocus}
isVisible={hasActiveTooltip}
animationDuration={animationDuration}
>
<>
Expand All @@ -227,9 +256,8 @@ export const Tooltip = ({
onMouseLeave={(): void => setIsHover(false)}
onFocusCapture={(): void => setIsFocus(true)}
onBlurCapture={(): void => setIsFocus(false)}
aria-describedby={tooltipId}
>
{children}
{renderChildren(children, tooltipId, hasActiveTooltip)}
</div>

{portalSelector && portalSelectorElementRef.current
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
import React from "react"
import { render } from "@testing-library/react"
import * as AppearanceAnim from "./AppearanceAnim"
import { Tooltip } from "./index"
import "@testing-library/jest-dom"
jest.mock("./AppearanceAnim")
const AnimationProvider = AppearanceAnim.AnimationProvider as jest.Mock

describe("<Tooltip /> animationDuration", () => {
beforeEach(() => {
AnimationProvider.mockReturnValue(<div />)
})
describe("When no animationDuration prop is given", () => {
it("animationDuration prop is passed to AnimationProvider as undefined", () => {
render(
<Tooltip text="Example">
<div />
</Tooltip>
)
expect(AnimationProvider).toHaveBeenCalledWith(
expect.objectContaining({
animationDuration: undefined,
}),
{}
)
})
})
describe("When animationDuration prop is given", () => {
it("animationDuration prop is passed to AnimationProvider", () => {
render(
<Tooltip text="Example" animationDuration={1000}>
<div />
</Tooltip>
)
expect(AnimationProvider).toHaveBeenCalledWith(
expect.objectContaining({
animationDuration: 1000,
}),
{}
)
})
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
import React from "react"
import { Button, IconButton } from "@kaizen/button"
import { Icon } from "@kaizen/component-library"
import ArrowIcon from "@kaizen/component-library/icons/arrow-right.icon.svg"
import { isSemanticElement } from "./isSemanticElement"

describe("isSemanticElement", () => {
it("returns true if provided a native element with a semantic role", () => {
expect(
isSemanticElement(
<button onClick={jest.fn()} type="button">
click
</button>
)
).toBe(true)
expect(isSemanticElement(<a href="/">link</a>)).toBe(true)
})

it("returns false if provided a non-semantic element", () => {
expect(isSemanticElement(<span>click</span>)).toBe(false)
expect(isSemanticElement(<div>link</div>)).toBe(false)
})

it("returns true if provided one of the Kaizen components from the allowed list", () => {
expect(isSemanticElement(<Button label="Click" />)).toBe(true)
expect(isSemanticElement(<IconButton label="click" />)).toBe(true)
})

it("will return true if provided a non-semantic element with a semantic role", () => {
expect(
isSemanticElement(
<span
tabIndex={0}
role="button"
onKeyDown={jest.fn}
onClick={jest.fn()}
>
custom semantic el
</span>
)
).toBe(true)
expect(
isSemanticElement(
<div tabIndex={0} role="button" onKeyDown={jest.fn} onClick={jest.fn()}>
custom semantic el
</div>
)
).toBe(true)
expect(
isSemanticElement(
<div role="textbox" contentEditable="true" aria-multiline="true" />
)
).toBe(true)
})

it("returns false if provided an element using a role 'presentation' or 'none'", () => {
expect(
isSemanticElement(<Icon role="presentation" icon={ArrowIcon} />)
).toBe(false)
expect(
isSemanticElement(
<span role="none">
<Icon icon={ArrowIcon} />
</span>
)
).toBe(false)
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import React, { ReactNode } from "react"

// Noting that this is contingent on our displayName for our components - this something we control
const allowedDisplayNames = ["Button", "IconButton", "FilterButtonBase"]

export function extractDisplayName(type: React.FunctionComponent): string {
return type.displayName || type.name || "Unknown"
}

/**
* Validates implicit or explicitly semantic roles required to make `aria-describedby` announce predictably with screen readers
*/
export const isSemanticElement = (
element: ReactNode
): element is React.ReactElement => {
if (!React.isValidElement(element)) return false

const { props, type } = element

if ("role" in props) {
return props.role !== "presentation" && props.role !== "none"
}

if (typeof type !== "string") {
// As we are only checking whether this matches to our allowedDisplayNames
// type casting should be fine
const displayName = extractDisplayName(
type as unknown as React.FunctionComponent
)

return allowedDisplayNames.includes(displayName)
}

return !(type === "div" || type === "span")
}
1 change: 1 addition & 0 deletions packages/button/src/Button/components/GenericButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export type GenericProps = {
onBlur?: (e: FocusEvent<HTMLElement>) => void
component?: ComponentType<CustomButtonProps>
classNameOverride?: string
"aria-describedby"?: string
}

export type ButtonType = "submit" | "reset" | "button"
Expand Down

0 comments on commit c8cf582

Please sign in to comment.