From dfb966d47e0b4d2f3786c19561287d3144f0dbad Mon Sep 17 00:00:00 2001 From: Khuddite <62555977+khuddite@users.noreply.github.com> Date: Tue, 26 Nov 2024 11:47:42 -0500 Subject: [PATCH] Treat no value view group as normal & enable hide/dnd for no value (#8613) Fixes #8591 1. Summary We disabled hide/dnd(drag-and-drop) options for `No value` view group intentionally in the first implementation. We want to change it to behave like normal view groups, so enable hide/dnd for `No value` view group as well. 2. Solution I have removed the code that filters the `No value` group out of view groups, so `No value` is stored in the same array as other view groups. I have removed the `No value` flag check for `Hide` button on the hamburger menu of the Kanban header. I had to update the code in `packages/twenty-front/src/modules/views/utils/mapViewGroupsToRecordGroupDefinitions.ts` because it was ignoring the visibility flag of the `No value` view group and set it always to true. Also, it was always putting the `No value` group last ignoring the previous position. >**_I am not 100% confident in the changes I made in `packages/twenty-front/src/modules/views/utils/mapViewGroupsToRecordGroupDefinitions.ts`. I'd like to have a review from someone more familiar with that part._** 3. Recording https://github.com/user-attachments/assets/e135e22e-6e3a-4f94-a898-aafc03bba060 --- .../RecordGroupMenuItemDraggable.tsx | 5 +- .../RecordGroupsVisibilityDropdownSection.tsx | 54 ++++++------------- .../hooks/useRecordGroupActions.ts | 21 ++++---- .../mapViewGroupsToRecordGroupDefinitions.ts | 31 +++++------ 4 files changed, 40 insertions(+), 71 deletions(-) diff --git a/packages/twenty-front/src/modules/object-record/record-group/components/RecordGroupMenuItemDraggable.tsx b/packages/twenty-front/src/modules/object-record/record-group/components/RecordGroupMenuItemDraggable.tsx index 8aa81fe1b..70bd645af 100644 --- a/packages/twenty-front/src/modules/object-record/record-group/components/RecordGroupMenuItemDraggable.tsx +++ b/packages/twenty-front/src/modules/object-record/record-group/components/RecordGroupMenuItemDraggable.tsx @@ -56,10 +56,9 @@ export const RecordGroupMenuItemDraggable = ({ /> } accent={isNoValue || showDragGrip ? 'placeholder' : 'default'} - iconButtons={!isNoValue ? getIconButtons(recordGroup) : undefined} + iconButtons={getIconButtons(recordGroup)} showGrip={isNoValue ? true : showDragGrip} - isDragDisabled={isNoValue ? true : !isDraggable} - isHoverDisabled={isNoValue} + isDragDisabled={!isDraggable} /> ); }; diff --git a/packages/twenty-front/src/modules/object-record/record-group/components/RecordGroupsVisibilityDropdownSection.tsx b/packages/twenty-front/src/modules/object-record/record-group/components/RecordGroupsVisibilityDropdownSection.tsx index 4b742a50c..e43afd1ee 100644 --- a/packages/twenty-front/src/modules/object-record/record-group/components/RecordGroupsVisibilityDropdownSection.tsx +++ b/packages/twenty-front/src/modules/object-record/record-group/components/RecordGroupsVisibilityDropdownSection.tsx @@ -6,10 +6,7 @@ import { import { useRef } from 'react'; import { RecordGroupMenuItemDraggable } from '@/object-record/record-group/components/RecordGroupMenuItemDraggable'; -import { - RecordGroupDefinition, - RecordGroupDefinitionType, -} from '@/object-record/record-group/types/RecordGroupDefinition'; +import { RecordGroupDefinition } from '@/object-record/record-group/types/RecordGroupDefinition'; 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'; @@ -38,15 +35,6 @@ export const RecordGroupsVisibilityDropdownSection = ({ onDragEnd?.(result, provided); }; - const noValueRecordGroups = - recordGroups.filter( - (recordGroup) => recordGroup.type === RecordGroupDefinitionType.NoValue, - ) ?? []; - - const recordGroupsWithoutNoValueGroups = recordGroups.filter( - (recordGroup) => recordGroup.type !== RecordGroupDefinitionType.NoValue, - ); - const ref = useRef(null); return ( @@ -58,7 +46,7 @@ export const RecordGroupsVisibilityDropdownSection = ({ {!!recordGroups.length && ( <> {!isDraggable ? ( - recordGroupsWithoutNoValueGroups.map((recordGroup) => ( + recordGroups.map((recordGroup) => ( - {recordGroupsWithoutNoValueGroups.map( - (recordGroup, index) => ( - - } - /> - ), - )} + {recordGroups.map((recordGroup, index) => ( + + } + /> + ))} } /> )} - {noValueRecordGroups.map((recordGroup) => ( - - ))} )} 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 42ad8b3f5..b99af28b6 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 @@ -5,7 +5,6 @@ import { RecordBoardColumnContext } from '@/object-record/record-board/record-bo 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 { RecordGroupDefinitionType } from '@/object-record/record-group/types/RecordGroupDefinition'; import { RecordIndexRootPropsContext } from '@/object-record/record-index/contexts/RecordIndexRootPropsContext'; import { navigationMemorizedUrlState } from '@/ui/navigation/states/navigationMemorizedUrlState'; import { ViewType } from '@/views/types/ViewType'; @@ -81,17 +80,15 @@ export const useRecordGroupActions = ({ navigateToSelectSettings(); }, }, - recordGroupDefinition.type !== RecordGroupDefinitionType.NoValue - ? { - id: 'hide', - label: 'Hide', - icon: IconEyeOff, - position: 1, - callback: () => { - handleRecordGroupVisibilityChange(recordGroupDefinition); - }, - } - : undefined, + { + id: 'hide', + label: 'Hide', + icon: IconEyeOff, + position: 1, + callback: () => { + handleRecordGroupVisibilityChange(recordGroupDefinition); + }, + }, ].filter(isDefined), [ handleRecordGroupVisibilityChange, diff --git a/packages/twenty-front/src/modules/views/utils/mapViewGroupsToRecordGroupDefinitions.ts b/packages/twenty-front/src/modules/views/utils/mapViewGroupsToRecordGroupDefinitions.ts index 3767abe6a..7c7aba97a 100644 --- a/packages/twenty-front/src/modules/views/utils/mapViewGroupsToRecordGroupDefinitions.ts +++ b/packages/twenty-front/src/modules/views/utils/mapViewGroupsToRecordGroupDefinitions.ts @@ -41,7 +41,18 @@ export const mapViewGroupsToRecordGroupDefinitions = ({ (option) => option.value === viewGroup.fieldValue, ); - if (!selectedOption) return null; + if (!selectedOption) { + return { + id: 'no-value', + title: 'No Value', + type: RecordGroupDefinitionType.NoValue, + value: null, + position: viewGroup.position, + isVisible: viewGroup.isVisible, + fieldMetadataId: selectFieldMetadataItem.id, + color: 'transparent', + } satisfies RecordGroupDefinition; + } return { id: viewGroup.id, @@ -57,23 +68,5 @@ export const mapViewGroupsToRecordGroupDefinitions = ({ .filter(isDefined) .sort((a, b) => a.position - b.position); - if (selectFieldMetadataItem.isNullable === true) { - const noValueColumn = { - id: 'no-value', - title: 'No Value', - type: RecordGroupDefinitionType.NoValue, - value: null, - position: - recordGroupDefinitionsFromViewGroups - .map((option) => option.position) - .reduce((a, b) => Math.max(a, b), 0) + 1, - isVisible: true, - fieldMetadataId: selectFieldMetadataItem.id, - color: 'transparent', - } satisfies RecordGroupDefinition; - - return [...recordGroupDefinitionsFromViewGroups, noValueColumn]; - } - return recordGroupDefinitionsFromViewGroups; };