Skip to content

Commit 4d85fec

Browse files
committed
fix(core): reveal a half-visible overflow menu trigger, not just tabs
The strip's focus reveal looked for [data-tab-value], so the TabMenu trigger -- the strip's other roving stop -- was left out. Chrome scrolls a focused element into view only when it is entirely outside the scrollport, so arrowing onto a trigger that straddles the edge left it cut off with nothing to finish the scroll: measured in Chromium at a 200px strip, the trigger stayed 21px outside the box with its focus ring running off the edge.
1 parent afe68ae commit 4d85fec

2 files changed

Lines changed: 40 additions & 11 deletions

File tree

packages/core/src/TabList/TabList.test.tsx

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1153,6 +1153,35 @@ describe('TabList overflow (scroll)', () => {
11531153
expect(scrollBy.mock.calls[0][0].left).toBeCloseTo(40);
11541154
});
11551155

1156+
it('reveals a half-visible overflow menu trigger too, not just tabs', () => {
1157+
const {container} = render(
1158+
<TabList value="a" onChange={() => {}}>
1159+
<Tab value="a" label="Alpha" />
1160+
<Tab value="b" label="Beta" />
1161+
<TabMenu label="More" options={[{value: 'c', label: 'Gamma'}]} />
1162+
</TabList>,
1163+
);
1164+
const strip = container.querySelector<HTMLElement>(STRIP);
1165+
if (!strip) {
1166+
throw new Error('no tab strip');
1167+
}
1168+
const scrollBy = vi.fn();
1169+
strip.scrollBy = scrollBy;
1170+
strip.getBoundingClientRect = () =>
1171+
({left: 0, right: 300, width: 300}) as DOMRect;
1172+
const trigger = strip.querySelector<HTMLElement>('[data-tab-menu]');
1173+
if (!trigger) {
1174+
throw new Error('no menu trigger');
1175+
}
1176+
trigger.getBoundingClientRect = () =>
1177+
({left: 260, right: 340, width: 80}) as DOMRect;
1178+
1179+
fireEvent.focus(trigger, {bubbles: true});
1180+
1181+
expect(scrollBy).toHaveBeenCalledTimes(1);
1182+
expect(scrollBy.mock.calls[0][0].left).toBeCloseTo(40);
1183+
});
1184+
11561185
it('does not hand focus to an arrow, which is hidden from assistive tech', () => {
11571186
const {container, strip} = renderStrip();
11581187
fakeScrollBox(strip, {scrollWidth: 600, clientWidth: 300});

packages/core/src/TabList/TabList.tsx

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -358,14 +358,14 @@ export function TabList({
358358
[hasScroll, scrollRef],
359359
);
360360

361-
const revealTab = useCallback(
362-
(tab: HTMLElement | null) => {
361+
const revealStop = useCallback(
362+
(stop: HTMLElement | null) => {
363363
const strip = stripRef.current;
364-
if (!hasScroll || !strip || !tab) {
364+
if (!hasScroll || !strip || !stop) {
365365
return;
366366
}
367367
const stripBox = strip.getBoundingClientRect();
368-
const tabBox = tab.getBoundingClientRect();
368+
const tabBox = stop.getBoundingClientRect();
369369
const inset = parseFloat(getComputedStyle(strip).scrollPaddingLeft) || 0;
370370
const pastEnd = tabBox.right - (stripBox.right - inset);
371371
const pastStart = tabBox.left - (stripBox.left + inset);
@@ -384,12 +384,12 @@ export function TabList({
384384
if (!strip) {
385385
return;
386386
}
387-
revealTab(
387+
revealStop(
388388
Array.from(strip.querySelectorAll<HTMLElement>('[data-tab-value]')).find(
389389
el => el.dataset.tabValue === value,
390390
) ?? null,
391391
);
392-
}, [revealTab, value]);
392+
}, [revealStop, value]);
393393

394394
// The tab you are on has to be visible. Selection can move without focus —
395395
// on mount, or when the host sets `value` itself — and neither scrolls the
@@ -460,12 +460,12 @@ export function TabList({
460460
}
461461
onHintFocus(e);
462462
handleFocus(e);
463-
// The browser scrolls a focused tab into view only when it is entirely
464-
// outside the scrollport, so arrowing onto a half-visible tab leaves it
465-
// cut off under the fade. Finish the job it started.
466-
revealTab(e.target.closest('[data-tab-value]'));
463+
// The browser scrolls a focused element into view only when it is
464+
// entirely outside the scrollport, so arrowing onto a half-visible stop
465+
// leaves it cut off under the fade. Finish the job it started.
466+
revealStop(e.target.closest(TAB_STOP_SELECTOR));
467467
},
468-
[onFocusProp, onHintFocus, handleFocus, revealTab],
468+
[onFocusProp, onHintFocus, handleFocus, revealStop],
469469
);
470470

471471
const handleRootBlur = useCallback(

0 commit comments

Comments
 (0)