-
Notifications
You must be signed in to change notification settings - Fork 2.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Simplify last visited #10259
Simplify last visited #10259
Changes from all commits
10bc82b
386eeff
d6310fb
2450f94
fdbc063
bf0b770
7c7abf0
bb9dd4b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,8 @@ | ||
import { contextStoreCurrentObjectMetadataIdComponentState } from '@/context-store/states/contextStoreCurrentObjectMetadataIdComponentState'; | ||
import { contextStoreCurrentViewIdComponentState } from '@/context-store/states/contextStoreCurrentViewIdComponentState'; | ||
import { mainContextStoreComponentInstanceIdState } from '@/context-store/states/mainContextStoreComponentInstanceId'; | ||
import { useLastVisitedObjectMetadataItem } from '@/navigation/hooks/useLastVisitedObjectMetadataItem'; | ||
import { useLastVisitedView } from '@/navigation/hooks/useLastVisitedView'; | ||
import { useSetLastVisitedObjectMetadataId } from '@/navigation/hooks/useSetLastVisitedObjectMetadataId'; | ||
import { useSetLastVisitedViewForObjectMetadataNamePlural } from '@/navigation/hooks/useSetLastVisitedViewForObjectMetadataNamePlural'; | ||
import { ObjectMetadataItem } from '@/object-metadata/types/ObjectMetadataItem'; | ||
import { useRecoilComponentStateV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentStateV2'; | ||
import { useEffect } from 'react'; | ||
|
@@ -22,15 +22,11 @@ export const MainContextStoreProviderEffect = ({ | |
setMainContextStoreComponentInstanceId, | ||
] = useRecoilState(mainContextStoreComponentInstanceIdState); | ||
|
||
const { getLastVisitedViewIdFromObjectNamePlural, setLastVisitedView } = | ||
useLastVisitedView(); | ||
const { setLastVisitedViewForObjectMetadataNamePlural } = | ||
useSetLastVisitedViewForObjectMetadataNamePlural(); | ||
|
||
const { lastVisitedObjectMetadataItemId, setLastVisitedObjectMetadataItem } = | ||
useLastVisitedObjectMetadataItem(); | ||
|
||
const lastVisitedViewId = getLastVisitedViewIdFromObjectNamePlural( | ||
objectMetadataItem.namePlural, | ||
); | ||
const { setLastVisitedObjectMetadataId } = | ||
useSetLastVisitedObjectMetadataId(); | ||
|
||
const [contextStoreCurrentViewId, setContextStoreCurrentViewId] = | ||
useRecoilComponentStateV2( | ||
|
@@ -60,33 +56,29 @@ export const MainContextStoreProviderEffect = ({ | |
); | ||
} | ||
|
||
if (viewId !== lastVisitedViewId) { | ||
setLastVisitedView({ | ||
objectNamePlural: objectMetadataItem.namePlural, | ||
viewId: viewId, | ||
}); | ||
} | ||
setLastVisitedViewForObjectMetadataNamePlural({ | ||
objectNamePlural: objectMetadataItem.namePlural, | ||
viewId: viewId, | ||
}); | ||
|
||
if (objectMetadataItem.id !== lastVisitedObjectMetadataItemId) { | ||
setLastVisitedObjectMetadataItem(objectMetadataItem.namePlural); | ||
} | ||
setLastVisitedObjectMetadataId({ | ||
objectMetadataItemId: objectMetadataItem.id, | ||
}); | ||
Comment on lines
+64
to
+66
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. style: setLastVisitedObjectMetadataId is called unconditionally on every effect run. Consider adding a condition to only update when the ID changes. |
||
|
||
if (contextStoreCurrentViewId !== viewId) { | ||
setContextStoreCurrentViewId(viewId); | ||
} | ||
}, [ | ||
contextStoreCurrentObjectMetadataId, | ||
contextStoreCurrentViewId, | ||
lastVisitedObjectMetadataItemId, | ||
lastVisitedViewId, | ||
mainContextStoreComponentInstanceId, | ||
mainContextStoreComponentInstanceIdToSet, | ||
objectMetadataItem, | ||
objectMetadataItem.namePlural, | ||
setContextStoreCurrentObjectMetadataId, | ||
setContextStoreCurrentViewId, | ||
setLastVisitedObjectMetadataItem, | ||
setLastVisitedView, | ||
setLastVisitedObjectMetadataId, | ||
setLastVisitedViewForObjectMetadataNamePlural, | ||
setMainContextStoreComponentInstanceId, | ||
viewId, | ||
]); | ||
|
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,12 @@ | ||
import { currentMobileNavigationDrawerState } from '@/navigation/states/currentMobileNavigationDrawerState'; | ||
import { isNavigationDrawerExpandedState } from '@/ui/navigation/states/isNavigationDrawerExpanded'; | ||
import { useRecoilState } from 'recoil'; | ||
import { useSetRecoilState } from 'recoil'; | ||
|
||
export const useOpenSettingsMenu = () => { | ||
const [, setIsNavigationDrawerExpanded] = useRecoilState( | ||
const setIsNavigationDrawerExpanded = useSetRecoilState( | ||
isNavigationDrawerExpandedState, | ||
); | ||
const [, setCurrentMobileNavigationDrawer] = useRecoilState( | ||
const setCurrentMobileNavigationDrawer = useSetRecoilState( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Praise: Nice |
||
currentMobileNavigationDrawerState, | ||
); | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
import { lastVisitedObjectMetadataItemIdState } from '@/navigation/states/lastVisitedObjectMetadataItemIdState'; | ||
import { objectMetadataItemsState } from '@/object-metadata/states/objectMetadataItemsState'; | ||
import { useRecoilCallback } from 'recoil'; | ||
import { isDefined } from 'twenty-shared'; | ||
|
||
export const useSetLastVisitedObjectMetadataId = () => { | ||
const setLastVisitedObjectMetadataId = useRecoilCallback( | ||
({ set, snapshot }) => | ||
({ objectMetadataItemId }: { objectMetadataItemId: string }) => { | ||
const objectMetadataItems = snapshot | ||
.getLoadable(objectMetadataItemsState) | ||
.getValue(); | ||
|
||
const objectMetadataItem = objectMetadataItems.find( | ||
(item) => item.id === objectMetadataItemId, | ||
); | ||
|
||
const lastVisitedObjectMetadataItemId = snapshot | ||
.getLoadable(lastVisitedObjectMetadataItemIdState) | ||
.getValue(); | ||
|
||
if ( | ||
isDefined(objectMetadataItem) && | ||
lastVisitedObjectMetadataItemId !== objectMetadataItemId | ||
) { | ||
set(lastVisitedObjectMetadataItemIdState, objectMetadataItemId); | ||
} | ||
}, | ||
[], | ||
); | ||
|
||
return { | ||
setLastVisitedObjectMetadataId, | ||
}; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
import { lastVisitedViewPerObjectMetadataItemState } from '@/navigation/states/lastVisitedViewPerObjectMetadataItemState'; | ||
import { objectMetadataItemsState } from '@/object-metadata/states/objectMetadataItemsState'; | ||
import { useLazyPrefetchedData } from '@/prefetch/hooks/useLazyPrefetchData'; | ||
import { PrefetchKey } from '@/prefetch/types/PrefetchKey'; | ||
import { View } from '@/views/types/View'; | ||
import { useRecoilCallback } from 'recoil'; | ||
import { isDefined } from 'twenty-shared'; | ||
|
||
export const useSetLastVisitedViewForObjectMetadataNamePlural = () => { | ||
const { records: views, findManyRecords } = useLazyPrefetchedData<View>( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. introducing this hook but I think we should use a state and stop reading from apollo cache here |
||
PrefetchKey.AllViews, | ||
); | ||
|
||
const setLastVisitedViewForObjectMetadataNamePlural = useRecoilCallback( | ||
({ set, snapshot }) => | ||
async ({ | ||
objectNamePlural, | ||
viewId, | ||
}: { | ||
objectNamePlural: string; | ||
viewId: string; | ||
}) => { | ||
await findManyRecords(); | ||
|
||
const view = views.find((view: View) => view.id === viewId); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick: Redundant arg typing |
||
|
||
const objectMetadataItems = snapshot | ||
.getLoadable(objectMetadataItemsState) | ||
.getValue(); | ||
|
||
const objectMetadataItem = objectMetadataItems.find( | ||
(item) => item.namePlural === objectNamePlural, | ||
); | ||
|
||
if (!isDefined(objectMetadataItem) || !isDefined(view)) { | ||
return; | ||
} | ||
|
||
if (view.objectMetadataId !== objectMetadataItem.id) { | ||
return; | ||
} | ||
|
||
const lastVisitedViewPerObjectMetadataItem = snapshot | ||
.getLoadable(lastVisitedViewPerObjectMetadataItemState) | ||
.getValue(); | ||
|
||
const lastVisitedViewId = | ||
lastVisitedViewPerObjectMetadataItem?.[objectMetadataItem?.id]; | ||
|
||
if (isDefined(objectMetadataItem) && lastVisitedViewId !== viewId) { | ||
set(lastVisitedViewPerObjectMetadataItemState, { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nitpick: Could avoid making |
||
...lastVisitedViewPerObjectMetadataItem, | ||
[objectMetadataItem.id]: viewId, | ||
}); | ||
} | ||
}, | ||
[findManyRecords, views], | ||
); | ||
|
||
return { | ||
setLastVisitedViewForObjectMetadataNamePlural, | ||
}; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,11 @@ | ||
import { createComponentState } from '@/ui/utilities/state/component-state/utils/createComponentState'; | ||
import { createState } from 'twenty-ui'; | ||
import { localStorageEffect } from '~/utils/recoil-effects'; | ||
|
||
export const lastVisitedViewPerObjectMetadataItemState = | ||
createComponentState<Record<string, string> | null>({ | ||
key: 'lastVisitedViewPerObjectMetadataItemState', | ||
defaultValue: null, | ||
effects: [localStorageEffect()], | ||
}); | ||
export const lastVisitedViewPerObjectMetadataItemState = createState<Record< | ||
string, | ||
string | ||
> | null>({ | ||
key: 'lastVisitedViewPerObjectMetadataItemState', | ||
defaultValue: null, | ||
effects: [localStorageEffect()], | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: setLastVisitedViewForObjectMetadataNamePlural is called unconditionally on every effect run. Consider adding a condition to only update when values change.