diff --git a/change/@fluentui-react-menu-ca40103c-dab8-48fa-bb6e-6672d9fb6db6.json b/change/@fluentui-react-menu-ca40103c-dab8-48fa-bb6e-6672d9fb6db6.json new file mode 100644 index 00000000000000..775ea11986e116 --- /dev/null +++ b/change/@fluentui-react-menu-ca40103c-dab8-48fa-bb6e-6672d9fb6db6.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "fix: update Menu open change event type to include `Event` (as `useOnScrollOutside` adds scroll event listener)", + "packageName": "@fluentui/react-menu", + "email": "yuanboxue@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/change/@fluentui-react-popover-62309005-c7b0-4c3b-8273-7ecfad0889f9.json b/change/@fluentui-react-popover-62309005-c7b0-4c3b-8273-7ecfad0889f9.json new file mode 100644 index 00000000000000..755615b39cb358 --- /dev/null +++ b/change/@fluentui-react-popover-62309005-c7b0-4c3b-8273-7ecfad0889f9.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "fix: update type `OpenPopoverEvents` to include `Event` (as `useOnScrollOutside` adds scroll event listener)", + "packageName": "@fluentui/react-popover", + "email": "yuanboxue@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/change/@fluentui-react-utilities-3d0f8452-1cf1-433e-9c40-cfdaae09343b.json b/change/@fluentui-react-utilities-3d0f8452-1cf1-433e-9c40-cfdaae09343b.json new file mode 100644 index 00000000000000..ec1c9a97992c9a --- /dev/null +++ b/change/@fluentui-react-utilities-3d0f8452-1cf1-433e-9c40-cfdaae09343b.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "fix: `useOnScrollOutside` should invoke callback on dragging scrollbar", + "packageName": "@fluentui/react-utilities", + "email": "yuanboxue@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/packages/react-components/react-avatar/src/components/AvatarGroupPopover/AvatarGroupPopover.test.tsx b/packages/react-components/react-avatar/src/components/AvatarGroupPopover/AvatarGroupPopover.test.tsx index ddec3ad5a3ca72..05a3ef50b518bf 100644 --- a/packages/react-components/react-avatar/src/components/AvatarGroupPopover/AvatarGroupPopover.test.tsx +++ b/packages/react-components/react-avatar/src/components/AvatarGroupPopover/AvatarGroupPopover.test.tsx @@ -47,6 +47,10 @@ describe('AvatarGroupPopover', () => { getPortalElement: getPopoverSurfaceElement, }, ], + 'consistent-callback-args': { + // Popover onOpenChange uses Event type due to scroll event in useOnScrollOutside + ignoreProps: ['onOpenChange'], + }, }, requiredProps: { children: ( diff --git a/packages/react-components/react-menu/etc/react-menu.api.md b/packages/react-components/react-menu/etc/react-menu.api.md index 842259d4efc19a..4990891d08ba14 100644 --- a/packages/react-components/react-menu/etc/react-menu.api.md +++ b/packages/react-components/react-menu/etc/react-menu.api.md @@ -281,7 +281,7 @@ export type MenuOpenChangeData = { event: MouseEvent | TouchEvent; } | { type: 'scrollOutside'; - event: MouseEvent | TouchEvent; + event: MouseEvent | TouchEvent | Event; } | { type: 'menuMouseEnter'; event: MouseEvent | TouchEvent; diff --git a/packages/react-components/react-menu/src/components/Menu/Menu.types.ts b/packages/react-components/react-menu/src/components/Menu/Menu.types.ts index dcadc40dd74f80..34f1281d8cc3bb 100644 --- a/packages/react-components/react-menu/src/components/Menu/Menu.types.ts +++ b/packages/react-components/react-menu/src/components/Menu/Menu.types.ts @@ -239,7 +239,7 @@ export type MenuOpenChangeData = { } | { type: 'scrollOutside'; - event: MouseEvent | TouchEvent; + event: MouseEvent | TouchEvent | Event; } | { type: 'menuMouseEnter'; diff --git a/packages/react-components/react-menu/src/utils/useOnMenuEnter.ts b/packages/react-components/react-menu/src/utils/useOnMenuEnter.ts index 2caa166e23a4a1..a091d3ccae0608 100644 --- a/packages/react-components/react-menu/src/utils/useOnMenuEnter.ts +++ b/packages/react-components/react-menu/src/utils/useOnMenuEnter.ts @@ -1,6 +1,7 @@ import * as React from 'react'; import { useEventCallback } from '@fluentui/react-utilities'; import { elementContains } from '@fluentui/react-portal'; +// eslint-disable-next-line deprecation/deprecation import type { UseOnClickOrScrollOutsideOptions } from '@fluentui/react-utilities'; /** @@ -19,6 +20,7 @@ export const MENU_ENTER_EVENT = 'fuimenuenter'; * Instead, dispatch custom DOM event from the menu so that it can bubble * Each nested menu can use the listener to check if the event is from a child or parent menu */ +// eslint-disable-next-line deprecation/deprecation export const useOnMenuMouseEnter = (options: UseOnClickOrScrollOutsideOptions) => { const { refs, callback, element, disabled } = options; diff --git a/packages/react-components/react-popover/etc/react-popover.api.md b/packages/react-components/react-popover/etc/react-popover.api.md index e6cca50af5bddc..7f03ab71b90b11 100644 --- a/packages/react-components/react-popover/etc/react-popover.api.md +++ b/packages/react-components/react-popover/etc/react-popover.api.md @@ -33,7 +33,7 @@ export type OnOpenChangeData = { }; // @public -export type OpenPopoverEvents = MouseEvent | TouchEvent | React_2.FocusEvent | React_2.KeyboardEvent | React_2.MouseEvent; +export type OpenPopoverEvents = Event | MouseEvent | TouchEvent | React_2.FocusEvent | React_2.KeyboardEvent | React_2.MouseEvent; // @public export const Popover: React_2.FC; diff --git a/packages/react-components/react-popover/src/components/Popover/Popover.test.tsx b/packages/react-components/react-popover/src/components/Popover/Popover.test.tsx index 680621a2dac2bf..23d374521ecb4a 100644 --- a/packages/react-components/react-popover/src/components/Popover/Popover.test.tsx +++ b/packages/react-components/react-popover/src/components/Popover/Popover.test.tsx @@ -18,6 +18,12 @@ describe('Popover', () => { // Popover does not have own styles 'make-styles-overrides-win', ], + testOptions: { + 'consistent-callback-args': { + // Popover onOpenChange uses Event type due to scroll event in useOnScrollOutside + ignoreProps: ['onOpenChange'], + }, + }, }); /** diff --git a/packages/react-components/react-popover/src/components/Popover/Popover.types.ts b/packages/react-components/react-popover/src/components/Popover/Popover.types.ts index 65b08a76a2dfe0..569d077d244902 100644 --- a/packages/react-components/react-popover/src/components/Popover/Popover.types.ts +++ b/packages/react-components/react-popover/src/components/Popover/Popover.types.ts @@ -209,6 +209,7 @@ export type OnOpenChangeData = { open: boolean }; * The supported events that will trigger open/close of the menu */ export type OpenPopoverEvents = + | Event | MouseEvent | TouchEvent | React.FocusEvent diff --git a/packages/react-components/react-utilities/etc/react-utilities.api.md b/packages/react-components/react-utilities/etc/react-utilities.api.md index 5ac7b70c5fe23a..5703c3896f0b2c 100644 --- a/packages/react-components/react-utilities/etc/react-utilities.api.md +++ b/packages/react-components/react-utilities/etc/react-utilities.api.md @@ -328,8 +328,14 @@ export function useIsSSR(): boolean; // @public export function useMergedRefs(...refs: (React_2.Ref | undefined)[]): RefObjectFunction; -// @internal (undocumented) -export type UseOnClickOrScrollOutsideOptions = { +// @public @deprecated (undocumented) +export type UseOnClickOrScrollOutsideOptions = UseOnClickOutsideOptions; + +// @internal +export const useOnClickOutside: (options: UseOnClickOutsideOptions) => void; + +// @public (undocumented) +export type UseOnClickOutsideOptions = { element: Document | undefined; refs: React_2.MutableRefObject[]; contains?(parent: HTMLElement | null, child: HTMLElement): boolean; @@ -339,10 +345,12 @@ export type UseOnClickOrScrollOutsideOptions = { }; // @internal -export const useOnClickOutside: (options: UseOnClickOrScrollOutsideOptions) => void; +export const useOnScrollOutside: (options: UseOnScrollOutsideOptions) => void; -// @internal -export const useOnScrollOutside: (options: UseOnClickOrScrollOutsideOptions) => void; +// @public (undocumented) +export type UseOnScrollOutsideOptions = Pick & { + callback: (ev: Event | MouseEvent | TouchEvent) => void; +}; // @internal (undocumented) export const usePrevious: (value: ValueType) => ValueType | null; diff --git a/packages/react-components/react-utilities/src/hooks/useOnClickOutside.ts b/packages/react-components/react-utilities/src/hooks/useOnClickOutside.ts index ff01345078de2e..9023d1b9362acf 100644 --- a/packages/react-components/react-utilities/src/hooks/useOnClickOutside.ts +++ b/packages/react-components/react-utilities/src/hooks/useOnClickOutside.ts @@ -1,10 +1,7 @@ import * as React from 'react'; import { useEventCallback } from './useEventCallback'; -/** - * @internal - */ -export type UseOnClickOrScrollOutsideOptions = { +export type UseOnClickOutsideOptions = { /** * The element to listen for the click event */ @@ -38,13 +35,18 @@ export type UseOnClickOrScrollOutsideOptions = { callback: (ev: MouseEvent | TouchEvent) => void; }; -const DEFAULT_CONTAINS: UseOnClickOrScrollOutsideOptions['contains'] = (parent, child) => !!parent?.contains(child); +/** + * @deprecated use UseOnClickOutsideOptions instead + */ +export type UseOnClickOrScrollOutsideOptions = UseOnClickOutsideOptions; + +const DEFAULT_CONTAINS: UseOnClickOutsideOptions['contains'] = (parent, child) => !!parent?.contains(child); /** * @internal * Utility to perform checks where a click/touch event was made outside a component */ -export const useOnClickOutside = (options: UseOnClickOrScrollOutsideOptions) => { +export const useOnClickOutside = (options: UseOnClickOutsideOptions) => { const { refs, callback, element, disabled, disabledFocusOnIframe, contains = DEFAULT_CONTAINS } = options; const timeoutId = React.useRef(undefined); @@ -130,7 +132,7 @@ const getWindowEvent = (target: Node | Window): Event | undefined => { const FUI_FRAME_EVENT = 'fuiframefocus'; interface UseIFrameFocusOptions - extends Pick { + extends Pick { /** * Millisecond duration to poll */ diff --git a/packages/react-components/react-utilities/src/hooks/useOnScrollOutside.test.ts b/packages/react-components/react-utilities/src/hooks/useOnScrollOutside.test.ts index 669c369ba831ba..0fe9d82959ace3 100644 --- a/packages/react-components/react-utilities/src/hooks/useOnScrollOutside.test.ts +++ b/packages/react-components/react-utilities/src/hooks/useOnScrollOutside.test.ts @@ -2,9 +2,9 @@ import { renderHook } from '@testing-library/react-hooks'; import { useOnScrollOutside } from './useOnScrollOutside'; describe('useOnScrollOutside', () => { - const supportedEvents = ['wheel', 'touchmove']; + const supportedEvents = [{ event: 'wheel' }, { event: 'touchmove' }, { event: 'scroll', capture: true }]; - it.each(supportedEvents)('should add %s listener', event => { + it.each(supportedEvents)('should add %s listener', ({ event, capture }) => { // Arrange const element = { addEventListener: jest.fn(), removeEventListener: jest.fn() } as unknown as Document; @@ -12,11 +12,13 @@ describe('useOnScrollOutside', () => { renderHook(() => useOnScrollOutside({ element, callback: jest.fn(), refs: [] })); // Assert - expect(element.addEventListener).toHaveBeenCalledTimes(2); - expect(element.addEventListener).toHaveBeenCalledWith(event, expect.anything()); + expect(element.addEventListener).toHaveBeenCalledTimes(supportedEvents.length); + expect(element.addEventListener).toHaveBeenCalledWith( + ...(capture ? [event, expect.anything(), true] : [event, expect.anything()]), + ); }); - it.each(supportedEvents)('should cleanup %s listener', event => { + it.each(supportedEvents)('should cleanup %s listener', ({ event, capture }) => { // Arrange const element = { addEventListener: jest.fn(), removeEventListener: jest.fn() } as unknown as Document; @@ -25,8 +27,10 @@ describe('useOnScrollOutside', () => { unmount(); // Assert - expect(element.removeEventListener).toHaveBeenCalledTimes(2); - expect(element.removeEventListener).toHaveBeenCalledWith(event, expect.anything()); + expect(element.removeEventListener).toHaveBeenCalledTimes(supportedEvents.length); + expect(element.removeEventListener).toHaveBeenCalledWith( + ...(capture ? [event, expect.anything(), true] : [event, expect.anything()]), + ); }); it('should not add or remove event listeners when disabled', () => { diff --git a/packages/react-components/react-utilities/src/hooks/useOnScrollOutside.ts b/packages/react-components/react-utilities/src/hooks/useOnScrollOutside.ts index 86043fe1c60320..7406203e511a6b 100644 --- a/packages/react-components/react-utilities/src/hooks/useOnScrollOutside.ts +++ b/packages/react-components/react-utilities/src/hooks/useOnScrollOutside.ts @@ -1,16 +1,23 @@ import * as React from 'react'; import { useEventCallback } from './useEventCallback'; -import type { UseOnClickOrScrollOutsideOptions } from './useOnClickOutside'; +import { UseOnClickOutsideOptions } from './useOnClickOutside'; + +export type UseOnScrollOutsideOptions = Pick & { + /** + * Called if the scroll is outside the element refs + */ + callback: (ev: Event | MouseEvent | TouchEvent) => void; +}; /** * @internal - * Utility to perform checks where a click/touch event was made outside a component + * Utility to perform checks where a scroll/touch event was made outside a component */ -export const useOnScrollOutside = (options: UseOnClickOrScrollOutsideOptions) => { +export const useOnScrollOutside = (options: UseOnScrollOutsideOptions) => { const { refs, callback, element, disabled, contains: containsProp } = options; - const listener = useEventCallback((ev: MouseEvent | TouchEvent) => { - const contains: UseOnClickOrScrollOutsideOptions['contains'] = + const listener = useEventCallback((ev: Event | MouseEvent | TouchEvent) => { + const contains: UseOnScrollOutsideOptions['contains'] = containsProp || ((parent, child) => !!parent?.contains(child)); const target = ev.composedPath()[0] as HTMLElement; @@ -28,10 +35,13 @@ export const useOnScrollOutside = (options: UseOnClickOrScrollOutsideOptions) => element?.addEventListener('wheel', listener); element?.addEventListener('touchmove', listener); + // use capture phase because scroll does not bubble + element?.addEventListener('scroll', listener, true); return () => { element?.removeEventListener('wheel', listener); element?.removeEventListener('touchmove', listener); + element?.removeEventListener('scroll', listener, true); }; }, [listener, element, disabled]); }; diff --git a/packages/react-components/react-utilities/src/index.ts b/packages/react-components/react-utilities/src/index.ts index 97fc981ebeb745..85e4e5209b095a 100644 --- a/packages/react-components/react-utilities/src/index.ts +++ b/packages/react-components/react-utilities/src/index.ts @@ -44,7 +44,14 @@ export { useScrollbarWidth, useTimeout, } from './hooks/index'; -export type { RefObjectFunction, UseControllableStateOptions, UseOnClickOrScrollOutsideOptions } from './hooks/index'; +export type { + RefObjectFunction, + UseControllableStateOptions, + // eslint-disable-next-line deprecation/deprecation + UseOnClickOrScrollOutsideOptions, + UseOnClickOutsideOptions, + UseOnScrollOutsideOptions, +} from './hooks/index'; export { canUseDOM, useIsSSR, SSRProvider } from './ssr/index';