From a76ea95e7e1bde3a0b04b34d4f88fcdca8e55fa8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=A9my=20Magrin?= Date: Tue, 22 Oct 2024 09:49:16 +0200 Subject: [PATCH] fix: make changes required by the review --- .../record-board/components/RecordBoard.tsx | 7 +- .../hooks/useRecordGroupActions.ts | 6 +- .../hooks/useRecordGroupReorder.ts | 14 +-- .../hooks/useRecordGroupVisibility.ts | 10 +- ...ecordGroupStates.ts => useRecordGroups.ts} | 6 +- .../RecordIndexOptionsDropdownContent.tsx | 30 +++--- .../ViewGroupsVisibilityDropdownSection.tsx | 101 ++++++++++++------ .../views/hooks/useSaveCurrentViewGroups.ts | 15 +-- .../isPersistingViewGroupsComponentState.ts | 9 -- ... mapRecordGroupDefinitionsToViewGroups.ts} | 2 +- .../utils/mapViewGroupsToGroupDefinitions.ts | 12 +-- 11 files changed, 112 insertions(+), 100 deletions(-) rename packages/twenty-front/src/modules/object-record/record-group/hooks/{useRecordGroupStates.ts => useRecordGroups.ts} (93%) delete mode 100644 packages/twenty-front/src/modules/views/states/isPersistingViewGroupsComponentState.ts rename packages/twenty-front/src/modules/views/utils/{mapGroupDefinitionsToViewGroups.ts => mapRecordGroupDefinitionsToViewGroups.ts} (91%) diff --git a/packages/twenty-front/src/modules/object-record/record-board/components/RecordBoard.tsx b/packages/twenty-front/src/modules/object-record/record-board/components/RecordBoard.tsx index 517c385ed..b08fe9e75 100644 --- a/packages/twenty-front/src/modules/object-record/record-board/components/RecordBoard.tsx +++ b/packages/twenty-front/src/modules/object-record/record-board/components/RecordBoard.tsx @@ -45,7 +45,7 @@ const StyledContainerContainer = styled.div` const StyledBoardContentContainer = styled.div` display: flex; flex-direction: column; - height: calc(100% - 48px);; + height: calc(100% - 48px); `; const RecordBoardScrollRestoreEffect = () => { @@ -69,11 +69,6 @@ export const RecordBoard = () => { const { resetRecordSelection, setRecordAsSelected } = useRecordBoardSelection(recordBoardId); - // const isPersistingViewGroups = useRecoilComponentValueV2( - // isPersistingViewGroupsComponentState, - // recordBoardId, - // ); - useListenClickOutsideByClassName({ classNames: ['record-board-card'], excludeClassNames: ['bottom-bar', 'action-menu-dropdown'], diff --git a/packages/twenty-front/src/modules/object-record/record-group/hooks/useRecordGroupActions.ts b/packages/twenty-front/src/modules/object-record/record-group/hooks/useRecordGroupActions.ts index 7a8a15ef3..d1c02958d 100644 --- a/packages/twenty-front/src/modules/object-record/record-group/hooks/useRecordGroupActions.ts +++ b/packages/twenty-front/src/modules/object-record/record-group/hooks/useRecordGroupActions.ts @@ -1,13 +1,13 @@ import { useObjectMetadataItem } from '@/object-metadata/hooks/useObjectMetadataItem'; import { getObjectSlug } from '@/object-metadata/utils/getObjectSlug'; import { RecordBoardColumnContext } from '@/object-record/record-board/record-board-column/contexts/RecordBoardColumnContext'; -import { useRecordGroupStates } from '@/object-record/record-group/hooks/useRecordGroupStates'; +import { useRecordGroups } from '@/object-record/record-group/hooks/useRecordGroups'; import { useRecordGroupVisibility } from '@/object-record/record-group/hooks/useRecordGroupVisibility'; import { RecordGroupAction } from '@/object-record/record-group/types/RecordGroupActions'; import { RecordIndexRootPropsContext } from '@/object-record/record-index/contexts/RecordIndexRootPropsContext'; import { navigationMemorizedUrlState } from '@/ui/navigation/states/navigationMemorizedUrlState'; import { useCallback, useContext, useMemo } from 'react'; -import { useNavigate, useLocation } from 'react-router-dom'; +import { useLocation, useNavigate } from 'react-router-dom'; import { useSetRecoilState } from 'recoil'; import { IconEyeOff, IconSettings } from 'twenty-ui'; @@ -27,7 +27,7 @@ export const useRecordGroupActions = () => { objectNameSingular, }); - const { viewGroupFieldMetadataItem } = useRecordGroupStates({ + const { viewGroupFieldMetadataItem } = useRecordGroups({ objectNameSingular, }); diff --git a/packages/twenty-front/src/modules/object-record/record-group/hooks/useRecordGroupReorder.ts b/packages/twenty-front/src/modules/object-record/record-group/hooks/useRecordGroupReorder.ts index d10bbf19c..aa3019615 100644 --- a/packages/twenty-front/src/modules/object-record/record-group/hooks/useRecordGroupReorder.ts +++ b/packages/twenty-front/src/modules/object-record/record-group/hooks/useRecordGroupReorder.ts @@ -1,13 +1,13 @@ import { OnDragEndResponder } from '@hello-pangea/dnd'; import { useCallback } from 'react'; -import { moveArrayItem } from '~/utils/array/moveArrayItem'; -import { isDeeplyEqual } from '~/utils/isDeeplyEqual'; -import { useSaveCurrentViewGroups } from '@/views/hooks/useSaveCurrentViewGroups'; -import { mapGroupDefinitionsToViewGroups } from '@/views/utils/mapGroupDefinitionsToViewGroups'; -import { useRecordGroupStates } from '@/object-record/record-group/hooks/useRecordGroupStates'; +import { useRecordGroups } from '@/object-record/record-group/hooks/useRecordGroups'; import { recordGroupDefinitionState } from '@/object-record/record-group/states/recordGroupDefinitionState'; import { useSetRecoilComponentStateV2 } from '@/ui/utilities/state/component-state/hooks/useSetRecoilComponentStateV2'; +import { useSaveCurrentViewGroups } from '@/views/hooks/useSaveCurrentViewGroups'; +import { mapRecordGroupDefinitionsToViewGroups } from '@/views/utils/mapRecordGroupDefinitionsToViewGroups'; +import { moveArrayItem } from '~/utils/array/moveArrayItem'; +import { isDeeplyEqual } from '~/utils/isDeeplyEqual'; type UseRecordGroupHandlersParams = { objectNameSingular: string; @@ -22,7 +22,7 @@ export const useRecordGroupReorder = ({ recordGroupDefinitionState, ); - const { visibleRecordGroups } = useRecordGroupStates({ + const { visibleRecordGroups } = useRecordGroups({ objectNameSingular, }); @@ -47,7 +47,7 @@ export const useRecordGroupReorder = ({ ); setRecordIndexGroupDefinitions(updatedGroups); - saveViewGroups(mapGroupDefinitionsToViewGroups(updatedGroups)); + saveViewGroups(mapRecordGroupDefinitionsToViewGroups(updatedGroups)); }, [saveViewGroups, setRecordIndexGroupDefinitions, visibleRecordGroups], ); diff --git a/packages/twenty-front/src/modules/object-record/record-group/hooks/useRecordGroupVisibility.ts b/packages/twenty-front/src/modules/object-record/record-group/hooks/useRecordGroupVisibility.ts index 168709b9f..05708e543 100644 --- a/packages/twenty-front/src/modules/object-record/record-group/hooks/useRecordGroupVisibility.ts +++ b/packages/twenty-front/src/modules/object-record/record-group/hooks/useRecordGroupVisibility.ts @@ -1,10 +1,10 @@ import { useCallback } from 'react'; -import { useSaveCurrentViewGroups } from '@/views/hooks/useSaveCurrentViewGroups'; -import { mapGroupDefinitionsToViewGroups } from '@/views/utils/mapGroupDefinitionsToViewGroups'; +import { recordGroupDefinitionState } from '@/object-record/record-group/states/recordGroupDefinitionState'; import { RecordGroupDefinition } from '@/object-record/record-group/types/RecordGroupDefinition'; import { useRecoilComponentStateV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentStateV2'; -import { recordGroupDefinitionState } from '@/object-record/record-group/states/recordGroupDefinitionState'; +import { useSaveCurrentViewGroups } from '@/views/hooks/useSaveCurrentViewGroups'; +import { mapRecordGroupDefinitionsToViewGroups } from '@/views/utils/mapRecordGroupDefinitionsToViewGroups'; type UseRecordGroupVisibilityParams = { viewBarId: string; @@ -32,7 +32,9 @@ export const useRecordGroupVisibility = ({ setRecordIndexGroupDefinitions(updatedGroupsDefinitions); - saveViewGroups(mapGroupDefinitionsToViewGroups(updatedGroupsDefinitions)); + saveViewGroups( + mapRecordGroupDefinitionsToViewGroups(updatedGroupsDefinitions), + ); }, [ recordIndexGroupDefinitions, diff --git a/packages/twenty-front/src/modules/object-record/record-group/hooks/useRecordGroupStates.ts b/packages/twenty-front/src/modules/object-record/record-group/hooks/useRecordGroups.ts similarity index 93% rename from packages/twenty-front/src/modules/object-record/record-group/hooks/useRecordGroupStates.ts rename to packages/twenty-front/src/modules/object-record/record-group/hooks/useRecordGroups.ts index cb945dad5..289647c74 100644 --- a/packages/twenty-front/src/modules/object-record/record-group/hooks/useRecordGroupStates.ts +++ b/packages/twenty-front/src/modules/object-record/record-group/hooks/useRecordGroups.ts @@ -4,13 +4,13 @@ import { useObjectMetadataItem } from '@/object-metadata/hooks/useObjectMetadata import { recordGroupDefinitionState } from '@/object-record/record-group/states/recordGroupDefinitionState'; import { useRecoilComponentValueV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValueV2'; -type UseRecordGroupStatesParams = { +type UseRecordGroupsParams = { objectNameSingular: string; }; -export const useRecordGroupStates = ({ +export const useRecordGroups = ({ objectNameSingular, -}: UseRecordGroupStatesParams) => { +}: UseRecordGroupsParams) => { const recordIndexGroupDefinitions = useRecoilComponentValueV2( recordGroupDefinitionState, ); diff --git a/packages/twenty-front/src/modules/object-record/record-index/options/components/RecordIndexOptionsDropdownContent.tsx b/packages/twenty-front/src/modules/object-record/record-index/options/components/RecordIndexOptionsDropdownContent.tsx index 6a243b9ba..26cd2c86c 100644 --- a/packages/twenty-front/src/modules/object-record/record-index/options/components/RecordIndexOptionsDropdownContent.tsx +++ b/packages/twenty-front/src/modules/object-record/record-index/options/components/RecordIndexOptionsDropdownContent.tsx @@ -21,6 +21,9 @@ import { useExportRecordData, } from '@/action-menu/hooks/useExportRecordData'; import { ObjectMetadataItem } from '@/object-metadata/types/ObjectMetadataItem'; +import { useRecordGroupReorder } from '@/object-record/record-group/hooks/useRecordGroupReorder'; +import { useRecordGroups } from '@/object-record/record-group/hooks/useRecordGroups'; +import { useRecordGroupVisibility } from '@/object-record/record-group/hooks/useRecordGroupVisibility'; import { useRecordIndexOptionsForBoard } from '@/object-record/record-index/options/hooks/useRecordIndexOptionsForBoard'; import { useRecordIndexOptionsForTable } from '@/object-record/record-index/options/hooks/useRecordIndexOptionsForTable'; import { TableOptionsHotkeyScope } from '@/object-record/record-table/types/TableOptionsHotkeyScope'; @@ -38,14 +41,11 @@ import { MenuItemToggle } from '@/ui/navigation/menu-item/components/MenuItemTog import { navigationMemorizedUrlState } from '@/ui/navigation/states/navigationMemorizedUrlState'; import { useScopedHotkeys } from '@/ui/utilities/hotkey/hooks/useScopedHotkeys'; import { ViewFieldsVisibilityDropdownSection } from '@/views/components/ViewFieldsVisibilityDropdownSection'; +import { ViewGroupsVisibilityDropdownSection } from '@/views/components/ViewGroupsVisibilityDropdownSection'; import { useGetCurrentView } from '@/views/hooks/useGetCurrentView'; import { ViewType } from '@/views/types/ViewType'; import { useLocation } from 'react-router-dom'; import { useSetRecoilState } from 'recoil'; -import { ViewGroupsVisibilityDropdownSection } from '@/views/components/ViewGroupsVisibilityDropdownSection'; -import { useRecordGroupStates } from '@/object-record/record-group/hooks/useRecordGroupStates'; -import { useRecordGroupVisibility } from '@/object-record/record-group/hooks/useRecordGroupVisibility'; -import { useRecordGroupReorder } from '@/object-record/record-group/hooks/useRecordGroupReorder'; type RecordIndexOptionsMenu = | 'viewGroups' @@ -126,7 +126,7 @@ export const RecordIndexOptionsDropdownContent = ({ hiddenRecordGroups, visibleRecordGroups, viewGroupFieldMetadataItem, - } = useRecordGroupStates({ + } = useRecordGroups({ objectNameSingular: objectMetadataItem.nameSingular, }); const { handleVisibilityChange: handleRecordGroupVisibilityChange } = @@ -176,16 +176,9 @@ export const RecordIndexOptionsDropdownContent = ({ navigationMemorizedUrlState, ); - const viewGroupMenuItem = + const isViewGroupMenuItemVisible = viewGroupFieldMetadataItem && - (visibleRecordGroups.length > 0 || hiddenRecordGroups.length > 0) ? ( - handleSelectMenu('viewGroups')} - LeftIcon={getIcon(currentViewWithCombinedFiltersAndSorts?.icon)} - text={viewGroupFieldMetadataItem.label} - hasSubMenu - /> - ) : null; + (visibleRecordGroups.length > 0 || hiddenRecordGroups.length > 0); useEffect(() => { if (currentMenu === 'hiddenViewGroups' && hiddenRecordGroups.length === 0) { @@ -197,7 +190,14 @@ export const RecordIndexOptionsDropdownContent = ({ <> {!currentMenu && ( - {viewGroupMenuItem} + {isViewGroupMenuItemVisible && ( + handleSelectMenu('viewGroups')} + LeftIcon={getIcon(currentViewWithCombinedFiltersAndSorts?.icon)} + text={viewGroupFieldMetadataItem.label} + hasSubMenu + /> + )} handleSelectMenu('fields')} LeftIcon={IconTag} diff --git a/packages/twenty-front/src/modules/views/components/ViewGroupsVisibilityDropdownSection.tsx b/packages/twenty-front/src/modules/views/components/ViewGroupsVisibilityDropdownSection.tsx index 9827ff02e..4291cd4af 100644 --- a/packages/twenty-front/src/modules/views/components/ViewGroupsVisibilityDropdownSection.tsx +++ b/packages/twenty-front/src/modules/views/components/ViewGroupsVisibilityDropdownSection.tsx @@ -6,16 +6,16 @@ import { import { useRef } from 'react'; import { IconEye, IconEyeOff, Tag } from 'twenty-ui'; -import { DropdownMenuItemsContainer } from '@/ui/layout/dropdown/components/DropdownMenuItemsContainer'; -import { StyledDropdownMenuSubheader } from '@/ui/layout/dropdown/components/StyledDropdownMenuSubheader'; -import { MenuItemDraggable } from '@/ui/navigation/menu-item/components/MenuItemDraggable'; import { RecordGroupDefinition, RecordGroupDefinitionType, } from '@/object-record/record-group/types/RecordGroupDefinition'; -import { isDefined } from '~/utils/isDefined'; import { DraggableItem } from '@/ui/layout/draggable-list/components/DraggableItem'; import { DraggableList } from '@/ui/layout/draggable-list/components/DraggableList'; +import { DropdownMenuItemsContainer } from '@/ui/layout/dropdown/components/DropdownMenuItemsContainer'; +import { StyledDropdownMenuSubheader } from '@/ui/layout/dropdown/components/StyledDropdownMenuSubheader'; +import { MenuItemDraggable } from '@/ui/navigation/menu-item/components/MenuItemDraggable'; +import { isDefined } from '~/utils/isDefined'; type ViewGroupsVisibilityDropdownSectionProps = { viewGroups: RecordGroupDefinition[]; @@ -53,31 +53,6 @@ export const ViewGroupsVisibilityDropdownSection = ({ const ref = useRef(null); - const renderItem = ( - viewGroup: RecordGroupDefinition, - viewGroupIndex: number, - ) => { - const isNoValueType = viewGroup.type === RecordGroupDefinitionType.NoValue; - - return ( - - } - iconButtons={getIconButtons(viewGroupIndex, viewGroup)} - accent={showDragGrip ? 'placeholder' : 'default'} - showGrip={showDragGrip} - isDragDisabled={!isDraggable} - /> - ); - }; - return (
{showSubheader && ( @@ -87,9 +62,35 @@ export const ViewGroupsVisibilityDropdownSection = ({ {!!viewGroups.length && ( <> {!isDraggable ? ( - viewGroups.map((viewGroup, viewGroupIndex) => - renderItem(viewGroup, viewGroupIndex), - ) + viewGroups.map((viewGroup, viewGroupIndex) => ( + + } + iconButtons={getIconButtons(viewGroupIndex, viewGroup)} + accent={showDragGrip ? 'placeholder' : 'default'} + showGrip={showDragGrip} + isDragDisabled={!isDraggable} + /> + )) ) : ( + } + iconButtons={getIconButtons( + viewGroupIndex, + viewGroup, + )} + accent={showDragGrip ? 'placeholder' : 'default'} + showGrip={showDragGrip} + isDragDisabled={!isDraggable} + /> + } /> ))} diff --git a/packages/twenty-front/src/modules/views/hooks/useSaveCurrentViewGroups.ts b/packages/twenty-front/src/modules/views/hooks/useSaveCurrentViewGroups.ts index 2c1208309..33c164b98 100644 --- a/packages/twenty-front/src/modules/views/hooks/useSaveCurrentViewGroups.ts +++ b/packages/twenty-front/src/modules/views/hooks/useSaveCurrentViewGroups.ts @@ -1,14 +1,13 @@ import { useRecoilCallback } from 'recoil'; import { useRecoilComponentCallbackStateV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentCallbackStateV2'; +import { usePersistViewGroupRecords } from '@/views/hooks/internal/usePersistViewGroupRecords'; import { useGetViewFromCache } from '@/views/hooks/useGetViewFromCache'; import { currentViewIdComponentState } from '@/views/states/currentViewIdComponentState'; +import { ViewGroup } from '@/views/types/ViewGroup'; import { isDeeplyEqual } from '~/utils/isDeeplyEqual'; import { isDefined } from '~/utils/isDefined'; import { isUndefinedOrNull } from '~/utils/isUndefinedOrNull'; -import { usePersistViewGroupRecords } from '@/views/hooks/internal/usePersistViewGroupRecords'; -import { ViewGroup } from '@/views/types/ViewGroup'; -import { isPersistingViewGroupsComponentState } from '@/views/states/isPersistingViewGroupsComponentState'; export const useSaveCurrentViewGroups = (viewBarComponentId?: string) => { const { createViewGroupRecords, updateViewGroupRecords } = @@ -21,11 +20,6 @@ export const useSaveCurrentViewGroups = (viewBarComponentId?: string) => { viewBarComponentId, ); - const isPersistingViewGroupsCallbackState = useRecoilComponentCallbackStateV2( - isPersistingViewGroupsComponentState, - viewBarComponentId, - ); - const saveViewGroups = useRecoilCallback( ({ set, snapshot }) => async (viewGroupsToSave: ViewGroup[]) => { @@ -37,8 +31,6 @@ export const useSaveCurrentViewGroups = (viewBarComponentId?: string) => { return; } - set(isPersistingViewGroupsCallbackState, true); - const view = await getViewFromCache(currentViewId); if (isUndefinedOrNull(view)) { @@ -89,14 +81,11 @@ export const useSaveCurrentViewGroups = (viewBarComponentId?: string) => { createViewGroupRecords(viewGroupsToCreate, view), updateViewGroupRecords(viewGroupsToUpdate), ]); - - set(isPersistingViewGroupsCallbackState, false); }, [ createViewGroupRecords, currentViewIdCallbackState, getViewFromCache, - isPersistingViewGroupsCallbackState, updateViewGroupRecords, ], ); diff --git a/packages/twenty-front/src/modules/views/states/isPersistingViewGroupsComponentState.ts b/packages/twenty-front/src/modules/views/states/isPersistingViewGroupsComponentState.ts deleted file mode 100644 index ac2096f4f..000000000 --- a/packages/twenty-front/src/modules/views/states/isPersistingViewGroupsComponentState.ts +++ /dev/null @@ -1,9 +0,0 @@ -import { createComponentStateV2 } from '@/ui/utilities/state/component-state/utils/createComponentStateV2'; -import { ViewComponentInstanceContext } from '@/views/states/contexts/ViewComponentInstanceContext'; - -export const isPersistingViewGroupsComponentState = - createComponentStateV2({ - key: 'isPersistingViewGroupsComponentState', - defaultValue: false, - componentInstanceContext: ViewComponentInstanceContext, - }); diff --git a/packages/twenty-front/src/modules/views/utils/mapGroupDefinitionsToViewGroups.ts b/packages/twenty-front/src/modules/views/utils/mapRecordGroupDefinitionsToViewGroups.ts similarity index 91% rename from packages/twenty-front/src/modules/views/utils/mapGroupDefinitionsToViewGroups.ts rename to packages/twenty-front/src/modules/views/utils/mapRecordGroupDefinitionsToViewGroups.ts index 932711717..2c5920a0f 100644 --- a/packages/twenty-front/src/modules/views/utils/mapGroupDefinitionsToViewGroups.ts +++ b/packages/twenty-front/src/modules/views/utils/mapRecordGroupDefinitionsToViewGroups.ts @@ -1,7 +1,7 @@ import { RecordGroupDefinition } from '@/object-record/record-group/types/RecordGroupDefinition'; import { ViewGroup } from '@/views/types/ViewGroup'; -export const mapGroupDefinitionsToViewGroups = ( +export const mapRecordGroupDefinitionsToViewGroups = ( groupDefinitions: RecordGroupDefinition[], ): ViewGroup[] => { return groupDefinitions.map( diff --git a/packages/twenty-front/src/modules/views/utils/mapViewGroupsToGroupDefinitions.ts b/packages/twenty-front/src/modules/views/utils/mapViewGroupsToGroupDefinitions.ts index 3167d7fb1..d814c8852 100644 --- a/packages/twenty-front/src/modules/views/utils/mapViewGroupsToGroupDefinitions.ts +++ b/packages/twenty-front/src/modules/views/utils/mapViewGroupsToGroupDefinitions.ts @@ -1,12 +1,12 @@ import { isDefined } from '~/utils/isDefined'; -import { ViewGroup } from '@/views/types/ViewGroup'; +import { ObjectMetadataItem } from '@/object-metadata/types/ObjectMetadataItem'; import { RecordGroupDefinition, RecordGroupDefinitionNoValue, RecordGroupDefinitionType, } from '@/object-record/record-group/types/RecordGroupDefinition'; -import { ObjectMetadataItem } from '@/object-metadata/types/ObjectMetadataItem'; +import { ViewGroup } from '@/views/types/ViewGroup'; import { FieldMetadataType } from '~/generated-metadata/graphql'; export const mapViewGroupsToGroupDefinitions = ({ @@ -35,7 +35,7 @@ export const mapViewGroupsToGroupDefinitions = ({ `Select Field ${objectMetadataItem.nameSingular} has no options`, ); } - const groupDefinitionsFromViewGroups = viewGroups + const recordGroupDefinitionsFromViewGroups = viewGroups .map((viewGroup) => { // It's actually check right above // eslint-disable-next-line @typescript-eslint/no-non-null-assertion @@ -66,14 +66,14 @@ export const mapViewGroupsToGroupDefinitions = ({ type: RecordGroupDefinitionType.NoValue, value: null, position: - groupDefinitionsFromViewGroups + recordGroupDefinitionsFromViewGroups .map((option) => option.position) .reduce((a, b) => Math.max(a, b), 0) + 1, isVisible: true, } satisfies RecordGroupDefinitionNoValue; - return [...groupDefinitionsFromViewGroups, noValueColumn]; + return [...recordGroupDefinitionsFromViewGroups, noValueColumn]; } - return groupDefinitionsFromViewGroups; + return recordGroupDefinitionsFromViewGroups; };