Refactored and simplified DropdownMenuItemsContainer height management (#12547)
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 | <!-- notionvc: a3a87101-9944-4b03-a29d-b2974d5ffa9d --> --------- Co-authored-by: Charles Bochet <charles@twenty.com>
This commit is contained in:
@@ -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}
|
||||
</StyledScrollWrapper>
|
||||
|
||||
Reference in New Issue
Block a user