Skip to content

Commit 041e7da

Browse files
authored
fix(ItemBase): tooltip logic and keyboard props (#809)
1 parent eb5a2ef commit 041e7da

File tree

4 files changed

+139
-18
lines changed

4 files changed

+139
-18
lines changed

.changeset/modern-dryers-give.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@cube-dev/ui-kit": patch
3+
---
4+
5+
Fix tooltip logic in ItemBase component.

.changeset/rude-ducks-share.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@cube-dev/ui-kit": patch
3+
---
4+
5+
Fix accessibility by setting keyboard props to hotkeys in ItemBase component.

src/components/content/ItemBase/ItemBase.stories.tsx

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import { IconCoin, IconSettings, IconUser } from '@tabler/icons-react';
2+
import { useState } from 'react';
23
import { expect, userEvent, waitFor, within } from 'storybook/test';
34

45
import { DirectionIcon } from '../../../icons';
56
import { baseProps } from '../../../stories/lists/baseProps';
7+
import { Button } from '../../actions';
68
import { Space } from '../../layout/Space';
79
import { Title } from '../Title';
810

@@ -1105,3 +1107,66 @@ WithAutoTooltip.parameters = {
11051107
},
11061108
},
11071109
};
1110+
1111+
export const DynamicAutoTooltip: StoryFn<CubeItemBaseProps> = () => {
1112+
const [width, setWidth] = useState('400px');
1113+
1114+
return (
1115+
<div>
1116+
<ItemBase
1117+
qa="dynamic-tooltip-item"
1118+
icon={<IconUser />}
1119+
style={{ width }}
1120+
tooltip={{ delay: 0 }}
1121+
>
1122+
This is a very long label that will eventually overflow
1123+
</ItemBase>
1124+
<Button
1125+
qa="resize-button"
1126+
onPress={() =>
1127+
width === '400px' ? setWidth('150px') : setWidth('400px')
1128+
}
1129+
>
1130+
Resize
1131+
</Button>
1132+
</div>
1133+
);
1134+
};
1135+
1136+
// DynamicAutoTooltip.play = async ({ canvasElement }) => {
1137+
// const canvas = within(canvasElement);
1138+
// await timeout(250);
1139+
1140+
// const item = await canvas.findByTestId('dynamic-tooltip-item');
1141+
1142+
// // Test 1: No tooltip when wide
1143+
// // this is a weird hack that makes tooltip working properly on page load
1144+
// await userEvent.unhover(item);
1145+
// await userEvent.hover(item);
1146+
// await timeout(1000);
1147+
// expect(canvas.queryByRole('tooltip')).toBe(null);
1148+
1149+
// // Change width to trigger overflow
1150+
// await userEvent.unhover(item);
1151+
// const resizeButton = await canvas.findByTestId('resize-button');
1152+
// await userEvent.click(resizeButton);
1153+
// // Unhover button after clicking to clear any hover state
1154+
// await userEvent.unhover(resizeButton);
1155+
// await timeout(1000);
1156+
1157+
// // Test 2: Tooltip appears when narrow
1158+
// // this is a weird hack that makes tooltip working properly
1159+
// await userEvent.unhover(item);
1160+
// await userEvent.hover(item);
1161+
1162+
// await waitFor(() => expect(canvas.getByRole('tooltip')).toBeVisible());
1163+
// };
1164+
1165+
DynamicAutoTooltip.parameters = {
1166+
docs: {
1167+
description: {
1168+
story:
1169+
'Tests the dynamic auto tooltip behavior that responds to width changes. Initially the item is wide and no tooltip appears. When the width is reduced, the label overflows and the tooltip automatically appears on hover.',
1170+
},
1171+
},
1172+
};

src/components/content/ItemBase/ItemBase.tsx

Lines changed: 64 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
useCallback,
88
useEffect,
99
useMemo,
10+
useRef,
1011
useState,
1112
} from 'react';
1213
import { OverlayProps } from 'react-aria';
@@ -125,6 +126,10 @@ export interface CubeItemBaseProps extends BaseProps, ContainerStyleProps {
125126
* @default "top"
126127
*/
127128
defaultTooltipPlacement?: OverlayProps['placement'];
129+
/**
130+
* Ref to access the label element directly
131+
*/
132+
labelRef?: RefObject<HTMLElement>;
128133
}
129134

