Skip to content

Commit 095b9cb

Browse files
committed
only set aria-describedby when tooltip visible
1 parent 3c7e909 commit 095b9cb

File tree

2 files changed

+22
-50
lines changed

2 files changed

+22
-50
lines changed

src/Tooltip.tsx

Lines changed: 10 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import type { ActionType, AlignType } from '@rc-component/trigger/lib/interface'
44
import useId from '@rc-component/util/lib/hooks/useId';
55
import { clsx } from 'clsx';
66
import * as React from 'react';
7-
import { useImperativeHandle, useRef, useEffect, useCallback } from 'react';
7+
import { useImperativeHandle, useRef, useState } from 'react';
88
import { placements } from './placements';
99
import Popup from './Popup';
1010

@@ -90,55 +90,22 @@ const Tooltip = React.forwardRef<TooltipRef, TooltipProps>((props, ref) => {
9090

9191
const extraProps: Partial<TooltipProps & TriggerProps> = { ...restProps };
9292

93-
if ('visible' in props) {
93+
const isControlled = 'visible' in props;
94+
95+
if (isControlled) {
9496
extraProps.popupVisible = props.visible;
9597
}
9698

97-
const isControlled = 'visible' in props;
98-
const mergedVisible = props.visible;
99-
const [popupMounted, setPopupMounted] = React.useState(() => {
100-
if (forceRender) {
101-
return true;
102-
}
103-
if (isControlled) {
104-
return mergedVisible;
105-
}
106-
return defaultVisible;
107-
});
108-
109-
const updatePopupMounted = useCallback(
110-
(nextVisible: boolean) => {
111-
setPopupMounted((prev) => {
112-
if (nextVisible) {
113-
return true;
114-
}
115-
116-
if (destroyOnHidden) {
117-
return false;
118-
}
119-
120-
return prev;
121-
});
122-
},
123-
[destroyOnHidden],
124-
);
99+
const [innerVisible, setInnerVisible] = useState(() => defaultVisible || false);
100+
const mergedVisible = isControlled ? props.visible : innerVisible;
125101

126102
const handleVisibleChange = (nextVisible: boolean) => {
127-
updatePopupMounted(nextVisible);
103+
if (!isControlled) {
104+
setInnerVisible(nextVisible);
105+
}
128106
onVisibleChange?.(nextVisible);
129107
};
130108

131-
useEffect(() => {
132-
if (forceRender) {
133-
setPopupMounted(true);
134-
return;
135-
}
136-
137-
if (isControlled) {
138-
updatePopupMounted(mergedVisible);
139-
}
140-
}, [forceRender, isControlled, mergedVisible, updatePopupMounted]);
141-
142109
// ========================= Arrow ==========================
143110
// Process arrow configuration
144111
const mergedArrow = React.useMemo(() => {
@@ -164,7 +131,7 @@ const Tooltip = React.forwardRef<TooltipRef, TooltipProps>((props, ref) => {
164131
const originalProps = child?.props || {};
165132
const childProps = {
166133
...originalProps,
167-
'aria-describedby': overlay && popupMounted ? mergedId : null,
134+
'aria-describedby': overlay && mergedVisible ? mergedId : null,
168135
};
169136
return React.cloneElement<any>(children, childProps) as any;
170137
};

tests/index.test.tsx

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -502,7 +502,7 @@ describe('rc-tooltip', () => {
502502
});
503503

504504
describe('children handling', () => {
505-
it('should only set aria-describedby once popup is mounted', async () => {
505+
it('should toggle aria-describedby with visibility', async () => {
506506
const { container } = render(
507507
<Tooltip trigger={['click']} overlay="tooltip content">
508508
<button>Click me</button>
@@ -519,7 +519,7 @@ describe('rc-tooltip', () => {
519519

520520
fireEvent.click(btn);
521521
await waitFakeTimers();
522-
expect(btn).toHaveAttribute('aria-describedby', describedby);
522+
expect(btn).not.toHaveAttribute('aria-describedby');
523523
});
524524

525525
it('should not pass aria-describedby when overlay is empty', () => {
@@ -542,17 +542,22 @@ describe('rc-tooltip', () => {
542542
expect(container.querySelector('button')).toHaveAttribute('aria-describedby');
543543
});
544544

545-
it('should set aria-describedby immediately when forceRender is true', () => {
545+
it('should only set aria-describedby for forceRender after visible', async () => {
546546
const { container } = render(
547-
<Tooltip forceRender overlay="tooltip content">
547+
<Tooltip forceRender trigger={['click']} overlay="tooltip content">
548548
<button>Click me</button>
549549
</Tooltip>,
550550
);
551551

552-
expect(container.querySelector('button')).toHaveAttribute('aria-describedby');
552+
const btn = container.querySelector('button');
553+
expect(btn).not.toHaveAttribute('aria-describedby');
554+
555+
fireEvent.click(btn);
556+
await waitFakeTimers();
557+
expect(btn).toHaveAttribute('aria-describedby');
553558
});
554559

555-
it('should keep aria-describedby when controlled hidden without destroy', () => {
560+
it('should remove aria-describedby when controlled hidden without destroy', () => {
556561
const overlay = 'tooltip content';
557562
const { container, rerender } = render(
558563
<Tooltip overlay={overlay} visible>
@@ -568,7 +573,7 @@ describe('rc-tooltip', () => {
568573
</Tooltip>,
569574
);
570575

571-
expect(container.querySelector('button')).toHaveAttribute('aria-describedby');
576+
expect(container.querySelector('button')).not.toHaveAttribute('aria-describedby');
572577
});
573578

574579
it('should remove aria-describedby when popup is destroyed on hide', async () => {

0 commit comments

Comments
 (0)