Skip to content

Commit 1d6a1ca

Browse files
committed
Ensure Select onScroll is bound to the Popover, not the listbox
1 parent edf6957 commit 1d6a1ca

File tree

6 files changed

+47
-5
lines changed

6 files changed

+47
-5
lines changed

src/components/feedback/Popover.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import classnames from 'classnames';
2-
import type { ComponentChildren, RefObject } from 'preact';
2+
import type { ComponentChildren, JSX, RefObject } from 'preact';
33
import { useCallback, useEffect, useLayoutEffect, useRef } from 'preact/hooks';
44

55
import { useClickAway } from '../../hooks/use-click-away';
@@ -256,6 +256,9 @@ export type PopoverProps = {
256256
* Defaults to true, as long as the browser supports it.
257257
*/
258258
asNativePopover?: boolean;
259+
260+
/** A callback passed to the outermost element onScroll */
261+
onScroll?: JSX.HTMLAttributes<HTMLElement>['onScroll'];
259262
};
260263

261264
export default function Popover({
@@ -267,6 +270,7 @@ export default function Popover({
267270
classes,
268271
variant = 'panel',
269272
restoreFocusOnClose = true,
273+
onScroll,
270274
/* eslint-disable-next-line no-prototype-builtins */
271275
asNativePopover = HTMLElement.prototype.hasOwnProperty('popover'),
272276
}: PopoverProps) {
@@ -320,6 +324,7 @@ export default function Popover({
320324
)}
321325
ref={popoverRef}
322326
popover={asNativePopover && 'auto'}
327+
onScroll={onScroll}
323328
data-testid="popover"
324329
data-component="Popover"
325330
>

src/components/feedback/test/Popover-test.js

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -87,6 +87,15 @@ describe('Popover', () => {
8787
);
8888
};
8989

90+
it('invokes onScroll when scrolling inside the popover', () => {
91+
const onScroll = sinon.stub();
92+
const wrapper = createComponent({ onScroll });
93+
94+
getPopover(wrapper).find('[data-testid="popover"]').simulate('scroll');
95+
96+
assert.called(onScroll);
97+
});
98+
9099
[
91100
{
92101
restoreFocusOnClose: undefined, // Defaults to true

src/components/input/Select.tsx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -305,9 +305,12 @@ type BaseSelectProps = CompositeProps & {
305305
*/
306306
listboxAsPopover?: boolean;
307307

308-
/** A callback passed to the listbox onScroll */
308+
/** @deprecated. Use onPopoverScroll instead */
309309
onListboxScroll?: JSX.HTMLAttributes<HTMLUListElement>['onScroll'];
310310

311+
/** A callback passed to the popover onScroll */
312+
onPopoverScroll?: JSX.HTMLAttributes<HTMLElement>['onScroll'];
313+
311314
/**
312315
* Indicates how overflowing content should be handled in the listbox.
313316
*
@@ -349,6 +352,7 @@ function SelectMain<T>({
349352
popoverClasses = listboxClasses,
350353
containerClasses,
351354
onListboxScroll,
355+
onPopoverScroll = onListboxScroll,
352356
right = false,
353357
alignListbox = right ? 'right' : 'left',
354358
multiple,
@@ -455,6 +459,7 @@ function SelectMain<T>({
455459
asNativePopover={listboxAsPopover}
456460
align={alignListbox}
457461
classes={popoverClasses}
462+
onScroll={onPopoverScroll}
458463
>
459464
<ul
460465
role="listbox"
@@ -463,7 +468,6 @@ function SelectMain<T>({
463468
aria-multiselectable={multiple}
464469
aria-labelledby={buttonId ?? defaultButtonId}
465470
aria-orientation="vertical"
466-
onScroll={onListboxScroll}
467471
>
468472
{children}
469473
</ul>

src/components/input/test/Select-test.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,19 @@ describe('Select', () => {
270270
assert.equal(getTabIndex(5), -1);
271271
});
272272

273+
[{ propName: 'onPopoverScroll' }, { propName: 'onListboxScroll' }].forEach(
274+
({ propName }) => {
275+
it(`invokes ${propName} when scrolling inside the Popover`, () => {
276+
const onScroll = sinon.stub();
277+
const wrapper = createComponent({ [propName]: onScroll });
278+
279+
wrapper.find('[data-testid="popover"]').simulate('scroll');
280+
281+
assert.called(onScroll);
282+
});
283+
},
284+
);
285+
273286
context('when Option is rendered outside of Select', () => {
274287
it('throws an error', () => {
275288
assert.throws(

src/pattern-library/components/patterns/feedback/PopoverPage.tsx

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,16 @@ export default function PopoverPage() {
142142
</Library.InfoItem>
143143
</Library.Info>
144144
</Library.SectionL3>
145+
<Library.SectionL3 title="onScroll">
146+
<Library.Info>
147+
<Library.InfoItem label="description">
148+
Handler for {'"'}scroll{'"'} events on the outermost element.
149+
</Library.InfoItem>
150+
<Library.InfoItem label="type">
151+
<code>{'() => void | undefined'}</code>
152+
</Library.InfoItem>
153+
</Library.Info>
154+
</Library.SectionL3>
145155
<Library.SectionL3 title="open">
146156
<Library.Info>
147157
<Library.InfoItem label="description">

src/pattern-library/components/patterns/input/SelectPage.tsx

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -454,10 +454,11 @@ export default function SelectPage() {
454454
withSource
455455
/>
456456
</Library.SectionL3>
457-
<Library.SectionL3 title="onListboxScroll">
457+
<Library.SectionL3 title="onPopoverScroll">
458458
<Library.Info>
459459
<Library.InfoItem label="description">
460-
A callback passed to the listbox <code>onScroll</code>.
460+
A callback passed to the <code>Popover</code>
461+
{"'"}s <code>onScroll</code>.
461462
</Library.InfoItem>
462463
<Library.InfoItem label="type">
463464
<code>

0 commit comments

Comments
 (0)