From ffdedf7af3c33bb02f789afb9dffecba7c6bb42b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rapha=C3=ABl=20Bosi?= <71827178+bosiraphael@users.noreply.github.com> Date: Thu, 22 May 2025 12:06:07 +0200 Subject: [PATCH] Fix "PageChangeEffect does not run when changing view on the same object" (#12196) Fixes https://github.com/twentyhq/core-team-issues/issues/950 This issue was due to the memoization inside `useIsMatchingLocation`, which was rerendered only if the pathname changed but not the search params. After discussion with @lucasbordeau, we decided to remove the hook `useIsMatchingLocation` and to create an equivalent util function which takes the location as an argument. --------- Co-authored-by: Lucas Bordeau --- .../__tests__/useIsMatchingLocation.test.tsx | 74 --------------- ...sePageChangeEffectNavigateLocation.test.ts | 12 +-- .../src/hooks/useIsMatchingLocation.ts | 34 ------- .../usePageChangeEffectNavigateLocation.ts | 25 +++--- .../modules/apollo/hooks/useApolloFactory.ts | 11 ++- .../effect-components/PageChangeEffect.tsx | 41 +++++---- .../auth/sign-in-up/hooks/useSignInUp.ts | 10 ++- .../components/ClientConfigProvider.tsx | 9 +- .../components/MainContextStoreProvider.tsx | 13 +-- .../fullscreen/hooks/useShowFullscreen.ts | 14 +-- .../hooks/__tests__/useShowAuthModal.test.tsx | 20 +++-- .../ui/layout/hooks/useShowAuthModal.ts | 29 +++--- .../modules/users/components/UserProvider.tsx | 11 +-- .../users/components/UserProviderEffect.tsx | 9 +- .../__tests__/isMatchingLocation.test.tsx | 89 +++++++++++++++++++ .../src/utils/isMatchingLocation.ts | 26 ++++++ 16 files changed, 232 insertions(+), 195 deletions(-) delete mode 100644 packages/twenty-front/src/hooks/__tests__/useIsMatchingLocation.test.tsx delete mode 100644 packages/twenty-front/src/hooks/useIsMatchingLocation.ts create mode 100644 packages/twenty-front/src/utils/__tests__/isMatchingLocation.test.tsx create mode 100644 packages/twenty-front/src/utils/isMatchingLocation.ts diff --git a/packages/twenty-front/src/hooks/__tests__/useIsMatchingLocation.test.tsx b/packages/twenty-front/src/hooks/__tests__/useIsMatchingLocation.test.tsx deleted file mode 100644 index b775f8988..000000000 --- a/packages/twenty-front/src/hooks/__tests__/useIsMatchingLocation.test.tsx +++ /dev/null @@ -1,74 +0,0 @@ -import { renderHook } from '@testing-library/react'; -import { useIsMatchingLocation } from '../useIsMatchingLocation'; -import { MemoryRouter } from 'react-router-dom'; -import { AppBasePath } from '@/types/AppBasePath'; - -const Wrapper = - (initialIndex = 0) => - ({ children }: { children: React.ReactNode }) => ( - - {children} - - ); - -describe('useIsMatchingLocation', () => { - it('returns true when paths match with no basePath', () => { - const { result } = renderHook(() => useIsMatchingLocation(), { - wrapper: Wrapper(), - }); - - expect(result.current.isMatchingLocation('/example')).toBe(true); - }); - - it('returns false when paths do not match with no basePath', () => { - const { result } = renderHook(() => useIsMatchingLocation(), { - wrapper: Wrapper(), - }); - - expect(result.current.isMatchingLocation('/non-match')).toBe(false); - }); - - it('returns true when paths match with basePath', () => { - const { result } = renderHook(() => useIsMatchingLocation(), { - wrapper: Wrapper(2), - }); - - expect( - result.current.isMatchingLocation('example', AppBasePath.Settings), - ).toBe(true); - }); - - it('returns false when paths do not match with basePath', () => { - const { result } = renderHook(() => useIsMatchingLocation(), { - wrapper: Wrapper(), - }); - - expect( - result.current.isMatchingLocation('non-match', AppBasePath.Settings), - ).toBe(false); - }); - - it('handles trailing slashes in basePath correctly', () => { - const { result } = renderHook(() => useIsMatchingLocation(), { - wrapper: Wrapper(2), - }); - - expect( - result.current.isMatchingLocation( - 'example', - (AppBasePath.Settings + '/') as AppBasePath, - ), - ).toBe(true); - }); - - it('handles without basePath correctly', () => { - const { result } = renderHook(() => useIsMatchingLocation(), { - wrapper: Wrapper(), - }); - - expect(result.current.isMatchingLocation('example')).toBe(true); - }); -}); diff --git a/packages/twenty-front/src/hooks/__tests__/usePageChangeEffectNavigateLocation.test.ts b/packages/twenty-front/src/hooks/__tests__/usePageChangeEffectNavigateLocation.test.ts index 29c4973e9..32f0bd3c8 100644 --- a/packages/twenty-front/src/hooks/__tests__/usePageChangeEffectNavigateLocation.test.ts +++ b/packages/twenty-front/src/hooks/__tests__/usePageChangeEffectNavigateLocation.test.ts @@ -8,9 +8,9 @@ import { useRecoilValue } from 'recoil'; import { OnboardingStatus } from '~/generated/graphql'; -import { useIsMatchingLocation } from '~/hooks/useIsMatchingLocation'; import { usePageChangeEffectNavigateLocation } from '~/hooks/usePageChangeEffectNavigateLocation'; import { UNTESTED_APP_PATHS } from '~/testing/constants/UntestedAppPaths'; +import { isMatchingLocation } from '~/utils/isMatchingLocation'; jest.mock('@/onboarding/hooks/useOnboardingStatus'); const setupMockOnboardingStatus = ( @@ -28,13 +28,13 @@ const setupMockIsWorkspaceActivationStatusEqualsTo = ( .mockReturnValueOnce(isWorkspaceSuspended); }; -jest.mock('~/hooks/useIsMatchingLocation'); -const mockUseIsMatchingLocation = jest.mocked(useIsMatchingLocation); +jest.mock('~/utils/isMatchingLocation'); +const mockIsMatchingLocation = jest.mocked(isMatchingLocation); const setupMockIsMatchingLocation = (pathname: string) => { - mockUseIsMatchingLocation.mockReturnValueOnce({ - isMatchingLocation: (path: string) => path === pathname, - }); + mockIsMatchingLocation.mockImplementation( + (_location, path) => path === pathname, + ); }; jest.mock('@/auth/hooks/useIsLogged'); diff --git a/packages/twenty-front/src/hooks/useIsMatchingLocation.ts b/packages/twenty-front/src/hooks/useIsMatchingLocation.ts deleted file mode 100644 index 70c175e3e..000000000 --- a/packages/twenty-front/src/hooks/useIsMatchingLocation.ts +++ /dev/null @@ -1,34 +0,0 @@ -import { matchPath, useLocation } from 'react-router-dom'; - -import { AppBasePath } from '@/types/AppBasePath'; -import { isNonEmptyString } from '@sniptt/guards'; -import { useCallback } from 'react'; -import { isDefined } from 'twenty-shared/utils'; - -const addTrailingSlash = (path: string) => - path.endsWith('/') ? path : path + '/'; - -const getConstructedPath = (path: string, basePath?: AppBasePath) => { - if (!isNonEmptyString(basePath)) return path; - - return addTrailingSlash(basePath) + path; -}; - -export const useIsMatchingLocation = () => { - const location = useLocation(); - - const isMatchingLocation = useCallback( - (path: string, basePath?: AppBasePath) => { - const match = matchPath( - getConstructedPath(path, basePath), - location.pathname, - ); - return isDefined(match); - }, - [location.pathname], - ); - - return { - isMatchingLocation, - }; -}; diff --git a/packages/twenty-front/src/hooks/usePageChangeEffectNavigateLocation.ts b/packages/twenty-front/src/hooks/usePageChangeEffectNavigateLocation.ts index ed3845b14..99c59c409 100644 --- a/packages/twenty-front/src/hooks/usePageChangeEffectNavigateLocation.ts +++ b/packages/twenty-front/src/hooks/usePageChangeEffectNavigateLocation.ts @@ -5,24 +5,24 @@ import { useOnboardingStatus } from '@/onboarding/hooks/useOnboardingStatus'; import { AppPath } from '@/types/AppPath'; import { SettingsPath } from '@/types/SettingsPath'; import { useIsWorkspaceActivationStatusEqualsTo } from '@/workspace/hooks/useIsWorkspaceActivationStatusEqualsTo'; -import { useParams } from 'react-router-dom'; +import { useLocation, useParams } from 'react-router-dom'; import { useRecoilValue } from 'recoil'; import { isDefined } from 'twenty-shared/utils'; import { WorkspaceActivationStatus } from 'twenty-shared/workspace'; import { OnboardingStatus } from '~/generated/graphql'; -import { useIsMatchingLocation } from '~/hooks/useIsMatchingLocation'; +import { isMatchingLocation } from '~/utils/isMatchingLocation'; export const usePageChangeEffectNavigateLocation = () => { - const { isMatchingLocation } = useIsMatchingLocation(); const isLoggedIn = useIsLogged(); const onboardingStatus = useOnboardingStatus(); const isWorkspaceSuspended = useIsWorkspaceActivationStatusEqualsTo( WorkspaceActivationStatus.SUSPENDED, ); const { defaultHomePagePath } = useDefaultHomePagePath(); + const location = useLocation(); const someMatchingLocationOf = (appPaths: AppPath[]): boolean => - appPaths.some((appPath) => isMatchingLocation(appPath)); + appPaths.some((appPath) => isMatchingLocation(location, appPath)); const onGoingUserCreationPaths = [ AppPath.Invite, AppPath.SignInUp, @@ -61,7 +61,10 @@ export const usePageChangeEffectNavigateLocation = () => { return AppPath.PlanRequired; } - if (isWorkspaceSuspended && !isMatchingLocation(AppPath.SettingsCatchAll)) { + if ( + isWorkspaceSuspended && + !isMatchingLocation(location, AppPath.SettingsCatchAll) + ) { return `${AppPath.SettingsCatchAll.replace('/*', '')}/${ SettingsPath.Billing }`; @@ -79,21 +82,21 @@ export const usePageChangeEffectNavigateLocation = () => { if ( onboardingStatus === OnboardingStatus.PROFILE_CREATION && - !isMatchingLocation(AppPath.CreateProfile) + !isMatchingLocation(location, AppPath.CreateProfile) ) { return AppPath.CreateProfile; } if ( onboardingStatus === OnboardingStatus.SYNC_EMAIL && - !isMatchingLocation(AppPath.SyncEmails) + !isMatchingLocation(location, AppPath.SyncEmails) ) { return AppPath.SyncEmails; } if ( onboardingStatus === OnboardingStatus.INVITE_TEAM && - !isMatchingLocation(AppPath.InviteTeam) + !isMatchingLocation(location, AppPath.InviteTeam) ) { return AppPath.InviteTeam; } @@ -101,18 +104,18 @@ export const usePageChangeEffectNavigateLocation = () => { if ( onboardingStatus === OnboardingStatus.COMPLETED && someMatchingLocationOf([...onboardingPaths, ...onGoingUserCreationPaths]) && - !isMatchingLocation(AppPath.ResetPassword) && + !isMatchingLocation(location, AppPath.ResetPassword) && isLoggedIn ) { return defaultHomePagePath; } - if (isMatchingLocation(AppPath.Index) && isLoggedIn) { + if (isMatchingLocation(location, AppPath.Index) && isLoggedIn) { return defaultHomePagePath; } if ( - isMatchingLocation(AppPath.RecordIndexPage) && + isMatchingLocation(location, AppPath.RecordIndexPage) && !isDefined(objectMetadataItem) ) { return AppPath.NotFound; diff --git a/packages/twenty-front/src/modules/apollo/hooks/useApolloFactory.ts b/packages/twenty-front/src/modules/apollo/hooks/useApolloFactory.ts index 54fb9ce5f..0779de0a1 100644 --- a/packages/twenty-front/src/modules/apollo/hooks/useApolloFactory.ts +++ b/packages/twenty-front/src/modules/apollo/hooks/useApolloFactory.ts @@ -11,8 +11,8 @@ import { tokenPairState } from '@/auth/states/tokenPairState'; import { workspacesState } from '@/auth/states/workspaces'; import { isDebugModeState } from '@/client-config/states/isDebugModeState'; import { REACT_APP_SERVER_BASE_URL } from '~/config'; -import { useIsMatchingLocation } from '~/hooks/useIsMatchingLocation'; import { useUpdateEffect } from '~/hooks/useUpdateEffect'; +import { isMatchingLocation } from '~/utils/isMatchingLocation'; import { currentUserWorkspaceState } from '@/auth/states/currentUserWorkspaceState'; import { AppPath } from '@/types/AppPath'; @@ -25,7 +25,6 @@ export const useApolloFactory = (options: Partial> = {}) => { const [isDebugMode] = useRecoilState(isDebugModeState); const navigate = useNavigate(); - const { isMatchingLocation } = useIsMatchingLocation(); const setTokenPair = useSetRecoilState(tokenPairState); const [currentWorkspace, setCurrentWorkspace] = useRecoilState( currentWorkspaceState, @@ -74,10 +73,10 @@ export const useApolloFactory = (options: Partial> = {}) => { setCurrentUserWorkspace(null); setWorkspaces([]); if ( - !isMatchingLocation(AppPath.Verify) && - !isMatchingLocation(AppPath.SignInUp) && - !isMatchingLocation(AppPath.Invite) && - !isMatchingLocation(AppPath.ResetPassword) + !isMatchingLocation(location, AppPath.Verify) && + !isMatchingLocation(location, AppPath.SignInUp) && + !isMatchingLocation(location, AppPath.Invite) && + !isMatchingLocation(location, AppPath.ResetPassword) ) { setPreviousUrl(`${location.pathname}${location.search}`); navigate(AppPath.SignInUp); diff --git a/packages/twenty-front/src/modules/app/effect-components/PageChangeEffect.tsx b/packages/twenty-front/src/modules/app/effect-components/PageChangeEffect.tsx index 0749116df..30d1850ba 100644 --- a/packages/twenty-front/src/modules/app/effect-components/PageChangeEffect.tsx +++ b/packages/twenty-front/src/modules/app/effect-components/PageChangeEffect.tsx @@ -36,15 +36,14 @@ import { useSetHotkeyScope } from '@/ui/utilities/hotkey/hooks/useSetHotkeyScope import { useRecoilComponentValueV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValueV2'; import { isDefined } from 'twenty-shared/utils'; import { AnalyticsType } from '~/generated/graphql'; -import { useIsMatchingLocation } from '~/hooks/useIsMatchingLocation'; import { usePageChangeEffectNavigateLocation } from '~/hooks/usePageChangeEffectNavigateLocation'; import { useInitializeQueryParamState } from '~/modules/app/hooks/useInitializeQueryParamState'; +import { isMatchingLocation } from '~/utils/isMatchingLocation'; import { getPageTitleFromPath } from '~/utils/title-utils'; // TODO: break down into smaller functions and / or hooks // - moved usePageChangeEffectNavigateLocation into dedicated hook export const PageChangeEffect = () => { const navigate = useNavigate(); - const { isMatchingLocation } = useIsMatchingLocation(); const [previousLocation, setPreviousLocation] = useState(''); @@ -126,7 +125,6 @@ export const PageChangeEffect = () => { } } }, [ - isMatchingLocation, previousLocation, resetTableSelections, unfocusRecordTableRow, @@ -139,28 +137,28 @@ export const PageChangeEffect = () => { useEffect(() => { switch (true) { - case isMatchingLocation(AppPath.RecordIndexPage): { + case isMatchingLocation(location, AppPath.RecordIndexPage): { setHotkeyScope(RecordIndexHotkeyScope.RecordIndex, { goto: true, keyboardShortcutMenu: true, }); break; } - case isMatchingLocation(AppPath.RecordShowPage): { + case isMatchingLocation(location, AppPath.RecordShowPage): { setHotkeyScope(PageHotkeyScope.CompanyShowPage, { goto: true, keyboardShortcutMenu: true, }); break; } - case isMatchingLocation(AppPath.OpportunitiesPage): { + case isMatchingLocation(location, AppPath.OpportunitiesPage): { setHotkeyScope(PageHotkeyScope.OpportunitiesPage, { goto: true, keyboardShortcutMenu: true, }); break; } - case isMatchingLocation(AppPath.TasksPage): { + case isMatchingLocation(location, AppPath.TasksPage): { setHotkeyScope(PageHotkeyScope.TaskPage, { goto: true, keyboardShortcutMenu: true, @@ -168,42 +166,50 @@ export const PageChangeEffect = () => { break; } - case isMatchingLocation(AppPath.SignInUp): { + case isMatchingLocation(location, AppPath.SignInUp): { setHotkeyScope(PageHotkeyScope.SignInUp); break; } - case isMatchingLocation(AppPath.Invite): { + case isMatchingLocation(location, AppPath.Invite): { setHotkeyScope(PageHotkeyScope.SignInUp); break; } - case isMatchingLocation(AppPath.CreateProfile): { + case isMatchingLocation(location, AppPath.CreateProfile): { setHotkeyScope(PageHotkeyScope.CreateProfile); break; } - case isMatchingLocation(AppPath.CreateWorkspace): { + case isMatchingLocation(location, AppPath.CreateWorkspace): { setHotkeyScope(PageHotkeyScope.CreateWorkspace); break; } - case isMatchingLocation(AppPath.SyncEmails): { + case isMatchingLocation(location, AppPath.SyncEmails): { setHotkeyScope(PageHotkeyScope.SyncEmail); break; } - case isMatchingLocation(AppPath.InviteTeam): { + case isMatchingLocation(location, AppPath.InviteTeam): { setHotkeyScope(PageHotkeyScope.InviteTeam); break; } - case isMatchingLocation(AppPath.PlanRequired): { + case isMatchingLocation(location, AppPath.PlanRequired): { setHotkeyScope(PageHotkeyScope.PlanRequired); break; } - case isMatchingLocation(SettingsPath.ProfilePage, AppBasePath.Settings): { + case isMatchingLocation( + location, + SettingsPath.ProfilePage, + AppBasePath.Settings, + ): { setHotkeyScope(PageHotkeyScope.ProfilePage, { goto: true, keyboardShortcutMenu: true, }); break; } - case isMatchingLocation(SettingsPath.Domain, AppBasePath.Settings): { + case isMatchingLocation( + location, + SettingsPath.Domain, + AppBasePath.Settings, + ): { setHotkeyScope(PageHotkeyScope.Settings, { goto: false, keyboardShortcutMenu: true, @@ -211,6 +217,7 @@ export const PageChangeEffect = () => { break; } case isMatchingLocation( + location, SettingsPath.WorkspaceMembersPage, AppBasePath.Settings, ): { @@ -221,7 +228,7 @@ export const PageChangeEffect = () => { break; } } - }, [isMatchingLocation, setHotkeyScope]); + }, [location, setHotkeyScope]); useEffect(() => { setTimeout(() => { diff --git a/packages/twenty-front/src/modules/auth/sign-in-up/hooks/useSignInUp.ts b/packages/twenty-front/src/modules/auth/sign-in-up/hooks/useSignInUp.ts index 5314cfb24..f5b644309 100644 --- a/packages/twenty-front/src/modules/auth/sign-in-up/hooks/useSignInUp.ts +++ b/packages/twenty-front/src/modules/auth/sign-in-up/hooks/useSignInUp.ts @@ -1,6 +1,6 @@ import { useCallback, useState } from 'react'; import { SubmitHandler, UseFormReturn } from 'react-hook-form'; -import { useParams, useSearchParams } from 'react-router-dom'; +import { useLocation, useParams, useSearchParams } from 'react-router-dom'; import { Form } from '@/auth/sign-in-up/hooks/useSignInUpForm'; import { signInUpModeState } from '@/auth/states/signInUpModeState'; @@ -15,7 +15,7 @@ import { AppPath } from '@/types/AppPath'; import { SnackBarVariant } from '@/ui/feedback/snack-bar-manager/components/SnackBar'; import { useSnackBar } from '@/ui/feedback/snack-bar-manager/hooks/useSnackBar'; import { useRecoilState } from 'recoil'; -import { useIsMatchingLocation } from '~/hooks/useIsMatchingLocation'; +import { isMatchingLocation } from '~/utils/isMatchingLocation'; import { useAuth } from '../../hooks/useAuth'; export const useSignInUp = (form: UseFormReturn
) => { @@ -24,14 +24,16 @@ export const useSignInUp = (form: UseFormReturn) => { const [signInUpStep, setSignInUpStep] = useRecoilState(signInUpStepState); const [signInUpMode, setSignInUpMode] = useRecoilState(signInUpModeState); - const { isMatchingLocation } = useIsMatchingLocation(); + const location = useLocation(); const workspaceInviteHash = useParams().workspaceInviteHash; const [searchParams] = useSearchParams(); const workspacePersonalInviteToken = searchParams.get('inviteToken') ?? undefined; - const [isInviteMode] = useState(() => isMatchingLocation(AppPath.Invite)); + const [isInviteMode] = useState(() => + isMatchingLocation(location, AppPath.Invite), + ); const { signInWithCredentials, diff --git a/packages/twenty-front/src/modules/client-config/components/ClientConfigProvider.tsx b/packages/twenty-front/src/modules/client-config/components/ClientConfigProvider.tsx index 4ff304e77..b0a7346cf 100644 --- a/packages/twenty-front/src/modules/client-config/components/ClientConfigProvider.tsx +++ b/packages/twenty-front/src/modules/client-config/components/ClientConfigProvider.tsx @@ -3,7 +3,8 @@ import { useRecoilValue } from 'recoil'; import { clientConfigApiStatusState } from '@/client-config/states/clientConfigApiStatusState'; import { AppFullScreenErrorFallback } from '@/error-handler/components/AppFullScreenErrorFallback'; import { AppPath } from '@/types/AppPath'; -import { useIsMatchingLocation } from '~/hooks/useIsMatchingLocation'; +import { useLocation } from 'react-router-dom'; +import { isMatchingLocation } from '~/utils/isMatchingLocation'; export const ClientConfigProvider: React.FC = ({ children, @@ -12,13 +13,13 @@ export const ClientConfigProvider: React.FC = ({ clientConfigApiStatusState, ); - const { isMatchingLocation } = useIsMatchingLocation(); + const location = useLocation(); // TODO: Implement a better loading strategy if ( !isLoaded && - !isMatchingLocation(AppPath.Verify) && - !isMatchingLocation(AppPath.VerifyEmail) + !isMatchingLocation(location, AppPath.Verify) && + !isMatchingLocation(location, AppPath.VerifyEmail) ) return null; diff --git a/packages/twenty-front/src/modules/context-store/components/MainContextStoreProvider.tsx b/packages/twenty-front/src/modules/context-store/components/MainContextStoreProvider.tsx index a5edc9a6b..bc8bf318b 100644 --- a/packages/twenty-front/src/modules/context-store/components/MainContextStoreProvider.tsx +++ b/packages/twenty-front/src/modules/context-store/components/MainContextStoreProvider.tsx @@ -5,10 +5,10 @@ import { objectMetadataItemsState } from '@/object-metadata/states/objectMetadat import { prefetchIndexViewIdFromObjectMetadataItemFamilySelector } from '@/prefetch/states/selector/prefetchIndexViewIdFromObjectMetadataItemFamilySelector'; import { AppPath } from '@/types/AppPath'; import { useShowAuthModal } from '@/ui/layout/hooks/useShowAuthModal'; -import { useParams, useSearchParams } from 'react-router-dom'; +import { useLocation, useParams, useSearchParams } from 'react-router-dom'; import { useRecoilValue } from 'recoil'; import { isDefined } from 'twenty-shared/utils'; -import { useIsMatchingLocation } from '~/hooks/useIsMatchingLocation'; +import { isMatchingLocation } from '~/utils/isMatchingLocation'; const getViewId = ( viewIdFromQueryParams: string | null, @@ -31,9 +31,12 @@ const getViewId = ( }; export const MainContextStoreProvider = () => { - const { isMatchingLocation } = useIsMatchingLocation(); - const isRecordIndexPage = isMatchingLocation(AppPath.RecordIndexPage); - const isRecordShowPage = isMatchingLocation(AppPath.RecordShowPage); + const location = useLocation(); + const isRecordIndexPage = isMatchingLocation( + location, + AppPath.RecordIndexPage, + ); + const isRecordShowPage = isMatchingLocation(location, AppPath.RecordShowPage); const isSettingsPage = useIsSettingsPage(); const objectNamePlural = useParams().objectNamePlural ?? ''; diff --git a/packages/twenty-front/src/modules/ui/layout/fullscreen/hooks/useShowFullscreen.ts b/packages/twenty-front/src/modules/ui/layout/fullscreen/hooks/useShowFullscreen.ts index 08e983b21..4a0e0d0c4 100644 --- a/packages/twenty-front/src/modules/ui/layout/fullscreen/hooks/useShowFullscreen.ts +++ b/packages/twenty-front/src/modules/ui/layout/fullscreen/hooks/useShowFullscreen.ts @@ -1,19 +1,23 @@ import { useMemo } from 'react'; import { SettingsPath } from '@/types/SettingsPath'; -import { useIsMatchingLocation } from '~/hooks/useIsMatchingLocation'; +import { useLocation } from 'react-router-dom'; +import { isMatchingLocation } from '~/utils/isMatchingLocation'; export const useShowFullscreen = () => { - const { isMatchingLocation } = useIsMatchingLocation(); + const location = useLocation(); return useMemo(() => { if ( - isMatchingLocation('settings/' + SettingsPath.RestPlayground + '/*') || - isMatchingLocation('settings/' + SettingsPath.GraphQLPlayground) + isMatchingLocation( + location, + 'settings/' + SettingsPath.RestPlayground + '/*', + ) || + isMatchingLocation(location, 'settings/' + SettingsPath.GraphQLPlayground) ) { return true; } return false; - }, [isMatchingLocation]); + }, [location]); }; diff --git a/packages/twenty-front/src/modules/ui/layout/hooks/__tests__/useShowAuthModal.test.tsx b/packages/twenty-front/src/modules/ui/layout/hooks/__tests__/useShowAuthModal.test.tsx index b0c3600c6..6cde9c538 100644 --- a/packages/twenty-front/src/modules/ui/layout/hooks/__tests__/useShowAuthModal.test.tsx +++ b/packages/twenty-front/src/modules/ui/layout/hooks/__tests__/useShowAuthModal.test.tsx @@ -1,17 +1,25 @@ import { renderHook } from '@testing-library/react'; +import * as reactRouterDom from 'react-router-dom'; import { RecoilRoot } from 'recoil'; import { AppPath } from '@/types/AppPath'; import { useShowAuthModal } from '@/ui/layout/hooks/useShowAuthModal'; -import { useIsMatchingLocation } from '~/hooks/useIsMatchingLocation'; +import { isMatchingLocation } from '~/utils/isMatchingLocation'; -jest.mock('~/hooks/useIsMatchingLocation'); -const mockUseIsMatchingLocation = jest.mocked(useIsMatchingLocation); +jest.mock('react-router-dom', () => ({ + useLocation: jest.fn(), +})); + +const mockUseLocation = reactRouterDom.useLocation as jest.Mock; + +jest.mock('~/utils/isMatchingLocation'); +const mockIsMatchingLocation = jest.mocked(isMatchingLocation); const setupMockIsMatchingLocation = (pathname: string) => { - mockUseIsMatchingLocation.mockReturnValueOnce({ - isMatchingLocation: (path: string) => path === pathname, - }); + mockUseLocation.mockReturnValue({ pathname }); + mockIsMatchingLocation.mockImplementation( + (_location, path) => path === pathname, + ); }; const getResult = () => diff --git a/packages/twenty-front/src/modules/ui/layout/hooks/useShowAuthModal.ts b/packages/twenty-front/src/modules/ui/layout/hooks/useShowAuthModal.ts index c4a8ca3a6..6cc23bda6 100644 --- a/packages/twenty-front/src/modules/ui/layout/hooks/useShowAuthModal.ts +++ b/packages/twenty-front/src/modules/ui/layout/hooks/useShowAuthModal.ts @@ -1,28 +1,29 @@ import { useMemo } from 'react'; import { AppPath } from '@/types/AppPath'; -import { useIsMatchingLocation } from '~/hooks/useIsMatchingLocation'; +import { useLocation } from 'react-router-dom'; +import { isMatchingLocation } from '~/utils/isMatchingLocation'; export const useShowAuthModal = () => { - const { isMatchingLocation } = useIsMatchingLocation(); + const location = useLocation(); return useMemo(() => { if ( - isMatchingLocation(AppPath.Invite) || - isMatchingLocation(AppPath.InviteTeam) || - isMatchingLocation(AppPath.CreateProfile) || - isMatchingLocation(AppPath.SyncEmails) || - isMatchingLocation(AppPath.ResetPassword) || - isMatchingLocation(AppPath.VerifyEmail) || - isMatchingLocation(AppPath.Verify) || - isMatchingLocation(AppPath.SignInUp) || - isMatchingLocation(AppPath.CreateWorkspace) || - isMatchingLocation(AppPath.PlanRequired) || - isMatchingLocation(AppPath.PlanRequiredSuccess) + isMatchingLocation(location, AppPath.Invite) || + isMatchingLocation(location, AppPath.InviteTeam) || + isMatchingLocation(location, AppPath.CreateProfile) || + isMatchingLocation(location, AppPath.SyncEmails) || + isMatchingLocation(location, AppPath.ResetPassword) || + isMatchingLocation(location, AppPath.VerifyEmail) || + isMatchingLocation(location, AppPath.Verify) || + isMatchingLocation(location, AppPath.SignInUp) || + isMatchingLocation(location, AppPath.CreateWorkspace) || + isMatchingLocation(location, AppPath.PlanRequired) || + isMatchingLocation(location, AppPath.PlanRequiredSuccess) ) { return true; } return false; - }, [isMatchingLocation]); + }, [location]); }; diff --git a/packages/twenty-front/src/modules/users/components/UserProvider.tsx b/packages/twenty-front/src/modules/users/components/UserProvider.tsx index a506bf1a4..81a48a216 100644 --- a/packages/twenty-front/src/modules/users/components/UserProvider.tsx +++ b/packages/twenty-front/src/modules/users/components/UserProvider.tsx @@ -5,19 +5,20 @@ import { isCurrentUserLoadedState } from '@/auth/states/isCurrentUserLoadingStat import { dateTimeFormatState } from '@/localization/states/dateTimeFormatState'; import { AppPath } from '@/types/AppPath'; import { UserContext } from '@/users/contexts/UserContext'; -import { useIsMatchingLocation } from '~/hooks/useIsMatchingLocation'; +import { useLocation } from 'react-router-dom'; import { UserOrMetadataLoader } from '~/loading/components/UserOrMetadataLoader'; +import { isMatchingLocation } from '~/utils/isMatchingLocation'; export const UserProvider = ({ children }: React.PropsWithChildren) => { const isCurrentUserLoaded = useRecoilValue(isCurrentUserLoadedState); - const { isMatchingLocation } = useIsMatchingLocation(); + const location = useLocation(); const dateTimeFormat = useRecoilValue(dateTimeFormatState); return !isCurrentUserLoaded && - !isMatchingLocation(AppPath.Verify) && - !isMatchingLocation(AppPath.VerifyEmail) && - !isMatchingLocation(AppPath.CreateWorkspace) ? ( + !isMatchingLocation(location, AppPath.Verify) && + !isMatchingLocation(location, AppPath.VerifyEmail) && + !isMatchingLocation(location, AppPath.CreateWorkspace) ? ( ) : ( { const [isLoading, setIsLoading] = useState(true); - const { isMatchingLocation } = useIsMatchingLocation(); + const location = useLocation(); const [isCurrentUserLoaded, setIsCurrentUserLoaded] = useRecoilState( isCurrentUserLoadedState, @@ -53,8 +54,8 @@ export const UserProviderEffect = () => { const { loading: queryLoading, data: queryData } = useGetCurrentUserQuery({ skip: isCurrentUserLoaded || - isMatchingLocation(AppPath.Verify) || - isMatchingLocation(AppPath.VerifyEmail), + isMatchingLocation(location, AppPath.Verify) || + isMatchingLocation(location, AppPath.VerifyEmail), }); useEffect(() => { diff --git a/packages/twenty-front/src/utils/__tests__/isMatchingLocation.test.tsx b/packages/twenty-front/src/utils/__tests__/isMatchingLocation.test.tsx new file mode 100644 index 000000000..11a4bd503 --- /dev/null +++ b/packages/twenty-front/src/utils/__tests__/isMatchingLocation.test.tsx @@ -0,0 +1,89 @@ +import { AppBasePath } from '@/types/AppBasePath'; +import { isMatchingLocation } from '../isMatchingLocation'; + +describe('isMatchingLocation', () => { + it('returns true when paths match with no basePath', () => { + const location = { + pathname: '/example', + state: null, + key: 'test-key', + search: '', + hash: '', + }; + const result = isMatchingLocation(location, '/example'); + expect(result).toBe(true); + }); + + it('returns false when paths do not match with no basePath', () => { + const location = { + pathname: '/example', + state: null, + key: 'test-key', + search: '', + hash: '', + }; + const result = isMatchingLocation(location, '/non-match'); + expect(result).toBe(false); + }); + + it('returns true when paths match with basePath', () => { + const location = { + pathname: `${AppBasePath.Settings}/example`, + state: null, + key: 'test-key', + search: '', + hash: '', + }; + const result = isMatchingLocation( + location, + 'example', + AppBasePath.Settings, + ); + expect(result).toBe(true); + }); + + it('returns false when paths do not match with basePath', () => { + const location = { + pathname: `${AppBasePath.Settings}/example`, + state: null, + key: 'test-key', + search: '', + hash: '', + }; + + const result = isMatchingLocation( + location, + 'non-match', + AppBasePath.Settings, + ); + expect(result).toBe(false); + }); + + it('handles trailing slashes in basePath correctly', () => { + const location = { + pathname: `${AppBasePath.Settings}/example`, + state: null, + key: 'test-key', + search: '', + hash: '', + }; + const result = isMatchingLocation( + location, + 'example', + (AppBasePath.Settings + '/') as AppBasePath, + ); + expect(result).toBe(true); + }); + + it('handles paths without basePath correctly', () => { + const location = { + pathname: '/example', + state: null, + key: 'test-key', + search: '', + hash: '', + }; + const result = isMatchingLocation(location, '/example'); + expect(result).toBe(true); + }); +}); diff --git a/packages/twenty-front/src/utils/isMatchingLocation.ts b/packages/twenty-front/src/utils/isMatchingLocation.ts new file mode 100644 index 000000000..3a5fa4987 --- /dev/null +++ b/packages/twenty-front/src/utils/isMatchingLocation.ts @@ -0,0 +1,26 @@ +import { Location, matchPath } from 'react-router-dom'; + +import { AppBasePath } from '@/types/AppBasePath'; +import { isNonEmptyString } from '@sniptt/guards'; +import { isDefined } from 'twenty-shared/utils'; + +const addTrailingSlash = (path: string) => + path.endsWith('/') ? path : path + '/'; + +const getConstructedPath = (path: string, basePath?: AppBasePath) => { + if (!isNonEmptyString(basePath)) return path; + + return addTrailingSlash(basePath) + path; +}; + +export const isMatchingLocation = ( + location: Location, + path: string, + basePath?: AppBasePath, +): boolean => { + const match = matchPath( + getConstructedPath(path, basePath), + location.pathname, + ); + return isDefined(match); +};