130135
const DEFAULT_ICON_STYLES: Styles = {
@@ -387,6 +392,8 @@ export function useAutoTooltip({
387392
// Track label overflow for auto tooltip (only when enabled)
388393
const mergedLabelRef = useCombinedRefs((labelProps as any)?.ref);
389394
const [isLabelOverflowed, setIsLabelOverflowed] = useState(false);
395+
const observedElementRef = useRef<HTMLElement | null>(null);
396+
const resizeObserverRef = useRef<ResizeObserver | null>(null);
390397

391398
const checkLabelOverflow = useCallback(() => {
392399
const label = mergedLabelRef.current;
@@ -396,7 +403,6 @@ export function useAutoTooltip({
396403
}
397404

398405
const hasOverflow = label.scrollWidth > label.clientWidth;
399-
400406
setIsLabelOverflowed(hasOverflow);
401407
}, [mergedLabelRef]);
402408

@@ -406,24 +412,64 @@ export function useAutoTooltip({
406412
}
407413
}, [isAutoTooltipEnabled, checkLabelOverflow]);
408414

409-
useEffect(() => {
410-
if (!isAutoTooltipEnabled) return;
411-
412-
const label = mergedLabelRef.current;
413-
if (!label) return;
415+
// Attach ResizeObserver via callback ref to handle DOM node changes
416+
const handleLabelElementRef = useCallback(
417+
(element: HTMLElement | null) => {
418+
// Sync to combined ref so external refs receive the node
419+
(mergedLabelRef as any).current = element;
420+
421+
// Disconnect previous observer
422+
if (resizeObserverRef.current) {
423+
try {
424+
resizeObserverRef.current.disconnect();
425+
} catch {
426+
// do nothing
427+
}
428+
resizeObserverRef.current = null;
429+
}
414430

415-
const resizeObserver = new ResizeObserver(checkLabelOverflow);
416-
resizeObserver.observe(label);
431+
observedElementRef.current = element;
432+
433+
if (element && isAutoTooltipEnabled) {
434+
// Create a fresh observer to capture the latest callback
435+
const obs = new ResizeObserver(() => {
436+
checkLabelOverflow();
437+
});
438+
resizeObserverRef.current = obs;
439+
obs.observe(element);
440+
// Initial check
441+
checkLabelOverflow();
442+
} else {
443+
setIsLabelOverflowed(false);
444+
}
445+
},
446+
[mergedLabelRef, isAutoTooltipEnabled, checkLabelOverflow],
447+
);
417448

418-
return () => resizeObserver.disconnect();
419-
}, [isAutoTooltipEnabled, checkLabelOverflow, mergedLabelRef]);
449+
// Cleanup on unmount
450+
useEffect(() => {
451+
return () => {
452+
if (resizeObserverRef.current) {
453+
try {
454+
resizeObserverRef.current.disconnect();
455+
} catch {
456+
// do nothing
457+
}
458+
resizeObserverRef.current = null;
459+
}
460+
observedElementRef.current = null;
461+
};
462+
}, []);
420463

421464
const finalLabelProps = useMemo(() => {
422-
return {
465+
const props = {
423466
...(labelProps || {}),
424-
ref: mergedLabelRef,
425-
} as Props & { ref?: any };
426-
}, [labelProps, mergedLabelRef]);
467+
};
468+
469+
delete props.ref;
470+
471+
return props;
472+
}, [labelProps]);
427473

428474
const renderWithTooltip = useCallback(
429475
(
@@ -498,7 +544,7 @@ export function useAutoTooltip({
498544
);
499545

500546
return {
501-
labelRef: mergedLabelRef,
547+
labelRef: handleLabelElementRef,
502548
labelProps: finalLabelProps,
503549
isLabelOverflowed,
504550
isAutoTooltipEnabled,
@@ -574,6 +620,7 @@ const ItemBase = <T extends HTMLElement = HTMLDivElement>(
574620
suffix ??
575621
(hotkeys ? (
576622
<HotKeys
623+
{...(keyboardShortcutProps as any)}
577624
type={type === 'primary' ? 'primary' : 'default'}
578625
styles={{ padding: '1x left', opacity: finalIsDisabled ? 0.5 : 1 }}
579626
>
@@ -633,7 +680,7 @@ const ItemBase = <T extends HTMLElement = HTMLDivElement>(
633680

634681
const {
635682
labelProps: finalLabelProps,
636-
hasTooltip,
683+
labelRef,
637684
renderWithTooltip,
638685
} = useAutoTooltip({ tooltip, children, labelProps });
639686

@@ -660,7 +707,6 @@ const ItemBase = <T extends HTMLElement = HTMLDivElement>(
660707
return (
661708
<ItemBaseElement
662709
ref={handleRef}
663-
tabIndex={0}
664710
variant={
665711
theme && type ? (`${theme}.${type}` as ItemVariant) : undefined
666712
}
@@ -682,7 +728,7 @@ const ItemBase = <T extends HTMLElement = HTMLDivElement>(
682728
)}
683729
{finalPrefix && <div data-element="Prefix">{finalPrefix}</div>}
684730
{children || labelProps ? (
685-
<div data-element="Label" {...finalLabelProps}>
731+
<div data-element="Label" {...finalLabelProps} ref={labelRef}>
686732
{children}
687733
</div>
688734
) : null}

0 commit comments

Comments
 (0)