From 8d16c96fd5462a3b313cb2cdc173890d7b087c36 Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Mon, 14 Sep 2026 17:53:14 +0000 Subject: [PATCH 1/4] fix: point pinned scrollbar aria-controls at the container's real id The thumb's aria-controls was hardcoded to this mount's useId value, but the sync effect only stamps that id onto the scroll container when the container has none. When the host app supplies its own id, or when the component remounts onto a container a previous mount already labelled, aria-controls names an element that does not exist and assistive tech cannot resolve the scrollbar's target. Track the id actually present on the element instead. --- src/__tests__/pinning.test.tsx | 73 ++++++++++++++++++++++++++++++ src/components/PinnedScrollbar.tsx | 10 +++- 2 files changed, 82 insertions(+), 1 deletion(-) diff --git a/src/__tests__/pinning.test.tsx b/src/__tests__/pinning.test.tsx index 0acd2324..1f85ee1e 100644 --- a/src/__tests__/pinning.test.tsx +++ b/src/__tests__/pinning.test.tsx @@ -195,4 +195,77 @@ describe('PinnedScrollbar', () => { expect(ref.current!.scrollLeft).toBeGreaterThanOrEqual(0); } }); + + // Control: the container has no id of its own, so the effect stamps it with + // this mount's useId and aria-controls matches either way. Green before and + // after the fix — it pins the untouched offset-0 case so the fix can't be + // read as changing the ordinary first-mount path. + test('aria-controls resolves to the scroll container it labelled (control)', async () => { + const ref = makeScrollRef(1000, 400); + document.body.appendChild(ref.current!); + const { container, unmount } = renderWithTheme(); + + await act(async () => { + ref.current!.dispatchEvent(new Event('scroll')); + }); + + const thumb = container.querySelector('.rdt_pinnedScrollbarThumb') as HTMLElement; + const controls = thumb.getAttribute('aria-controls'); + expect(controls).toBe(ref.current!.id); + expect(document.getElementById(controls!)).toBe(ref.current); + + unmount(); + ref.current!.remove(); + }); + + // A dangling aria-controls is invisible in a rendered-DOM assertion — it only + // shows up when you resolve the id, which is what assistive tech does. + test('aria-controls points at a host-supplied container id instead of a fresh one', async () => { + const ref = makeScrollRef(1000, 400); + ref.current!.id = 'host-app-scroll-container'; + document.body.appendChild(ref.current!); + const { container, unmount } = renderWithTheme(); + + await act(async () => { + ref.current!.dispatchEvent(new Event('scroll')); + }); + + const thumb = container.querySelector('.rdt_pinnedScrollbarThumb') as HTMLElement; + // The container already had an id, so the effect leaves it alone — the thumb + // must follow it rather than pointing at its own unused useId value. + expect(ref.current!.id).toBe('host-app-scroll-container'); + expect(thumb.getAttribute('aria-controls')).toBe('host-app-scroll-container'); + expect(document.getElementById(thumb.getAttribute('aria-controls')!)).toBe(ref.current); + + unmount(); + ref.current!.remove(); + }); + + test('aria-controls survives a remount onto the same container', async () => { + const ref = makeScrollRef(1000, 400); + document.body.appendChild(ref.current!); + + // First mount stamps the container with its useId value. + const first = renderWithTheme(); + await act(async () => { + ref.current!.dispatchEvent(new Event('scroll')); + }); + const stampedId = ref.current!.id; + first.unmount(); + + // Unmounting doesn't clear the id, and the remount gets a *different* useId. + // Unpinning then re-pinning a column does exactly this in DataTable. + const second = renderWithTheme(); + await act(async () => { + ref.current!.dispatchEvent(new Event('scroll')); + }); + + const thumb = second.container.querySelector('.rdt_pinnedScrollbarThumb') as HTMLElement; + expect(ref.current!.id).toBe(stampedId); + expect(thumb.getAttribute('aria-controls')).toBe(stampedId); + expect(document.getElementById(thumb.getAttribute('aria-controls')!)).toBe(ref.current); + + second.unmount(); + ref.current!.remove(); + }); }); diff --git a/src/components/PinnedScrollbar.tsx b/src/components/PinnedScrollbar.tsx index 6f61b393..b1d89c87 100644 --- a/src/components/PinnedScrollbar.tsx +++ b/src/components/PinnedScrollbar.tsx @@ -13,6 +13,9 @@ export default function PinnedScrollbar({ rightInset, }: PinnedScrollbarProps): JSX.Element | null { const scrollContainerId = React.useId(); + // The id the thumb points at. Only equals scrollContainerId when this + // component is the one that labelled the container — see the sync effect. + const [controlsId, setControlsId] = React.useState(scrollContainerId); const trackRef = React.useRef(null); const thumbRef = React.useRef(null); const [thumbWidth, setThumbWidth] = React.useState(0); @@ -62,6 +65,11 @@ export default function PinnedScrollbar({ if (!el.id) { el.id = scrollContainerId; } + // The container keeps whatever id it already had — a host-supplied one, or + // one left behind by a previous mount (useId hands out a fresh value each + // time). Point aria-controls at the id that is actually on the element, or + // it dangles and assistive tech can't resolve the scrollbar's target. + setControlsId(el.id); el.addEventListener('scroll', update, { passive: true }); const ro = new ResizeObserver(update); ro.observe(el); @@ -191,7 +199,7 @@ export default function PinnedScrollbar({ ref={thumbRef} role="scrollbar" tabIndex={0} - aria-controls={scrollContainerId} + aria-controls={controlsId} aria-orientation="horizontal" aria-valuenow={scrollPercent} aria-valuemin={0} From fcd3a19f0a3384aa2f0410f06c30b353d28628f1 Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Mon, 14 Sep 2026 18:55:52 +0000 Subject: [PATCH 2/4] test: cover pinned scrollbar aria-controls across an unpin/re-pin cycle The unit test drives the remount directly against PinnedScrollbar. Exercise the same path through DataTable's public API, where unpinning every column drops hasPinnedColumns and re-pinning mounts a fresh scrollbar over the wrapper the previous mount already labelled. --- src/__tests__/pinning.test.tsx | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/src/__tests__/pinning.test.tsx b/src/__tests__/pinning.test.tsx index 1f85ee1e..a988b49f 100644 --- a/src/__tests__/pinning.test.tsx +++ b/src/__tests__/pinning.test.tsx @@ -101,6 +101,32 @@ describe('DataTable column pinning', () => { expect(offsets).toContain(0); expect(offsets.some(v => v > 0)).toBe(true); }); + + test('pinned scrollbar aria-controls still resolves after a column is unpinned and re-pinned', async () => { + const unpinnedCols: TableColumn[] = columns.map(c => ({ ...c, pinned: undefined })); + const { container, rerender } = render(); + + const wrapper = container.querySelector('.rdt_responsiveWrapper') as HTMLElement; + Object.defineProperty(wrapper, 'scrollWidth', { configurable: true, get: () => 1000 }); + Object.defineProperty(wrapper, 'clientWidth', { configurable: true, get: () => 400 }); + await act(async () => { + wrapper.dispatchEvent(new Event('scroll')); + }); + + // Unpinning every column drops hasPinnedColumns, which unmounts the + // scrollbar; re-pinning mounts a fresh one with a new useId, while the + // wrapper keeps the id the first mount stamped on it. + rerender(); + rerender(); + await act(async () => { + wrapper.dispatchEvent(new Event('scroll')); + }); + + const thumb = container.querySelector('.rdt_pinnedScrollbarThumb') as HTMLElement; + const controls = thumb.getAttribute('aria-controls'); + expect(controls).toBe(wrapper.id); + expect(document.getElementById(controls!)).toBe(wrapper); + }); }); // ── PinnedScrollbar ────────────────────────────────────────────────────────── From 7a9d65b453dd3bcfafde92b1b1fc18938b318b80 Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Mon, 14 Sep 2026 20:26:32 +0000 Subject: [PATCH 3/4] test: cover pinned scrollbar aria-controls container changes --- src/__tests__/pinning.test.tsx | 90 ++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/src/__tests__/pinning.test.tsx b/src/__tests__/pinning.test.tsx index a988b49f..290891d6 100644 --- a/src/__tests__/pinning.test.tsx +++ b/src/__tests__/pinning.test.tsx @@ -294,4 +294,94 @@ describe('PinnedScrollbar', () => { second.unmount(); ref.current!.remove(); }); + + // The effect keys on the *ref object*, so pointing the component at a + // different container mid-life re-runs it. The container swapped in already + // has an id, so nothing is stamped and aria-controls has to follow it. + test('aria-controls re-points when the scroll container is swapped for another one', async () => { + const first = makeScrollRef(1000, 400).current!; + const second = makeScrollRef(1000, 400).current!; + second.id = 'second-scroll-container'; + document.body.append(first, second); + + let swap!: (el: HTMLDivElement) => void; + function SwapHarness({ initial }: { initial: HTMLDivElement }): JSX.Element { + const [target, setTarget] = React.useState(initial); + swap = setTarget; + const ref = React.useMemo(() => ({ current: target }) as React.RefObject, [target]); + return ; + } + + const { container, unmount } = renderWithTheme(); + await act(async () => { + first.dispatchEvent(new Event('scroll')); + }); + await act(async () => { + swap(second); + }); + + const thumb = container.querySelector('.rdt_pinnedScrollbarThumb') as HTMLElement; + const controls = thumb.getAttribute('aria-controls'); + expect(controls).toBe('second-scroll-container'); + expect(document.getElementById(controls!)).toBe(second); + + unmount(); + first.remove(); + second.remove(); + }); + + // Reverse of the host-supplied-id case: the stamp happens first and the host + // takes the id over afterwards, so the remount sees an id it did not write. + test('aria-controls follows an id the host assigns after the first mount stamped one', async () => { + const ref = makeScrollRef(1000, 400); + document.body.appendChild(ref.current!); + + const first = renderWithTheme(); + await act(async () => { + ref.current!.dispatchEvent(new Event('scroll')); + }); + first.unmount(); + + ref.current!.id = 'host-took-over-later'; + const second = renderWithTheme(); + await act(async () => { + ref.current!.dispatchEvent(new Event('scroll')); + }); + + const thumb = second.container.querySelector('.rdt_pinnedScrollbarThumb') as HTMLElement; + expect(thumb.getAttribute('aria-controls')).toBe('host-took-over-later'); + expect(document.getElementById(thumb.getAttribute('aria-controls')!)).toBe(ref.current); + + second.unmount(); + ref.current!.remove(); + }); + + // Control: two scrollbars on the page at once, each on its own container. + // Green before and after — the id each thumb points at is per-instance state, + // and this pins that the two never share one. + test('two concurrent scrollbars each control their own container (control)', async () => { + const a = makeScrollRef(1000, 400); + const b = makeScrollRef(1000, 400); + document.body.append(a.current!, b.current!); + + const firstRender = renderWithTheme(); + const secondRender = renderWithTheme(); + await act(async () => { + a.current!.dispatchEvent(new Event('scroll')); + b.current!.dispatchEvent(new Event('scroll')); + }); + + const thumbA = firstRender.container.querySelector('.rdt_pinnedScrollbarThumb') as HTMLElement; + const thumbB = secondRender.container.querySelector('.rdt_pinnedScrollbarThumb') as HTMLElement; + const controlsA = thumbA.getAttribute('aria-controls'); + const controlsB = thumbB.getAttribute('aria-controls'); + expect(controlsA).not.toBe(controlsB); + expect(document.getElementById(controlsA!)).toBe(a.current); + expect(document.getElementById(controlsB!)).toBe(b.current); + + firstRender.unmount(); + secondRender.unmount(); + a.current!.remove(); + b.current!.remove(); + }); }); From 6bad58bc393938c7e3b9adf49541b00a8d96f8ad Mon Sep 17 00:00:00 2001 From: askalf <263217947+askalf@users.noreply.github.com> Date: Mon, 14 Sep 2026 21:34:57 +0000 Subject: [PATCH 4/4] test: pin that an empty scroll ref emits no aria-controls at all The sync effect returns at `if (!el)` before it can stamp an id or set controlsId, so no thumb is rendered and no control reference is emitted. Green on both source arms - it pins the early-return row of the boundary ledger, which had no coverage of its own. --- src/__tests__/pinning.test.tsx | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/__tests__/pinning.test.tsx b/src/__tests__/pinning.test.tsx index 290891d6..79c53b85 100644 --- a/src/__tests__/pinning.test.tsx +++ b/src/__tests__/pinning.test.tsx @@ -384,4 +384,22 @@ describe('PinnedScrollbar', () => { a.current!.remove(); b.current!.remove(); }); + + // Control: the effect bails at `if (!el) return` before it can stamp an id or + // call setControlsId, and `visible` never flips, so no thumb — and no + // aria-controls — is emitted at all. Green before and after the fix; it pins + // that the new state doesn't leak an attribute onto a scrollbar that the + // early return means was never rendered. + test('emits no thumb and no aria-controls when the scroll ref is empty (control)', async () => { + const ref = { current: null } as React.RefObject; + const { container, unmount } = renderWithTheme(); + + await act(async () => {}); + + expect(container.querySelector('.rdt_pinnedScrollbarTrack')).toBeNull(); + expect(container.querySelector('.rdt_pinnedScrollbarThumb')).toBeNull(); + expect(container.querySelector('[aria-controls]')).toBeNull(); + + unmount(); + }); });