Skip to content

Commit

Permalink
fix(tooltip-popover-combo): fix aria labelledby and id issue
Browse files Browse the repository at this point in the history
  • Loading branch information
jor-row committed Jun 12, 2024
1 parent ca5f778 commit d47e571
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 37 deletions.
8 changes: 6 additions & 2 deletions src/components/Toggletip/Toggletip.unit.test.tsx.snap
Original file line number Diff line number Diff line change
Expand Up @@ -719,7 +719,9 @@ exports[`<Toggletip /> snapshot should match snapshot with color 2`] = `

exports[`<Toggletip /> snapshot should match snapshot with id 1`] = `
<div>
<button>
<button
id="example-id"
>
Click Me!
</button>
<div
Expand All @@ -730,7 +732,9 @@ exports[`<Toggletip /> snapshot should match snapshot with id 1`] = `

exports[`<Toggletip /> snapshot should match snapshot with id 2`] = `
<div>
<button>
<button
id="example-id"
>
Click Me!
</button>
<div
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ const Example: Story<TooltipPopoverComboProps> = () => {
margin: '10rem auto',
display: 'flex'
}}
id="storybook-id-example"
>
<Icon name='cancel' weight='bold' strokeColor="transparent" />
</ButtonCircle>);
Expand Down
24 changes: 15 additions & 9 deletions src/components/TooltipPopoverCombo/TooltipPopoverCombo.tsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import React, { FC, useCallback, useEffect, useState } from 'react';
import React, { FC, useCallback, useEffect, useRef, useState } from 'react';

import { Props } from './TooltipPopoverCombo.types';
import Popover from '../Popover';
import { TooltipPopoverComboProps } from '.';
import Tooltip from '../Tooltip/Tooltip';
import { PopoverInstance } from '../Popover/Popover.types';
import { v4 as uuidV4 } from 'uuid';

/**
* The TooltipPopoverCombo component.
Expand All @@ -21,6 +22,10 @@ const TooltipPopoverCombo: FC<Props> = (props: TooltipPopoverComboProps) => {
const [isPopoverOpen, setIsPopoverOpen] = useState(false);
const [isTooltipOpen, setIsTooltipOpen] = useState(false);

const triggerComponentId = useRef(triggerComponent.props?.id || uuidV4());

const clonedTriggerComponent = React.cloneElement(triggerComponent, {id: triggerComponentId.current});

// Modified tooltipSetInstance to call both setInstance and updateInstance
const setMergedTooltipInstances = useCallback(
(popoverInstance: PopoverInstance | undefined) => {
Expand Down Expand Up @@ -64,14 +69,15 @@ const TooltipPopoverCombo: FC<Props> = (props: TooltipPopoverComboProps) => {
interactive
triggerComponent={
<Tooltip
triggerComponent={triggerComponent}
{...otherTooltipProps}
onHide={handleOnHideTooltip}
onShow={handleOnShowTooltip}
setInstance={setMergedTooltipInstances}
>
{tooltipContent}
</Tooltip>
triggerComponent={clonedTriggerComponent}
{...otherTooltipProps}
onHide={handleOnHideTooltip}
onShow={handleOnShowTooltip}
setInstance={setMergedTooltipInstances}
id={triggerComponentId.current}
>
{tooltipContent}
</Tooltip>
}
{...otherPopoverProps}
onHide={handleOnHidePopover}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ export interface Props {
/**
* An object of Popover props to be passed through to the Popover component.
*/
otherPopoverProps?: Partial<PopoverProps>;
otherPopoverProps?: Partial<Omit<PopoverProps, 'id'>>;

/**
* An object of Tooltip props to be passed through to the Tooltip component.
*/
otherTooltipProps?: Partial<TooltipProps>;
otherTooltipProps?: Partial<Omit<TooltipProps, 'id'>>;
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jest.mock('uuid', () => {
});

describe('<TooltipPopoverCombo />', () => {
const triggerComponent = <button>Example button</button>;
const triggerComponent = <button id="test-id">Example button</button>;
const tooltipContent = <Text>Example tooltip content</Text>;
const popoverContent = <button>Example popover content button</button>;

Expand Down Expand Up @@ -76,7 +76,7 @@ describe('<TooltipPopoverCombo />', () => {
setInstance: expect.any(Function),
triggerComponent: triggerComponent,
'aria-haspopup': 'dialog',
'id': '1',
'id': 'test-id',
});
});

