From 566676e50ec61432951fb80671c2c1ba65928d1f Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Wed, 31 Jan 2024 12:44:12 +1000 Subject: [PATCH 1/3] Allow external id to be passed to label the trigger --- src/drafts/Tooltip/Tooltip.examples.stories.tsx | 7 +++++++ src/drafts/Tooltip/Tooltip.tsx | 8 ++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/src/drafts/Tooltip/Tooltip.examples.stories.tsx b/src/drafts/Tooltip/Tooltip.examples.stories.tsx index a54a549a1bb..04ba0cf3526 100644 --- a/src/drafts/Tooltip/Tooltip.examples.stories.tsx +++ b/src/drafts/Tooltip/Tooltip.examples.stories.tsx @@ -11,6 +11,7 @@ import { SmileyIcon, EyeIcon, CommentIcon, + XIcon, } from '@primer/octicons-react' import {default as VisuallyHidden} from '../../_VisuallyHidden' @@ -19,6 +20,12 @@ export default { component: Tooltip, } +export const CustomId = () => ( + + {}} /> + +) + export const FilesPage = () => ( diff --git a/src/drafts/Tooltip/Tooltip.tsx b/src/drafts/Tooltip/Tooltip.tsx index 949ea46c34b..efbdd9a4ec5 100644 --- a/src/drafts/Tooltip/Tooltip.tsx +++ b/src/drafts/Tooltip/Tooltip.tsx @@ -186,7 +186,7 @@ export const TooltipContext = React.createContext<{tooltipId?: string}>({}) export const Tooltip = React.forwardRef( ({direction = 's', text, type = 'description', children, ...rest}: TooltipProps, forwardedRef) => { - const tooltipId = useId() + const tooltipId = useId(rest.id) const child = Children.only(children) const triggerRef = useProvidedRefOrCreate(forwardedRef as React.RefObject) const tooltipElRef = useRef(null) @@ -273,9 +273,9 @@ export const Tooltip = React.forwardRef( React.cloneElement(child as React.ReactElement, { ref: triggerRef, // If it is a type description, we use tooltip to describe the trigger - 'aria-describedby': type === 'description' ? `tooltip-${tooltipId}` : child.props['aria-describedby'], + 'aria-describedby': type === 'description' ? tooltipId : child.props['aria-describedby'], // If it is a label type, we use tooltip to label the trigger - 'aria-labelledby': type === 'label' ? `tooltip-${tooltipId}` : child.props['aria-labelledby'], + 'aria-labelledby': type === 'label' ? tooltipId : child.props['aria-labelledby'], onBlur: (event: React.FocusEvent) => { closeTooltip() child.props.onBlur?.(event) @@ -301,7 +301,7 @@ export const Tooltip = React.forwardRef( role={type === 'description' ? 'tooltip' : undefined} // stop AT from announcing the tooltip twice when it is a label type because it will be announced with "aria-labelledby" aria-hidden={type === 'label' ? true : undefined} - id={`tooltip-${tooltipId}`} + id={tooltipId} // mouse leave and enter on the tooltip itself is needed to keep the tooltip open when the mouse is over the tooltip onMouseEnter={openTooltip} onMouseLeave={closeTooltip} From 0f9164975a2344ceaf953def682032a5b4a870f4 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Wed, 31 Jan 2024 12:46:40 +1000 Subject: [PATCH 2/3] add changeset --- .changeset/honest-kings-occur.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/honest-kings-occur.md diff --git a/.changeset/honest-kings-occur.md b/.changeset/honest-kings-occur.md new file mode 100644 index 00000000000..979fc736a5a --- /dev/null +++ b/.changeset/honest-kings-occur.md @@ -0,0 +1,5 @@ +--- +'@primer/react': patch +--- + +Tooltip v2: Allow external id to be passed down in tooltip so that the trigger can be labelled/described by the that id From c93cffe0a6b6f5a58c5429696f336eb927612e70 Mon Sep 17 00:00:00 2001 From: Armagan Ersoz Date: Fri, 2 Feb 2024 14:39:26 +1000 Subject: [PATCH 3/3] deconstruct id from props and add unit tests --- src/drafts/Tooltip/Tooltip.tsx | 4 ++-- src/drafts/Tooltip/__tests__/Tooltip.test.tsx | 21 ++++++++++++++++++- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/src/drafts/Tooltip/Tooltip.tsx b/src/drafts/Tooltip/Tooltip.tsx index efbdd9a4ec5..f5dbd95b561 100644 --- a/src/drafts/Tooltip/Tooltip.tsx +++ b/src/drafts/Tooltip/Tooltip.tsx @@ -185,8 +185,8 @@ const isInteractive = (element: HTMLElement) => { export const TooltipContext = React.createContext<{tooltipId?: string}>({}) export const Tooltip = React.forwardRef( - ({direction = 's', text, type = 'description', children, ...rest}: TooltipProps, forwardedRef) => { - const tooltipId = useId(rest.id) + ({direction = 's', text, type = 'description', children, id, ...rest}: TooltipProps, forwardedRef) => { + const tooltipId = useId(id) const child = Children.only(children) const triggerRef = useProvidedRefOrCreate(forwardedRef as React.RefObject) const tooltipElRef = useRef(null) diff --git a/src/drafts/Tooltip/__tests__/Tooltip.test.tsx b/src/drafts/Tooltip/__tests__/Tooltip.test.tsx index e8fbea212f3..c6ffa4ac8b5 100644 --- a/src/drafts/Tooltip/__tests__/Tooltip.test.tsx +++ b/src/drafts/Tooltip/__tests__/Tooltip.test.tsx @@ -3,7 +3,8 @@ import {Tooltip, TooltipProps} from '../Tooltip' import {checkStoriesForAxeViolations} from '../../../utils/testing' import {render as HTMLRender} from '@testing-library/react' import theme from '../../../theme' -import {Button, ActionMenu, ActionList, ThemeProvider, SSRProvider, BaseStyles} from '../../../' +import {Button, IconButton, ActionMenu, ActionList, ThemeProvider, SSRProvider, BaseStyles} from '../../../' +import {XIcon} from '@primer/octicons-react' const TooltipComponent = (props: Omit & {text?: string}) => ( @@ -91,4 +92,22 @@ describe('Tooltip', () => { expect(menuButton).toHaveAttribute('aria-describedby', tooltip.id) expect(menuButton).toHaveAttribute('aria-haspopup', 'true') }) + it('should use the custom tooltip id (if present) to label the trigger element', () => { + const {getByRole} = HTMLRender( + + {}} /> + , + ) + const triggerEL = getByRole('button') + expect(triggerEL).toHaveAttribute('aria-labelledby', 'custom-tooltip-id') + }) + it('should use the custom tooltip id (if present) to described the trigger element', () => { + const {getByRole} = HTMLRender( + + + , + ) + const triggerEL = getByRole('button') + expect(triggerEL).toHaveAttribute('aria-describedby', 'custom-tooltip-id') + }) })