From 333d7081ef91c2ce3e1c0a3403d9f44b48aaca2c Mon Sep 17 00:00:00 2001 From: Lucas Bordeau Date: Thu, 12 Jun 2025 12:22:26 +0200 Subject: [PATCH] Refactored and simplified DropdownMenuItemsContainer height management (#12547) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This PR refactors the `DropdownMenuItemsContainer` component and simplifies its inner parts, which have been modified over months for different needs without taking the time to have a global approach. It should however be noted that due to the recent refactor of the `DropdownContent`, it is now much easier to refactor `DropdownMenuItemsContainer`, mainly because of the width management being nicely handled by `DropdownContent` now. Fixes https://github.com/twentyhq/twenty/issues/11766 # Changes The `width` props of `DropdownMenuItemsContainer` and its usage in calling components have been removed. The multiple ternaries inside `DropdownMenuItemsContainer` have been reduced to one ternary on `scrollable` props. The `ScrollWrapper` usage has been removed from `DropdownMenuItemsContainer`, because the only thing we need is to have a simple `overflow-y: scroll;` CSS property. Why ? Because it was previously relevant to have a `ScrollWrapper`, when we were using an external library, but now that `ScrollWrapper` is a simple `div` with overflowing, which only benefit is to expose a hook to imperatively toggle this overflowing behavior from outside (mainly useful for table fixed row and column), and that we don’t need this for `DropdownMenuItemsContainer`, then it follows that we just need a simple overflowing `div` container, which simplifies everything and boils down our `DropdownMenuItemsContainer` to a straightforward and standard CSS stack. We remove the temporary `scrollWrapperHeightAuto` props that was used to fix a bug in a previous PR, we also rollback `ScrollWrapper` to its previous state with `width: 100%` and `height: 100%` and removed `heightAuto` props. The `hasMaxHeight` props is kept, but the `168` pixels value is extracted in a constant. # QA Component | Comment -- | -- CommandMenuActionDropdown | Reported bug https://github.com/twentyhq/twenty/issues/12541 RecordIndexActionMenuDropdown |   AttachmentDropdown | Cannot test because cannot add a file (currently broken, maybe because of permissions ?) CommandMenuContextChipGroups |   FavoriteFolderNavigationDrawerItemDropdown |   FavoriteFolderPicker |   FavoriteFolderPickerFooter |   AdvancedFilterAddFilterRuleSelect |   AdvancedFilterFieldSelectMenu |   AdvancedFilterRecordFilterGroupOptionsDropdown |   AdvancedFilterRecordFilterOperandSelect |   AdvancedFilterRecordFilterOptionsDropdown |   AdvancedFilterSubFieldSelectMenu |   ObjectFilterDropdownBooleanSelect |   ObjectFilterDropdownCountrySelect |   ObjectFilterDropdownCurrencySelect |   ObjectFilterDropdownNumberInput |   ObjectFilterDropdownOptionSelect | Fixed “No result” case ObjectFilterDropdownRecordRemoveFilterMenuItem | Removed because unused ObjectFilterDropdownTextInput |   ObjectOptionsDropdownFieldsContent | Spotted bug with icon eye https://github.com/twentyhq/twenty/issues/12545 ObjectOptionsDropdownHiddenFieldsContent | Spotted bug with icon eye https://github.com/twentyhq/twenty/issues/12545 ObjectOptionsDropdownLayoutContent | Refactored DropdownMenuItemsContainer usage with DropdownMenuSeparator, spotted bug switch view type https://github.com/twentyhq/twenty/issues/12546 ObjectOptionsDropdownMenuContent | Refactored DropdownMenuItemsContainer usage with DropdownMenuSeparator ObjectOptionsDropdownLayoutOpenInContent |   ObjectOptionsDropdownMenuViewName |   ObjectOptionsDropdownRecordGroupFieldsContent |   ObjectOptionsDropdownRecordGroupSortContent |   ObjectSortDropdownButton |   RecordBoardColumnDropdownMenu |   RecordBoardColumnDropdownMenu |   RecordBoardColumnHeaderAggregateDropdownFieldsContent |   RecordBoardColumnHeaderAggregateDropdownMenuContent |   RecordBoardColumnHeaderAggregateDropdownOptionsContent |   MultiItemFieldInput | Added hasMaxHeight on list of items MultiItemFieldMenuItem |   RecordGroupsVisibilityDropdownSection |   MultipleRecordPicker |   MultipleRecordPickerMenuItems |   SingleRecordPickerMenuItems |   SingleRecordPickerMenuItemsWithSearch |   RecordDetailRelationRecordsListItem |   RecordTableColumnAggregateFooterDropdownSubmenuContent |   RecordTableColumnAggregateFooterMenuContent |   RecordTableHeaderPlusButtonContent |   RecordTableHeaderPlusButtonContent |   MultipleSelectDropdown |   SettingsAccountsRowDropdownMenu |   ConfigVariableDatabaseInput |   ConfigVariableOptionsDropdownContent |   SettingsDataModelNewFieldBreadcrumbDropDown |   SettingsDataModelFieldSelectFormOptionRow |   SettingsObjectFieldActiveActionDropdown |   SettingsObjectFieldInactiveActionDropdown |   SettingsObjectInactiveMenuDropDown |   SettingsIntegrationDatabaseConnectionSummaryCard |   SettingsRoleAssignmentWorkspaceMemberPickerDropdown |   SettingsRolePermissionsObjectLevelObjectPickerDropdownContent | Cannot test SettingsSecurityApprovedAccessDomainRowDropdownMenu | Cannot test SettingsSecuritySSORowDropdownMenu | Cannot test SettingsServerlessFunctionTabEnvironmentVariableTableRow | Cannot test MatchColumnSelectFieldSelectDropdownContent |   MatchColumnSelectSubFieldSelectDropdownContent |   SubMatchingSelectInput |   SupportDropdown |   IconPicker |   Select |   SelectInput |   CurrencyPickerDropdownSelect |   PhoneCountryPickerDropdownSelect |   CustomSlashMenu |   TabListDropdown | Cannot test MultiWorkspaceDropdownDefaultComponents | Removed unnecessary StyledDropdownMenuItemsContainer MultiWorkspaceDropdownThemesComponents |   MultiWorkspaceDropdownWorkspacesListComponents |   UpdateViewButtonGroup |   ViewBarFilterDropdownFieldSelectMenu |   ViewFieldsVisibilityDropdownSection |   ViewPickerContentCreateMode |   ViewPickerContentEditMode |   ViewPickerListContent | Add hasMaxHeight to limit the height of view list ViewPickerOptionDropdown |   WorkflowEditTriggerDatabaseEventForm |   WorkflowVariablesDropdownFieldItems |   WorkflowVariablesDropdownObjectItems |   WorkflowVariablesDropdownWorkflowStepItems |   --------- Co-authored-by: Charles Bochet --- .../AdvancedFilterFieldSelectMenu.tsx | 4 +- .../ObjectFilterDropdownBooleanSelect.tsx | 2 +- .../ObjectFilterDropdownCurrencySelect.tsx | 2 +- .../ObjectFilterDropdownNumberInput.tsx | 2 +- .../ObjectFilterDropdownOptionSelect.tsx | 31 +++--- ...lterDropdownRecordRemoveFilterMenuItem.tsx | 26 ----- .../ObjectFilterDropdownTextInput.tsx | 2 +- .../ObjectOptionsDropdownLayoutContent.tsx | 20 ++-- .../ObjectOptionsDropdownMenuContent.tsx | 4 +- .../components/ObjectSortDropdownButton.tsx | 4 +- .../input/components/MultiItemFieldInput.tsx | 2 +- .../components/MultipleSelectDropdown.tsx | 2 +- .../components/DropdownMenuItemsContainer.tsx | 99 ++++++------------- .../DropdownMenuItemsContainerMaxHeight.ts | 1 + ...ultiWorkspaceDropdownDefaultComponents.tsx | 9 +- .../scroll/components/ScrollWrapper.tsx | 7 +- .../ViewBarFilterDropdownFieldSelectMenu.tsx | 5 +- .../components/ViewPickerListContent.tsx | 2 +- 18 files changed, 81 insertions(+), 143 deletions(-) delete mode 100644 packages/twenty-front/src/modules/object-record/object-filter-dropdown/components/ObjectFilterDropdownRecordRemoveFilterMenuItem.tsx create mode 100644 packages/twenty-front/src/modules/ui/layout/dropdown/constants/DropdownMenuItemsContainerMaxHeight.ts diff --git a/packages/twenty-front/src/modules/object-record/advanced-filter/components/AdvancedFilterFieldSelectMenu.tsx b/packages/twenty-front/src/modules/object-record/advanced-filter/components/AdvancedFilterFieldSelectMenu.tsx index 603f44d1b..48ac76efb 100644 --- a/packages/twenty-front/src/modules/object-record/advanced-filter/components/AdvancedFilterFieldSelectMenu.tsx +++ b/packages/twenty-front/src/modules/object-record/advanced-filter/components/AdvancedFilterFieldSelectMenu.tsx @@ -150,7 +150,7 @@ export const AdvancedFilterFieldSelectMenu = ({ {shouldShowVisibleFields && ( <> - + {visibleColumnsFieldMetadataItems.map( (visibleFieldMetadataItem, index) => ( - + {hiddenColumnsFieldMetadataItems.map( (hiddenFieldMetadataItem, index) => ( { selectableItemIdArray={options.map((option) => option.toString())} hotkeyScope={SingleRecordPickerHotkeyScope.SingleRecordPicker} > - + {options.map((option) => ( { }} /> - + {filteredSelectedItems?.map((item) => { return ( { }; return ( - + { hotkeyScope={SingleRecordPickerHotkeyScope.SingleRecordPicker} > - {optionsInDropdown?.map((option) => ( - - handleMultipleOptionSelectChange(option, selected) - } - text={option.label} - color={option.color} - className="" - /> - ))} + {showNoResult ? ( + + ) : ( + optionsInDropdown?.map((option) => ( + + handleMultipleOptionSelectChange(option, selected) + } + text={option.label} + color={option.color} + className="" + /> + )) + )} - {showNoResult && } ); }; diff --git a/packages/twenty-front/src/modules/object-record/object-filter-dropdown/components/ObjectFilterDropdownRecordRemoveFilterMenuItem.tsx b/packages/twenty-front/src/modules/object-record/object-filter-dropdown/components/ObjectFilterDropdownRecordRemoveFilterMenuItem.tsx deleted file mode 100644 index ba896562e..000000000 --- a/packages/twenty-front/src/modules/object-record/object-filter-dropdown/components/ObjectFilterDropdownRecordRemoveFilterMenuItem.tsx +++ /dev/null @@ -1,26 +0,0 @@ -import { useEmptyRecordFilter } from '@/object-record/object-filter-dropdown/hooks/useEmptyRecordFilter'; -import { DropdownMenuItemsContainer } from '@/ui/layout/dropdown/components/DropdownMenuItemsContainer'; -import { useDropdown } from '@/ui/layout/dropdown/hooks/useDropdown'; -import { IconFilterOff } from 'twenty-ui/display'; -import { MenuItem } from 'twenty-ui/navigation'; - -export const ObjectFilterDropdownRecordRemoveFilterMenuItem = () => { - const { emptyRecordFilter } = useEmptyRecordFilter(); - - const { closeDropdown } = useDropdown(); - - const handleRemoveFilter = () => { - emptyRecordFilter(); - closeDropdown(); - }; - - return ( - - - - ); -}; diff --git a/packages/twenty-front/src/modules/object-record/object-filter-dropdown/components/ObjectFilterDropdownTextInput.tsx b/packages/twenty-front/src/modules/object-record/object-filter-dropdown/components/ObjectFilterDropdownTextInput.tsx index fc23a0e87..4c39c6892 100644 --- a/packages/twenty-front/src/modules/object-record/object-filter-dropdown/components/ObjectFilterDropdownTextInput.tsx +++ b/packages/twenty-front/src/modules/object-record/object-filter-dropdown/components/ObjectFilterDropdownTextInput.tsx @@ -38,7 +38,7 @@ export const ObjectFilterDropdownTextInput = () => { }; return ( - + { {!!currentView && ( - - + + { @@ -157,7 +157,9 @@ export const ObjectOptionsDropdownLayoutContent = () => { onClick={handleSelectKanbanViewType} /> - + + + { @@ -232,8 +234,8 @@ export const ObjectOptionsDropdownLayoutContent = () => { )} - - + + )} ); diff --git a/packages/twenty-front/src/modules/object-record/object-options-dropdown/components/ObjectOptionsDropdownMenuContent.tsx b/packages/twenty-front/src/modules/object-record/object-options-dropdown/components/ObjectOptionsDropdownMenuContent.tsx index b2ddd4240..c67b79412 100644 --- a/packages/twenty-front/src/modules/object-record/object-options-dropdown/components/ObjectOptionsDropdownMenuContent.tsx +++ b/packages/twenty-front/src/modules/object-record/object-options-dropdown/components/ObjectOptionsDropdownMenuContent.tsx @@ -173,7 +173,9 @@ export const ObjectOptionsDropdownMenuContent = () => { width="100%" /> )} - + + + { diff --git a/packages/twenty-front/src/modules/object-record/object-sort-dropdown/components/ObjectSortDropdownButton.tsx b/packages/twenty-front/src/modules/object-record/object-sort-dropdown/components/ObjectSortDropdownButton.tsx index 5d3cf82fb..029082805 100644 --- a/packages/twenty-front/src/modules/object-record/object-sort-dropdown/components/ObjectSortDropdownButton.tsx +++ b/packages/twenty-front/src/modules/object-record/object-sort-dropdown/components/ObjectSortDropdownButton.tsx @@ -286,7 +286,7 @@ export const ObjectSortDropdownButton = ({ {shouldShowVisibleFields && ( <> - + {visibleFieldMetadataItems.map( (visibleFieldMetadataItem, index) => ( - + {hiddenFieldMetadataItems.map( (hiddenFieldMetadataItem, index) => ( ({ {!!items.length && ( <> - + {items.map((item, index) => renderItem({ value: item, diff --git a/packages/twenty-front/src/modules/object-record/select/components/MultipleSelectDropdown.tsx b/packages/twenty-front/src/modules/object-record/select/components/MultipleSelectDropdown.tsx index 2d988f984..bff9993ea 100644 --- a/packages/twenty-front/src/modules/object-record/select/components/MultipleSelectDropdown.tsx +++ b/packages/twenty-front/src/modules/object-record/select/components/MultipleSelectDropdown.tsx @@ -87,7 +87,7 @@ export const MultipleSelectDropdown = ({ selectableItemIdArray={selectableItemIds} hotkeyScope={hotkeyScope} > - + {itemsInDropdown?.map((item) => { return ( ` --padding: ${({ theme }) => theme.spacing(1)}; @@ -14,22 +10,27 @@ const StyledDropdownMenuItemsExternalContainer = styled.div<{ display: flex; flex-direction: column; - max-height: ${({ hasMaxHeight }) => (hasMaxHeight ? '168px' : 'none')}; + max-height: ${({ maxHeight }) => (maxHeight ? `${maxHeight}px` : 'none')}; + + width: 100%; + + height: fit-content; padding: var(--padding); - - ${({ width }) => - isDefined(width) && width === '100%' - ? css` - width: 100%; - ` - : css` - width: ${width}px; - `} + box-sizing: border-box; `; -const StyledDropdownMenuItemsInternalContainer = styled.div` - align-items: stretch; +const StyledScrollableContainer = styled.div<{ maxHeight?: number }>` + box-sizing: border-box; + + display: flex; + max-height: ${({ maxHeight }) => (maxHeight ? `${maxHeight}px` : 'none')}; + width: 100%; + + overflow-y: scroll; +`; + +const StyledInternalContainer = styled.div` display: flex; flex-direction: column; @@ -39,64 +40,28 @@ const StyledDropdownMenuItemsInternalContainer = styled.div` width: 100%; `; -const StyledScrollWrapper = styled(ScrollWrapper)` - width: 100%; -`; - export const DropdownMenuItemsContainer = ({ children, hasMaxHeight, - className, scrollable = true, - width = 'auto', - scrollWrapperHeightAuto, }: { children: React.ReactNode; hasMaxHeight?: boolean; - className?: string; scrollable?: boolean; - width?: number | 'auto' | '100%'; - scrollWrapperHeightAuto?: boolean; }) => { - const id = useId(); - - return scrollable !== true ? ( - - {hasMaxHeight ? ( - - - {children} - - - ) : ( - - {children} - - )} - + + {children} + + ) : ( - - - - {children} - - - + + {children} + ); }; diff --git a/packages/twenty-front/src/modules/ui/layout/dropdown/constants/DropdownMenuItemsContainerMaxHeight.ts b/packages/twenty-front/src/modules/ui/layout/dropdown/constants/DropdownMenuItemsContainerMaxHeight.ts new file mode 100644 index 000000000..23a6872c3 --- /dev/null +++ b/packages/twenty-front/src/modules/ui/layout/dropdown/constants/DropdownMenuItemsContainerMaxHeight.ts @@ -0,0 +1 @@ +export const DROPDOWN_MENU_ITEMS_CONTAINER_MAX_HEIGHT = 168; diff --git a/packages/twenty-front/src/modules/ui/navigation/navigation-drawer/components/MultiWorkspaceDropdown/internal/MultiWorkspaceDropdownDefaultComponents.tsx b/packages/twenty-front/src/modules/ui/navigation/navigation-drawer/components/MultiWorkspaceDropdown/internal/MultiWorkspaceDropdownDefaultComponents.tsx index 764425253..62bbbe0be 100644 --- a/packages/twenty-front/src/modules/ui/navigation/navigation-drawer/components/MultiWorkspaceDropdown/internal/MultiWorkspaceDropdownDefaultComponents.tsx +++ b/packages/twenty-front/src/modules/ui/navigation/navigation-drawer/components/MultiWorkspaceDropdown/internal/MultiWorkspaceDropdownDefaultComponents.tsx @@ -46,11 +46,6 @@ const StyledDescription = styled.div` padding-left: ${({ theme }) => theme.spacing(1)}; `; -const StyledDropdownMenuItemsContainer = styled.div` - margin: ${({ theme }) => theme.spacing(1)} 0; - padding: 0 ${({ theme }) => theme.spacing(1)}; -`; - export const MultiWorkspaceDropdownDefaultComponents = () => { const currentWorkspace = useRecoilValue(currentWorkspaceState); const { t } = useLingui(); @@ -134,7 +129,7 @@ export const MultiWorkspaceDropdownDefaultComponents = () => { {workspaces.length > 1 && ( <> - + {workspaces .filter(({ id }) => id !== currentWorkspace?.id) .slice(0, 3) @@ -171,7 +166,7 @@ export const MultiWorkspaceDropdownDefaultComponents = () => { hasSubMenu={true} /> )} - + )} diff --git a/packages/twenty-front/src/modules/ui/utilities/scroll/components/ScrollWrapper.tsx b/packages/twenty-front/src/modules/ui/utilities/scroll/components/ScrollWrapper.tsx index f7010de79..80a9edaae 100644 --- a/packages/twenty-front/src/modules/ui/utilities/scroll/components/ScrollWrapper.tsx +++ b/packages/twenty-front/src/modules/ui/utilities/scroll/components/ScrollWrapper.tsx @@ -7,7 +7,7 @@ import { scrollWrapperScrollLeftComponentState } from '@/ui/utilities/scroll/sta import { scrollWrapperScrollTopComponentState } from '@/ui/utilities/scroll/states/scrollWrapperScrollTopComponentState'; import { useSetRecoilComponentStateV2 } from '@/ui/utilities/state/component-state/hooks/useSetRecoilComponentStateV2'; -const StyledScrollWrapper = styled.div<{ height: string }>` +const StyledScrollWrapper = styled.div` &.scroll-wrapper-x-enabled { overflow-x: overlay; } @@ -17,7 +17,7 @@ const StyledScrollWrapper = styled.div<{ height: string }>` overflow-x: hidden; overflow-y: hidden; width: 100%; - height: ${({ height }) => height}; + height: 100%; `; export type ScrollWrapperProps = { @@ -26,7 +26,6 @@ export type ScrollWrapperProps = { defaultEnableXScroll?: boolean; defaultEnableYScroll?: boolean; componentInstanceId: string; - heightAuto?: boolean; }; export const ScrollWrapper = ({ @@ -35,7 +34,6 @@ export const ScrollWrapper = ({ className, defaultEnableXScroll = true, defaultEnableYScroll = true, - heightAuto = false, }: ScrollWrapperProps) => { const setScrollTop = useSetRecoilComponentStateV2( scrollWrapperScrollTopComponentState, @@ -73,7 +71,6 @@ export const ScrollWrapper = ({ id={`scroll-wrapper-${componentInstanceId}`} className={className} onScroll={handleScroll} - height={heightAuto ? 'auto' : '100%'} > {children} diff --git a/packages/twenty-front/src/modules/views/components/ViewBarFilterDropdownFieldSelectMenu.tsx b/packages/twenty-front/src/modules/views/components/ViewBarFilterDropdownFieldSelectMenu.tsx index 7c82518c6..b0efeccbe 100644 --- a/packages/twenty-front/src/modules/views/components/ViewBarFilterDropdownFieldSelectMenu.tsx +++ b/packages/twenty-front/src/modules/views/components/ViewBarFilterDropdownFieldSelectMenu.tsx @@ -98,8 +98,7 @@ export const ViewBarFilterDropdownFieldSelectMenu = () => { {shouldShowVisibleFields && ( <> - - + {selectableVisibleFieldMetadataItems.map( (visibleFieldMetadataItem) => ( { {shouldShowHiddenFields && ( <> - + {selectableHiddenFieldMetadataItems.map( (hiddenFieldMetadataItem) => ( { return ( - + {