Ensure Select onScroll is bound to the Popover, not the listbox

hypothesis · Jan 31, 2025 · 1d6a1ca · 1d6a1ca
1 parent edf6957
commit 1d6a1ca
Show file tree

Hide file tree

Showing 6 changed files with 47 additions and 5 deletions.
diff --git a/src/components/feedback/Popover.tsx b/src/components/feedback/Popover.tsx
@@ -1,5 +1,5 @@
 import classnames from 'classnames';
-import type { ComponentChildren, RefObject } from 'preact';
+import type { ComponentChildren, JSX, RefObject } from 'preact';
 import { useCallback, useEffect, useLayoutEffect, useRef } from 'preact/hooks';
 
 import { useClickAway } from '../../hooks/use-click-away';
@@ -256,6 +256,9 @@ export type PopoverProps = {
    * Defaults to true, as long as the browser supports it.
    */
   asNativePopover?: boolean;
+
+  /** A callback passed to the outermost element onScroll */
+  onScroll?: JSX.HTMLAttributes<HTMLElement>['onScroll'];
 };
 
 export default function Popover({
@@ -267,6 +270,7 @@ export default function Popover({
   classes,
   variant = 'panel',
   restoreFocusOnClose = true,
+  onScroll,
   /* eslint-disable-next-line no-prototype-builtins */
   asNativePopover = HTMLElement.prototype.hasOwnProperty('popover'),
 }: PopoverProps) {
@@ -320,6 +324,7 @@ export default function Popover({
       )}
       ref={popoverRef}
       popover={asNativePopover && 'auto'}
+      onScroll={onScroll}
       data-testid="popover"
       data-component="Popover"
     >

diff --git a/src/components/feedback/test/Popover-test.js b/src/components/feedback/test/Popover-test.js
@@ -87,6 +87,15 @@ describe('Popover', () => {
     );
   };
 
+  it('invokes onScroll when scrolling inside the popover', () => {
+    const onScroll = sinon.stub();
+    const wrapper = createComponent({ onScroll });
+
+    getPopover(wrapper).find('[data-testid="popover"]').simulate('scroll');
+
+    assert.called(onScroll);
+  });
+
   [
     {
       restoreFocusOnClose: undefined, // Defaults to true

diff --git a/src/components/input/Select.tsx b/src/components/input/Select.tsx
@@ -305,9 +305,12 @@ type BaseSelectProps = CompositeProps & {
    */
   listboxAsPopover?: boolean;
 
-  /** A callback passed to the listbox onScroll */
+  /** @deprecated. Use onPopoverScroll instead */
   onListboxScroll?: JSX.HTMLAttributes<HTMLUListElement>['onScroll'];
 
+  /** A callback passed to the popover onScroll */
+  onPopoverScroll?: JSX.HTMLAttributes<HTMLElement>['onScroll'];
+
   /**
    * Indicates how overflowing content should be handled in the listbox.
    *
@@ -349,6 +352,7 @@ function SelectMain<T>({
   popoverClasses = listboxClasses,
   containerClasses,
   onListboxScroll,
+  onPopoverScroll = onListboxScroll,
   right = false,
   alignListbox = right ? 'right' : 'left',
   multiple,
@@ -455,6 +459,7 @@ function SelectMain<T>({
           asNativePopover={listboxAsPopover}
           align={alignListbox}
           classes={popoverClasses}
+          onScroll={onPopoverScroll}
         >
           <ul
             role="listbox"
@@ -463,7 +468,6 @@ function SelectMain<T>({
             aria-multiselectable={multiple}
             aria-labelledby={buttonId ?? defaultButtonId}
             aria-orientation="vertical"
-            onScroll={onListboxScroll}
           >
             {children}
           </ul>

diff --git a/src/components/input/test/Select-test.js b/src/components/input/test/Select-test.js
@@ -270,6 +270,19 @@ describe('Select', () => {
     assert.equal(getTabIndex(5), -1);
   });
 
+  [{ propName: 'onPopoverScroll' }, { propName: 'onListboxScroll' }].forEach(
+    ({ propName }) => {
+      it(`invokes ${propName} when scrolling inside the Popover`, () => {
+        const onScroll = sinon.stub();
+        const wrapper = createComponent({ [propName]: onScroll });
+
+        wrapper.find('[data-testid="popover"]').simulate('scroll');
+
+        assert.called(onScroll);
+      });
+    },
+  );
+
   context('when Option is rendered outside of Select', () => {
     it('throws an error', () => {
       assert.throws(

diff --git a/src/pattern-library/components/patterns/feedback/PopoverPage.tsx b/src/pattern-library/components/patterns/feedback/PopoverPage.tsx
@@ -142,6 +142,16 @@ export default function PopoverPage() {
               </Library.InfoItem>
             </Library.Info>
           </Library.SectionL3>
+          <Library.SectionL3 title="onScroll">
+            <Library.Info>
+              <Library.InfoItem label="description">
+                Handler for {'"'}scroll{'"'} events on the outermost element.
+              </Library.InfoItem>
+              <Library.InfoItem label="type">
+                <code>{'() => void | undefined'}</code>
+              </Library.InfoItem>
+            </Library.Info>
+          </Library.SectionL3>
           <Library.SectionL3 title="open">
             <Library.Info>
               <Library.InfoItem label="description">

diff --git a/src/pattern-library/components/patterns/input/SelectPage.tsx b/src/pattern-library/components/patterns/input/SelectPage.tsx
@@ -454,10 +454,11 @@ export default function SelectPage() {
               withSource
             />
           </Library.SectionL3>
-          <Library.SectionL3 title="onListboxScroll">
+          <Library.SectionL3 title="onPopoverScroll">
             <Library.Info>
               <Library.InfoItem label="description">
-                A callback passed to the listbox <code>onScroll</code>.
+                A callback passed to the <code>Popover</code>
+                {"'"}s <code>onScroll</code>.
               </Library.InfoItem>
               <Library.InfoItem label="type">
                 <code>