From 1ac6a1608a092a71f0e306dc1e31477474ea34a7 Mon Sep 17 00:00:00 2001 From: dreamwasp Date: Wed, 26 Jun 2024 09:59:49 -0400 Subject: [PATCH 1/8] feat(IconButton): aria-describedby override --- packages/gamut/src/Button/IconButton.tsx | 5 ++++- packages/gamut/src/Tip/ToolTip/index.tsx | 10 ++++++++-- .../styleguide/stories/Atoms/Button/index.stories.mdx | 2 +- 3 files changed, 13 insertions(+), 4 deletions(-) diff --git a/packages/gamut/src/Button/IconButton.tsx b/packages/gamut/src/Button/IconButton.tsx index 4041930b11..7609a360f3 100644 --- a/packages/gamut/src/Button/IconButton.tsx +++ b/packages/gamut/src/Button/IconButton.tsx @@ -20,7 +20,10 @@ export type IconButtonProps = ComponentProps & IconComponentType & { 'aria-label'?: string; tip: string; - tipProps?: Omit; + tipProps?: Omit< + ToolTipProps, + 'info' | 'id' | 'children' | 'hasRepetitiveLabel' + >; }; export const IconButton = forwardRef( diff --git a/packages/gamut/src/Tip/ToolTip/index.tsx b/packages/gamut/src/Tip/ToolTip/index.tsx index 6bf899cc0d..5cc422e4eb 100644 --- a/packages/gamut/src/Tip/ToolTip/index.tsx +++ b/packages/gamut/src/Tip/ToolTip/index.tsx @@ -21,6 +21,10 @@ export type ToolTipProps = TipBaseProps & * If your button has a label that is repeated in the first word of the tooltip, you can set this to `true` to avoid repetition. If your info tip is not a string, you cannot do this. */ hasRepetitiveLabel?: boolean; + /** + * If you would like to forgo the aria-describedby attribute set this to `false`. When using this props, the `aria-label` should always be identical to the `tip`. + */ + hideAriaToolTip?: boolean; }; export const ToolTip: React.FC = ({ @@ -30,6 +34,7 @@ export const ToolTip: React.FC = ({ placement = tipDefaultProps.placement, id, hasRepetitiveLabel, + hideAriaToolTip, ...rest }) => { const wrapperRef = useRef(null); @@ -41,15 +46,16 @@ export const ToolTip: React.FC = ({ const isFloating = placement === 'floating'; const Tip = loaded && isFloating ? FloatingTip : InlineTip; + const adjustedInfo = useMemo(() => { return hasRepetitiveLabel && typeof info === 'string' ? info.split(' ').slice(1).join(' ') : info; }, [info, hasRepetitiveLabel]); - // this should only happen if the button has an aria-label that is the same is and ONLY the content of the tooltip + const shouldRenderAriaTip = hideAriaToolTip ? false : adjustedInfo !== ''; - const shouldRenderAriaTip = adjustedInfo !== ''; + // this should only happen if the button has an aria-label that is the same is and ONLY the content of the tooltip const tipProps = { alignment, diff --git a/packages/styleguide/stories/Atoms/Button/index.stories.mdx b/packages/styleguide/stories/Atoms/Button/index.stories.mdx index 345c8a2ee8..5abf6e7e41 100644 --- a/packages/styleguide/stories/Atoms/Button/index.stories.mdx +++ b/packages/styleguide/stories/Atoms/Button/index.stories.mdx @@ -132,7 +132,7 @@ Writing words for a link or button? Above all else, make sure that the words alo ## Icon Button -You must pass a string for the `tip` prop to label your icon button. The `tipProps` prop is optional and can be used to customize the tooltip. +You must pass a string for the `tip` prop to label your icon button. The `tipProps` prop is optional and can be used to customize the tooltip - for more information on `tooltip`s + `IconButton`s see the [ToolTip](../Tooltip/index.stories.mdx) component. From 64bf51b5d39a454fb4700bb5a9f99f069b68f87a Mon Sep 17 00:00:00 2001 From: dreamwasp Date: Wed, 26 Jun 2024 10:16:12 -0400 Subject: [PATCH 2/8] test --- packages/gamut/src/Tip/ToolTip/index.tsx | 8 ++++++-- .../gamut/src/Tip/__tests__/ToolTip.test.tsx | 20 +++++++++++++++++++ packages/gamut/src/Tip/__tests__/mocks.tsx | 2 ++ .../stories/Molecules/Tip/ToolTip.stories.mdx | 12 ++++++++++- 4 files changed, 39 insertions(+), 3 deletions(-) diff --git a/packages/gamut/src/Tip/ToolTip/index.tsx b/packages/gamut/src/Tip/ToolTip/index.tsx index 5cc422e4eb..92b9d86371 100644 --- a/packages/gamut/src/Tip/ToolTip/index.tsx +++ b/packages/gamut/src/Tip/ToolTip/index.tsx @@ -22,7 +22,7 @@ export type ToolTipProps = TipBaseProps & */ hasRepetitiveLabel?: boolean; /** - * If you would like to forgo the aria-describedby attribute set this to `false`. When using this props, the `aria-label` should always be identical to the `tip`. + * If you would like to forgo the aria-describedby attribute set this to `true`. When using this props, the `aria-label` should always be identical to the `tip`. */ hideAriaToolTip?: boolean; }; @@ -53,7 +53,11 @@ export const ToolTip: React.FC = ({ : info; }, [info, hasRepetitiveLabel]); - const shouldRenderAriaTip = hideAriaToolTip ? false : adjustedInfo !== ''; + const shouldRenderAriaTip = useMemo(() => { + return hideAriaToolTip ? false : adjustedInfo !== ''; + }, [hideAriaToolTip, adjustedInfo]); + + // const shouldRenderAriaTip = hideAriaToolTip ? false : adjustedInfo !== ''; // this should only happen if the button has an aria-label that is the same is and ONLY the content of the tooltip diff --git a/packages/gamut/src/Tip/__tests__/ToolTip.test.tsx b/packages/gamut/src/Tip/__tests__/ToolTip.test.tsx index 2b42cca4b4..0c23691819 100644 --- a/packages/gamut/src/Tip/__tests__/ToolTip.test.tsx +++ b/packages/gamut/src/Tip/__tests__/ToolTip.test.tsx @@ -41,6 +41,26 @@ describe('ToolTip', () => { view.getByRole('button', { name: 'Click' }); expect(view.queryByRole('tooltip')).toBeNull(); }); + + it('calls onClick when clicked', () => { + const { view } = renderView({}); + + userEvent.click(view.getByRole('button')); + + expect(onClick).toHaveBeenCalled(); + }); + + it('hides ariaTooltip when there is hideAriaToolTip is true', () => { + const { view } = renderView({ + 'aria-label': ariaLabel, + hideAriaToolTip: true, + info: `${ariaLabel}`, + }); + + view.getByRole('button', { name: 'Click' }); + expect(view.queryByRole('tooltip')).toBeNull(); + }); + it('calls onClick when clicked', () => { const { view } = renderView({}); diff --git a/packages/gamut/src/Tip/__tests__/mocks.tsx b/packages/gamut/src/Tip/__tests__/mocks.tsx index 65aa758ce4..b016595935 100644 --- a/packages/gamut/src/Tip/__tests__/mocks.tsx +++ b/packages/gamut/src/Tip/__tests__/mocks.tsx @@ -11,6 +11,7 @@ export const ToolTipMock: React.FC< placement, onClick, hasRepetitiveLabel, + hideAriaToolTip, }) => { return ( {() => ( - + + )} From 5c341097ced3fdd74f33ca3f51b85a21793b3ab2 Mon Sep 17 00:00:00 2001 From: dreamwasp Date: Wed, 26 Jun 2024 10:53:44 -0400 Subject: [PATCH 3/8] VO not being read on re-mount --- packages/gamut/src/Tip/InfoTip/index.tsx | 15 ++++++++++++--- packages/gamut/src/Tip/ToolTip/index.tsx | 8 +++----- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/packages/gamut/src/Tip/InfoTip/index.tsx b/packages/gamut/src/Tip/InfoTip/index.tsx index 3d52d32092..e1c562730f 100644 --- a/packages/gamut/src/Tip/InfoTip/index.tsx +++ b/packages/gamut/src/Tip/InfoTip/index.tsx @@ -1,4 +1,4 @@ -import { useEffect, useRef, useState } from 'react'; +import { useEffect, useMemo, useRef, useState } from 'react'; import { Text } from '../../Typography'; import { FloatingTip } from '../shared/FloatingTip'; @@ -30,6 +30,7 @@ export const InfoTip: React.FC = ({ const [isTipHidden, setHideTip] = useState(true); const wrapperRef = useRef(null); const [loaded, setLoaded] = useState(false); + const [clickNum, setClickNum] = useState(0); useEffect(() => { setLoaded(true); @@ -57,6 +58,7 @@ export const InfoTip: React.FC = ({ const clickHandler = () => { const currentTipState = !isTipHidden; setHideTip(currentTipState); + if (currentTipState) setClickNum(clickNum + 1); // we want to call the onClick handler after the tip has mounted if (onClick) setTimeout(() => onClick({ isTipHidden: currentTipState }), 0); }; @@ -68,12 +70,19 @@ export const InfoTip: React.FC = ({ }; }); + const screenReaderText = useMemo(() => { + // Voiceover won't read duplicated on subsequent mounts - this is a workaround to make Voiceover think "new" text is being added every mount. + const text = clickNum % 2 === 0 ? info : `${info}\u00A0`; + + return !isTipHidden ? text : ''; + }, [clickNum, info, isTipHidden]); + const Tip = loaded && placement === 'floating' ? FloatingTip : InlineTip; const tipProps = { alignment, escapeKeyPressHandler, - info, + info: screenReaderText, isTipHidden, wrapperRef, ...rest, @@ -82,7 +91,7 @@ export const InfoTip: React.FC = ({ return ( <> - {!isTipHidden ? info : ''} + {screenReaderText} = ({ : info; }, [info, hasRepetitiveLabel]); + // this should only happen if the button has an aria-label that is the same is and ONLY the content of the tooltip + const shouldRenderAriaTip = useMemo(() => { return hideAriaToolTip ? false : adjustedInfo !== ''; }, [hideAriaToolTip, adjustedInfo]); - // const shouldRenderAriaTip = hideAriaToolTip ? false : adjustedInfo !== ''; - - // this should only happen if the button has an aria-label that is the same is and ONLY the content of the tooltip - const tipProps = { alignment, info, From d3f275adea08d65feca75b070944efebc99dbc61 Mon Sep 17 00:00:00 2001 From: dreamwasp Date: Wed, 26 Jun 2024 13:52:06 -0400 Subject: [PATCH 4/8] fix test + docs --- packages/gamut/src/Tip/InfoTip/index.tsx | 2 +- packages/styleguide/stories/Molecules/Tip/InfoTip.stories.mdx | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/gamut/src/Tip/InfoTip/index.tsx b/packages/gamut/src/Tip/InfoTip/index.tsx index e1c562730f..9f9c7d9bdb 100644 --- a/packages/gamut/src/Tip/InfoTip/index.tsx +++ b/packages/gamut/src/Tip/InfoTip/index.tsx @@ -82,7 +82,7 @@ export const InfoTip: React.FC = ({ const tipProps = { alignment, escapeKeyPressHandler, - info: screenReaderText, + info, isTipHidden, wrapperRef, ...rest, diff --git a/packages/styleguide/stories/Molecules/Tip/InfoTip.stories.mdx b/packages/styleguide/stories/Molecules/Tip/InfoTip.stories.mdx index df37e39d4f..66d306137c 100644 --- a/packages/styleguide/stories/Molecules/Tip/InfoTip.stories.mdx +++ b/packages/styleguide/stories/Molecules/Tip/InfoTip.stories.mdx @@ -114,7 +114,7 @@ Links or buttons within InfoTips should be used sparingly and only when the info ## InfoTips and zIndex -Links or buttons within InfoTips should be used sparingly and only when the information is critical to the user's understanding of the content. If an infotip _absolutely requires_ a link or button, it needs to provide a programmatic focus by way of the `onClick` prop. The `onClick` prop accepts a function that calls the object `{isTipHidden}` and should focus when the tip is visible. +You can change the zIndex of your `InfoTip` with the zIndex property. @@ -132,3 +132,5 @@ Links or buttons within InfoTips should be used sparingly and only when the info )} + + From 7080b9322a39ba1b8c1c2202b045376f89d72a4e Mon Sep 17 00:00:00 2001 From: dreamwasp Date: Wed, 26 Jun 2024 14:31:24 -0400 Subject: [PATCH 5/8] remove infotip fix --- packages/gamut/src/Tip/InfoTip/index.tsx | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/packages/gamut/src/Tip/InfoTip/index.tsx b/packages/gamut/src/Tip/InfoTip/index.tsx index 9f9c7d9bdb..3d52d32092 100644 --- a/packages/gamut/src/Tip/InfoTip/index.tsx +++ b/packages/gamut/src/Tip/InfoTip/index.tsx @@ -1,4 +1,4 @@ -import { useEffect, useMemo, useRef, useState } from 'react'; +import { useEffect, useRef, useState } from 'react'; import { Text } from '../../Typography'; import { FloatingTip } from '../shared/FloatingTip'; @@ -30,7 +30,6 @@ export const InfoTip: React.FC = ({ const [isTipHidden, setHideTip] = useState(true); const wrapperRef = useRef(null); const [loaded, setLoaded] = useState(false); - const [clickNum, setClickNum] = useState(0); useEffect(() => { setLoaded(true); @@ -58,7 +57,6 @@ export const InfoTip: React.FC = ({ const clickHandler = () => { const currentTipState = !isTipHidden; setHideTip(currentTipState); - if (currentTipState) setClickNum(clickNum + 1); // we want to call the onClick handler after the tip has mounted if (onClick) setTimeout(() => onClick({ isTipHidden: currentTipState }), 0); }; @@ -70,13 +68,6 @@ export const InfoTip: React.FC = ({ }; }); - const screenReaderText = useMemo(() => { - // Voiceover won't read duplicated on subsequent mounts - this is a workaround to make Voiceover think "new" text is being added every mount. - const text = clickNum % 2 === 0 ? info : `${info}\u00A0`; - - return !isTipHidden ? text : ''; - }, [clickNum, info, isTipHidden]); - const Tip = loaded && placement === 'floating' ? FloatingTip : InlineTip; const tipProps = { @@ -91,7 +82,7 @@ export const InfoTip: React.FC = ({ return ( <> - {screenReaderText} + {!isTipHidden ? info : ''} Date: Wed, 26 Jun 2024 14:45:53 -0400 Subject: [PATCH 6/8] correct linkto + remove unneeded tests --- packages/gamut/src/Tip/__tests__/ToolTip.test.tsx | 8 -------- .../styleguide/stories/Atoms/Button/index.stories.mdx | 2 +- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/packages/gamut/src/Tip/__tests__/ToolTip.test.tsx b/packages/gamut/src/Tip/__tests__/ToolTip.test.tsx index 0c23691819..aa9cba8dc5 100644 --- a/packages/gamut/src/Tip/__tests__/ToolTip.test.tsx +++ b/packages/gamut/src/Tip/__tests__/ToolTip.test.tsx @@ -61,14 +61,6 @@ describe('ToolTip', () => { expect(view.queryByRole('tooltip')).toBeNull(); }); - it('calls onClick when clicked', () => { - const { view } = renderView({}); - - userEvent.click(view.getByRole('button')); - - expect(onClick).toHaveBeenCalled(); - }); - }); }); describe('floating placement', () => { it('has an accessible tooltip', () => { diff --git a/packages/styleguide/stories/Atoms/Button/index.stories.mdx b/packages/styleguide/stories/Atoms/Button/index.stories.mdx index 5abf6e7e41..e031a67399 100644 --- a/packages/styleguide/stories/Atoms/Button/index.stories.mdx +++ b/packages/styleguide/stories/Atoms/Button/index.stories.mdx @@ -132,7 +132,7 @@ Writing words for a link or button? Above all else, make sure that the words alo ## Icon Button -You must pass a string for the `tip` prop to label your icon button. The `tipProps` prop is optional and can be used to customize the tooltip - for more information on `tooltip`s + `IconButton`s see the [ToolTip](../Tooltip/index.stories.mdx) component. +You must pass a string for the `tip` prop to label your icon button. The `tipProps` prop is optional and can be used to customize the tooltip - for more information on `tooltip`s + `IconButton`s see the ToolTip page. From 1c939dd30f81dbb4971ab51f40e93fb548cb4b3b Mon Sep 17 00:00:00 2001 From: dreamwasp Date: Wed, 26 Jun 2024 15:00:35 -0400 Subject: [PATCH 7/8] fix test formatting --- packages/gamut/src/Tip/__tests__/ToolTip.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/gamut/src/Tip/__tests__/ToolTip.test.tsx b/packages/gamut/src/Tip/__tests__/ToolTip.test.tsx index aa9cba8dc5..8793b242e0 100644 --- a/packages/gamut/src/Tip/__tests__/ToolTip.test.tsx +++ b/packages/gamut/src/Tip/__tests__/ToolTip.test.tsx @@ -60,7 +60,7 @@ describe('ToolTip', () => { view.getByRole('button', { name: 'Click' }); expect(view.queryByRole('tooltip')).toBeNull(); }); - + }); }); describe('floating placement', () => { it('has an accessible tooltip', () => { From e87098a7e1a4198293e38235282f1b82ba70fac6 Mon Sep 17 00:00:00 2001 From: dreamwasp Date: Wed, 26 Jun 2024 15:03:50 -0400 Subject: [PATCH 8/8] run format --- packages/gamut/src/Button/__tests__/IconButton.test.tsx | 1 - packages/styleguide/stories/Molecules/Tip/InfoTip.stories.mdx | 2 -- packages/styleguide/stories/Molecules/Tip/ToolTip.stories.mdx | 4 ++-- 3 files changed, 2 insertions(+), 5 deletions(-) diff --git a/packages/gamut/src/Button/__tests__/IconButton.test.tsx b/packages/gamut/src/Button/__tests__/IconButton.test.tsx index 55feca50d2..dfde346e29 100644 --- a/packages/gamut/src/Button/__tests__/IconButton.test.tsx +++ b/packages/gamut/src/Button/__tests__/IconButton.test.tsx @@ -5,7 +5,6 @@ import { setupRtl } from 'component-test-setup'; import { IconButton } from '../IconButton'; import { IconButtonFloatingMock } from './mocks'; - const label = 'Click'; const tip = 'Click this button'; const tipText = 'this button'; diff --git a/packages/styleguide/stories/Molecules/Tip/InfoTip.stories.mdx b/packages/styleguide/stories/Molecules/Tip/InfoTip.stories.mdx index 66d306137c..6d269155a0 100644 --- a/packages/styleguide/stories/Molecules/Tip/InfoTip.stories.mdx +++ b/packages/styleguide/stories/Molecules/Tip/InfoTip.stories.mdx @@ -132,5 +132,3 @@ You can change the zIndex of your `InfoTip` with the zIndex property. )} - - diff --git a/packages/styleguide/stories/Molecules/Tip/ToolTip.stories.mdx b/packages/styleguide/stories/Molecules/Tip/ToolTip.stories.mdx index aab0906456..e10ee854c6 100644 --- a/packages/styleguide/stories/Molecules/Tip/ToolTip.stories.mdx +++ b/packages/styleguide/stories/Molecules/Tip/ToolTip.stories.mdx @@ -68,13 +68,13 @@ If your `IconButton` doesn't require `aria-describedby` text, you can set the `h {() => ( - + -