Expand Down Expand Up @@ -114,7 +114,7 @@ describe('<TooltipPopoverCombo />', () => {
setInstance: expect.any(Function),
triggerComponent: triggerComponent,
'aria-haspopup': 'dialog',
'id': '1',
'id': 'test-id',
'placement': 'top',
});
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@ exports[`<TooltipPopoverCombo /> snapshot should match snapshot 1`] = `
</Text>
}
triggerComponent={
<button>
<button
id="test-id"
>
Example button
</button>
}
Expand All @@ -25,11 +27,14 @@ exports[`<TooltipPopoverCombo /> snapshot should match snapshot 1`] = `
trigger="click"
triggerComponent={
<Tooltip
id="test-id"
onHide={[Function]}
onShow={[Function]}
setInstance={[Function]}
triggerComponent={
<button>
<button
id="test-id"
>
Example button
</button>
}
Expand Down Expand Up @@ -217,12 +222,14 @@ exports[`<TooltipPopoverCombo /> snapshot should match snapshot 1`] = `
>
<Tooltip
aria-haspopup="dialog"
id="1"
id="test-id"
onHide={[Function]}
onShow={[Function]}
setInstance={[Function]}
triggerComponent={
<button>
<button
id="test-id"
>
Example button
</button>
}
Expand All @@ -232,7 +239,7 @@ exports[`<TooltipPopoverCombo /> snapshot should match snapshot 1`] = `
aria-haspopup="dialog"
boundary="scrollParent"
color="primary"
id="1"
id="test-id"
interactive={false}
offsetDistance={5}
offsetSkidding={0}
Expand All @@ -245,7 +252,9 @@ exports[`<TooltipPopoverCombo /> snapshot should match snapshot 1`] = `
strategy="absolute"
trigger="mouseenter focus"
triggerComponent={
<button>
<button
id="test-id"
>
Example button
</button>
}
Expand Down Expand Up @@ -415,7 +424,7 @@ exports[`<TooltipPopoverCombo /> snapshot should match snapshot 1`] = `
trigger="mouseenter focus"
>
<button
id="1"
id="test-id"
>
Example button
</button>
Expand Down Expand Up @@ -471,7 +480,9 @@ exports[`<TooltipPopoverCombo /> snapshot should match snapshot with otherPopove
</Text>
}
triggerComponent={
<button>
<button
id="test-id"
>
Example button
</button>
}
Expand All @@ -483,12 +494,15 @@ exports[`<TooltipPopoverCombo /> snapshot should match snapshot with otherPopove
trigger="click"
triggerComponent={
<Tooltip
id="test-id"
onHide={[Function]}
onShow={[Function]}
placement="bottom"
setInstance={[Function]}
triggerComponent={
<button>
<button
id="test-id"
>
Example button
</button>
}
Expand Down Expand Up @@ -676,13 +690,15 @@ exports[`<TooltipPopoverCombo /> snapshot should match snapshot with otherPopove
>
<Tooltip
aria-haspopup="dialog"
id="1"
id="test-id"
onHide={[Function]}
onShow={[Function]}
placement="bottom"
setInstance={[Function]}
triggerComponent={
<button>
<button
id="test-id"
>
Example button
</button>
}
Expand All @@ -692,7 +708,7 @@ exports[`<TooltipPopoverCombo /> snapshot should match snapshot with otherPopove
aria-haspopup="dialog"
boundary="scrollParent"
color="primary"
id="1"
id="test-id"
interactive={false}
offsetDistance={5}
offsetSkidding={0}
Expand All @@ -705,7 +721,9 @@ exports[`<TooltipPopoverCombo /> snapshot should match snapshot with otherPopove
strategy="absolute"
trigger="mouseenter focus"
triggerComponent={
<button>
<button
id="test-id"
>
Example button
</button>
}
Expand Down Expand Up @@ -875,7 +893,7 @@ exports[`<TooltipPopoverCombo /> snapshot should match snapshot with otherPopove
trigger="mouseenter focus"
>
<button
id="1"
id="test-id"
>
Example button
</button>
Expand Down Expand Up @@ -931,7 +949,9 @@ exports[`<TooltipPopoverCombo /> snapshot should match snapshot with otherToolti
</Text>
}
triggerComponent={
<button>
<button
id="test-id"
>
Example button
</button>
}
Expand All @@ -943,12 +963,15 @@ exports[`<TooltipPopoverCombo /> snapshot should match snapshot with otherToolti
trigger="click"
triggerComponent={
<Tooltip
id="test-id"
onHide={[Function]}
onShow={[Function]}
placement="top"
setInstance={[Function]}
triggerComponent={
<button>
<button
id="test-id"
>
Example button
</button>
}
Expand Down Expand Up @@ -1136,13 +1159,15 @@ exports[`<TooltipPopoverCombo /> snapshot should match snapshot with otherToolti
>
<Tooltip
aria-haspopup="dialog"
id="1"
id="test-id"
onHide={[Function]}
onShow={[Function]}
placement="top"
setInstance={[Function]}
triggerComponent={
<button>
<button
id="test-id"
>
Example button
</button>
}
Expand All @@ -1152,7 +1177,7 @@ exports[`<TooltipPopoverCombo /> snapshot should match snapshot with otherToolti
aria-haspopup="dialog"
boundary="scrollParent"
color="primary"
id="1"
id="test-id"
interactive={false}
offsetDistance={5}
offsetSkidding={0}
Expand All @@ -1165,7 +1190,9 @@ exports[`<TooltipPopoverCombo /> snapshot should match snapshot with otherToolti
strategy="absolute"
trigger="mouseenter focus"
triggerComponent={
<button>
<button
id="test-id"
>
Example button
</button>
}
Expand Down Expand Up @@ -1335,7 +1362,7 @@ exports[`<TooltipPopoverCombo /> snapshot should match snapshot with otherToolti
trigger="mouseenter focus"
>
<button
id="1"
id="test-id"
>
Example button
</button>
Expand Down

0 comments on commit d47e571

Please sign in to comment.