Fix dropdown (#12126)

In this PR:
- deprecating listenClickOutside ComparePixel mode as this is not
accurate. We were using to avoid portal issue with CompareHtmlRef mode
but this is still an issue when portalled content overflows the
container.
- add ClickOutsideContext to specify excluded className so portal
children can use it easily (part of the tooling)
- fix stories
- remove avoidPortal from dropdown as this was not used
This commit is contained in:
Charles Bochet
2025-05-19 16:37:51 +02:00
committed by GitHub
parent bb4fed5a5e
commit cba36af1e8
16 changed files with 153 additions and 326 deletions

View File

@ -0,0 +1,10 @@
import { createContext } from 'react';
type ClickOutsideListenerContextType = {
excludeClassName: string | undefined;
};
export const ClickOutsideListenerContext =
createContext<ClickOutsideListenerContextType>({
excludeClassName: undefined,
});

View File

@ -1,16 +1,11 @@
import { fireEvent, renderHook } from '@testing-library/react';
import React from 'react';
import { act } from 'react-dom/test-utils';
import React, { act } from 'react';
import { RecoilRoot } from 'recoil';
import {
ClickOutsideMode,
useListenClickOutside,
} from '@/ui/utilities/pointer-event/hooks/useListenClickOutside';
import { useListenClickOutside } from '@/ui/utilities/pointer-event/hooks/useListenClickOutside';
import { isDefined } from 'twenty-shared/utils';
const containerRef = React.createRef<HTMLDivElement>();
const nullRef = React.createRef<HTMLDivElement>();
const Wrapper = ({ children }: { children: React.ReactNode }) => (
<RecoilRoot>
@ -41,28 +36,6 @@ describe('useListenClickOutside', () => {
expect(callback).toHaveBeenCalled();
});
it('should trigger the callback when clicking outside the specified ref with pixel comparison', async () => {
const callback = jest.fn();
renderHook(
() =>
useListenClickOutside({
refs: [nullRef],
callback,
mode: ClickOutsideMode.comparePixels,
listenerId,
}),
{ wrapper: Wrapper },
);
act(() => {
fireEvent.mouseDown(document);
fireEvent.click(document);
});
expect(callback).toHaveBeenCalled();
});
it('should not call the callback when clicking inside the specified refs using default comparison', () => {
const callback = jest.fn();
@ -85,28 +58,4 @@ describe('useListenClickOutside', () => {
expect(callback).not.toHaveBeenCalled();
});
it('should not call the callback when clicking inside the specified refs using pixel comparison', () => {
const callback = jest.fn();
renderHook(
() =>
useListenClickOutside({
refs: [containerRef],
callback,
mode: ClickOutsideMode.comparePixels,
listenerId,
}),
{ wrapper: Wrapper },
);
act(() => {
if (isDefined(containerRef.current)) {
fireEvent.mouseDown(containerRef.current);
fireEvent.click(containerRef.current);
}
});
expect(callback).not.toHaveBeenCalled();
});
});

View File

@ -7,16 +7,10 @@ import { useRecoilCallback } from 'recoil';
const CLICK_OUTSIDE_DEBUG_MODE = false;
export enum ClickOutsideMode {
comparePixels = 'comparePixels',
compareHTMLRef = 'compareHTMLRef',
}
export type ClickOutsideListenerProps<T extends Element> = {
refs: Array<React.RefObject<T>>;
excludeClassNames?: string[];
callback: (event: MouseEvent | TouchEvent) => void;
mode?: ClickOutsideMode;
listenerId: string;
enabled?: boolean;
};
@ -25,7 +19,6 @@ export const useListenClickOutside = <T extends Element>({
refs,
excludeClassNames,
callback,
mode = ClickOutsideMode.compareHTMLRef,
listenerId,
enabled = true,
}: ClickOutsideListenerProps<T>) => {
@ -60,67 +53,16 @@ export const useListenClickOutside = <T extends Element>({
return;
}
switch (mode) {
case ClickOutsideMode.compareHTMLRef: {
const clickedOnAtLeastOneRef = refs
.filter((ref) => !!ref.current)
.some((ref) => ref.current?.contains(event.target as Node));
const clickedOnAtLeastOneRef = refs
.filter((ref) => !!ref.current)
.some((ref) => ref.current?.contains(event.target as Node));
set(
clickOutsideListenerIsMouseDownInsideState,
clickedOnAtLeastOneRef,
);
break;
}
case ClickOutsideMode.comparePixels: {
const clickedOnAtLeastOneRef = refs
.filter((ref) => !!ref.current)
.some((ref) => {
if (!ref.current) {
return false;
}
const { x, y, width, height } =
ref.current.getBoundingClientRect();
const clientX =
'clientX' in event
? event.clientX
: event.changedTouches[0].clientX;
const clientY =
'clientY' in event
? event.clientY
: event.changedTouches[0].clientY;
if (
clientX < x ||
clientX > x + width ||
clientY < y ||
clientY > y + height
) {
return false;
}
return true;
});
set(
clickOutsideListenerIsMouseDownInsideState,
clickedOnAtLeastOneRef,
);
break;
}
default: {
break;
}
}
set(clickOutsideListenerIsMouseDownInsideState, clickedOnAtLeastOneRef);
},
[
clickOutsideListenerIsActivatedState,
clickOutsideListenerMouseDownHappenedState,
enabled,
mode,
refs,
clickOutsideListenerIsMouseDownInsideState,
],
@ -162,94 +104,34 @@ export const useListenClickOutside = <T extends Element>({
currentElement = currentElement.parentElement;
}
if (mode === ClickOutsideMode.compareHTMLRef) {
const clickedOnAtLeastOneRef = refs
.filter((ref) => !!ref.current)
.some((ref) => ref.current?.contains(event.target as Node));
const clickedOnAtLeastOneRef = refs
.filter((ref) => !!ref.current)
.some((ref) => ref.current?.contains(event.target as Node));
const shouldTrigger =
isListening &&
hasMouseDownHappened &&
!clickedOnAtLeastOneRef &&
!isMouseDownInside &&
!isClickedOnExcluded;
const shouldTrigger =
isListening &&
hasMouseDownHappened &&
!clickedOnAtLeastOneRef &&
!isMouseDownInside &&
!isClickedOnExcluded;
if (CLICK_OUTSIDE_DEBUG_MODE) {
// eslint-disable-next-line no-console
console.log('click outside compare ref', {
listenerId,
shouldTrigger,
clickedOnAtLeastOneRef,
isMouseDownInside,
isListening,
hasMouseDownHappened,
isClickedOnExcluded,
enabled,
event,
});
}
if (shouldTrigger) {
callback(event);
}
if (CLICK_OUTSIDE_DEBUG_MODE) {
// eslint-disable-next-line no-console
console.log('click outside compare ref', {
listenerId,
shouldTrigger,
clickedOnAtLeastOneRef,
isMouseDownInside,
isListening,
hasMouseDownHappened,
isClickedOnExcluded,
enabled,
event,
});
}
if (mode === ClickOutsideMode.comparePixels) {
const clickedOnAtLeastOneRef = refs
.filter((ref) => !!ref.current)
.some((ref) => {
if (!ref.current) {
return false;
}
const { x, y, width, height } =
ref.current.getBoundingClientRect();
const clientX =
'clientX' in event
? event.clientX
: event.changedTouches[0].clientX;
const clientY =
'clientY' in event
? event.clientY
: event.changedTouches[0].clientY;
if (
clientX < x ||
clientX > x + width ||
clientY < y ||
clientY > y + height
) {
return false;
}
return true;
});
const shouldTrigger =
!clickedOnAtLeastOneRef &&
!isMouseDownInside &&
isListening &&
hasMouseDownHappened &&
!isClickedOnExcluded;
if (CLICK_OUTSIDE_DEBUG_MODE) {
// eslint-disable-next-line no-console
console.log('click outside compare pixel', {
listenerId,
shouldTrigger,
clickedOnAtLeastOneRef,
isMouseDownInside,
isListening,
hasMouseDownHappened,
isClickedOnExcluded,
enabled,
event,
});
}
if (shouldTrigger) {
callback(event);
}
if (shouldTrigger) {
callback(event);
}
},
[
@ -257,7 +139,6 @@ export const useListenClickOutside = <T extends Element>({
enabled,
clickOutsideListenerIsMouseDownInsideState,
clickOutsideListenerMouseDownHappenedState,
mode,
refs,
excludeClassNames,
callback,
@ -291,5 +172,5 @@ export const useListenClickOutside = <T extends Element>({
capture: true,
});
};
}, [refs, callback, mode, handleClickOutside, handleMouseDown]);
}, [refs, callback, handleClickOutside, handleMouseDown]);
};