Introduce generic way to close any open dropdown on page location change (#11504)

In this PR we introduce a generic way to close any open dropdown
idempotently, with the hook useCloseAnyOpenDropdown.

We also introduce a generic hook useExecuteTasksOnAnyLocationChange that
is called each time the page location changes.

This way we can close any open dropdown when the page location changes,
which fixes the original issue of having advanced filter dropdown
staying open between page changes.

Fixes https://github.com/twentyhq/core-team-issues/issues/659
This commit is contained in:
Lucas Bordeau
2025-04-10 16:00:50 +02:00
committed by GitHub
parent 1844c39a99
commit f8f11894e8
8 changed files with 176 additions and 6 deletions

View File

@ -13,6 +13,7 @@ import {
setSessionId,
useEventTracker,
} from '@/analytics/hooks/useEventTracker';
import { useExecuteTasksOnAnyLocationChange } from '@/app/hooks/useExecuteTasksOnAnyLocationChange';
import { useRequestFreshCaptchaToken } from '@/captcha/hooks/useRequestFreshCaptchaToken';
import { isCaptchaScriptLoadedState } from '@/captcha/states/isCaptchaScriptLoadedState';
import { isCaptchaRequiredForPath } from '@/captcha/utils/isCaptchaRequiredForPath';
@ -52,13 +53,17 @@ export const PageChangeEffect = () => {
const resetTableSelections = useResetTableRowSelection(objectNamePlural);
const { executeTasksOnAnyLocationChange } =
useExecuteTasksOnAnyLocationChange();
useEffect(() => {
if (!previousLocation || previousLocation !== location.pathname) {
setPreviousLocation(location.pathname);
executeTasksOnAnyLocationChange();
} else {
return;
}
}, [location, previousLocation]);
}, [location, previousLocation, executeTasksOnAnyLocationChange]);
const [searchParams] = useSearchParams();

View File

@ -0,0 +1,16 @@
import { useCloseAnyOpenDropdown } from '@/ui/layout/dropdown/hooks/useCloseAnyOpenDropdown';
export const useExecuteTasksOnAnyLocationChange = () => {
const { closeAnyOpenDropdown } = useCloseAnyOpenDropdown();
/**
* Be careful to put idempotent tasks here.
*
* Because it might be called multiple times.
*/
const executeTasksOnAnyLocationChange = () => {
closeAnyOpenDropdown();
};
return { executeTasksOnAnyLocationChange };
};

View File

@ -0,0 +1,44 @@
import { expect } from '@storybook/test';
import { renderHook } from '@testing-library/react';
import { act } from 'react';
import { RecoilRoot } from 'recoil';
import { useCloseAnyOpenDropdown } from '@/ui/layout/dropdown/hooks/useCloseAnyOpenDropdown';
import { useDropdown } from '@/ui/layout/dropdown/hooks/useDropdown';
const dropdownId = 'test-dropdown-id';
const Wrapper = ({ children }: { children: React.ReactNode }) => {
return <RecoilRoot>{children}</RecoilRoot>;
};
describe('useCloseAnyOpenDropdown', () => {
it('should open dropdown and then close it with closeAnyOpenDropdown', async () => {
const { result } = renderHook(
() => {
const { openDropdown, isDropdownOpen } = useDropdown(dropdownId);
const { closeAnyOpenDropdown } = useCloseAnyOpenDropdown();
return { closeAnyOpenDropdown, isDropdownOpen, openDropdown };
},
{
wrapper: Wrapper,
},
);
expect(result.current.isDropdownOpen).toBe(false);
act(() => {
result.current.openDropdown();
});
expect(result.current.isDropdownOpen).toBe(true);
act(() => {
result.current.closeAnyOpenDropdown();
});
expect(result.current.isDropdownOpen).toBe(false);
});
});

View File

@ -0,0 +1,43 @@
import { expect } from '@storybook/test';
import { renderHook } from '@testing-library/react';
import { act } from 'react';
import { RecoilRoot } from 'recoil';
import { useCloseDropdownFromOutside } from '@/ui/layout/dropdown/hooks/useCloseDropdownFromOutside';
import { useDropdown } from '@/ui/layout/dropdown/hooks/useDropdown';
const dropdownId = 'test-dropdown-id';
const Wrapper = ({ children }: { children: React.ReactNode }) => {
return <RecoilRoot>{children}</RecoilRoot>;
};
describe('useCloseDropdownFromOutside', () => {
it('should close open dropdown', async () => {
const { result } = renderHook(
() => {
const { isDropdownOpen, openDropdown } = useDropdown(dropdownId);
const { closeDropdownFromOutside } = useCloseDropdownFromOutside();
return { closeDropdownFromOutside, isDropdownOpen, openDropdown };
},
{
wrapper: Wrapper,
},
);
expect(result.current.isDropdownOpen).toBe(false);
act(() => {
result.current.openDropdown();
});
expect(result.current.isDropdownOpen).toBe(true);
act(() => {
result.current.closeDropdownFromOutside(dropdownId);
});
expect(result.current.isDropdownOpen).toBe(false);
});
});

View File

@ -0,0 +1,51 @@
import { useCloseDropdownFromOutside } from '@/ui/layout/dropdown/hooks/useCloseDropdownFromOutside';
import { activeDropdownFocusIdState } from '@/ui/layout/dropdown/states/activeDropdownFocusIdState';
import { previousDropdownFocusIdState } from '@/ui/layout/dropdown/states/previousDropdownFocusIdState';
import { usePreviousHotkeyScope } from '@/ui/utilities/hotkey/hooks/usePreviousHotkeyScope';
import { useRecoilCallback } from 'recoil';
import { isDefined } from 'twenty-shared/utils';
export const useCloseAnyOpenDropdown = () => {
const { goBackToPreviousHotkeyScope } = usePreviousHotkeyScope();
const { closeDropdownFromOutside } = useCloseDropdownFromOutside();
const closeAnyOpenDropdown = useRecoilCallback(
({ snapshot, set }) =>
() => {
const previousDropdownFocusId = snapshot
.getLoadable(previousDropdownFocusIdState)
.getValue();
const activeDropdownFocusId = snapshot
.getLoadable(activeDropdownFocusIdState)
.getValue();
const thereIsNoDropdownOpen =
!isDefined(activeDropdownFocusId) &&
!isDefined(previousDropdownFocusId);
if (thereIsNoDropdownOpen) {
return;
}
const thereIsOneNestedDropdownOpen = isDefined(previousDropdownFocusId);
if (isDefined(activeDropdownFocusId)) {
closeDropdownFromOutside(activeDropdownFocusId);
}
if (thereIsOneNestedDropdownOpen) {
closeDropdownFromOutside(previousDropdownFocusId);
}
set(previousDropdownFocusIdState, null);
set(activeDropdownFocusIdState, null);
goBackToPreviousHotkeyScope();
},
[closeDropdownFromOutside, goBackToPreviousHotkeyScope],
);
return { closeAnyOpenDropdown };
};

View File

@ -0,0 +1,14 @@
import { isDropdownOpenComponentState } from '@/ui/layout/dropdown/states/isDropdownOpenComponentState';
import { useRecoilCallback } from 'recoil';
export const useCloseDropdownFromOutside = () => {
const closeDropdownFromOutside = useRecoilCallback(
({ set }) =>
(dropdownId: string) => {
set(isDropdownOpenComponentState({ scopeId: dropdownId }), false);
},
[],
);
return { closeDropdownFromOutside };
};

View File

@ -3,6 +3,7 @@ import { useRecoilCallback } from 'recoil';
import { activeDropdownFocusIdState } from '@/ui/layout/dropdown/states/activeDropdownFocusIdState';
import { previousDropdownFocusIdState } from '@/ui/layout/dropdown/states/previousDropdownFocusIdState';
// TODO: this won't work for more than 1 nested dropdown
export const useGoBackToPreviousDropdownFocusId = () => {
const goBackToPreviousDropdownFocusId = useRecoilCallback(
({ snapshot, set }) =>

View File

@ -7,10 +7,6 @@ export const useSetActiveDropdownFocusIdAndMemorizePrevious = () => {
const setActiveDropdownFocusIdAndMemorizePrevious = useRecoilCallback(
({ snapshot, set }) =>
(dropdownId: string | null) => {
const focusedDropdownId = snapshot
.getLoadable(activeDropdownFocusIdState)
.getValue();
const activeDropdownFocusId = snapshot
.getLoadable(activeDropdownFocusIdState)
.getValue();
@ -19,7 +15,7 @@ export const useSetActiveDropdownFocusIdAndMemorizePrevious = () => {
return;
}
set(previousDropdownFocusIdState, focusedDropdownId);
set(previousDropdownFocusIdState, activeDropdownFocusId);
set(activeDropdownFocusIdState, dropdownId);
},
[],