From 468744298b15ddf14468928459dafb7aec47e075 Mon Sep 17 00:00:00 2001 From: Lucas Bordeau Date: Thu, 14 Dec 2023 18:54:23 +0100 Subject: [PATCH] Fix hook bug (#2995) * Fix hook bug * Fix --------- Co-authored-by: Charles Bochet --- .../command-menu/components/CommandMenu.tsx | 2 +- .../components/MultipleEntitySelect.tsx | 2 +- .../components/SelectableMenuItemSelect.tsx | 50 +++++ .../components/SingleEntitySelect.tsx | 13 +- .../components/SingleEntitySelectBase.tsx | 177 +++++++----------- .../hooks/useEntitySelectScroll.ts | 101 ---------- .../ui/input/components/IconPicker.tsx | 2 +- .../components/CreateNewButton.tsx | 2 +- .../components/SelectableItem.tsx | 19 +- .../components/SelectableList.tsx | 32 ++-- .../internal/useSelectableListHotKeys.tsx | 16 +- .../components/StyledMenuItemBase.tsx | 2 +- 12 files changed, 170 insertions(+), 248 deletions(-) create mode 100644 packages/twenty-front/src/modules/object-record/relation-picker/components/SelectableMenuItemSelect.tsx delete mode 100644 packages/twenty-front/src/modules/object-record/relation-picker/hooks/useEntitySelectScroll.ts diff --git a/packages/twenty-front/src/modules/command-menu/components/CommandMenu.tsx b/packages/twenty-front/src/modules/command-menu/components/CommandMenu.tsx index 5d0fc5e20..5805ce1a9 100644 --- a/packages/twenty-front/src/modules/command-menu/components/CommandMenu.tsx +++ b/packages/twenty-front/src/modules/command-menu/components/CommandMenu.tsx @@ -228,7 +228,7 @@ export const CommandMenu = () => { { const command = [ diff --git a/packages/twenty-front/src/modules/object-record/relation-picker/components/MultipleEntitySelect.tsx b/packages/twenty-front/src/modules/object-record/relation-picker/components/MultipleEntitySelect.tsx index bb7aff7a1..b2cb18d1c 100644 --- a/packages/twenty-front/src/modules/object-record/relation-picker/components/MultipleEntitySelect.tsx +++ b/packages/twenty-front/src/modules/object-record/relation-picker/components/MultipleEntitySelect.tsx @@ -89,7 +89,7 @@ export const MultipleEntitySelect = < { if (_itemId in value === false || value[_itemId] === false) { diff --git a/packages/twenty-front/src/modules/object-record/relation-picker/components/SelectableMenuItemSelect.tsx b/packages/twenty-front/src/modules/object-record/relation-picker/components/SelectableMenuItemSelect.tsx new file mode 100644 index 000000000..a40737f33 --- /dev/null +++ b/packages/twenty-front/src/modules/object-record/relation-picker/components/SelectableMenuItemSelect.tsx @@ -0,0 +1,50 @@ +import styled from '@emotion/styled'; + +import { EntityForSelect } from '@/object-record/relation-picker/types/EntityForSelect'; +import { SelectableItem } from '@/ui/layout/selectable-list/components/SelectableItem'; +import { useSelectableList } from '@/ui/layout/selectable-list/hooks/useSelectableList'; +import { MenuItemSelectAvatar } from '@/ui/navigation/menu-item/components/MenuItemSelectAvatar'; +import { Avatar } from '@/users/components/Avatar'; + +type SelectableMenuItemSelectProps = { + entity: EntityForSelect; + onEntitySelected: (entitySelected?: EntityForSelect) => void; + selectedEntity?: EntityForSelect; +}; + +const StyledSelectableItem = styled(SelectableItem)` + width: 100%; +`; + +export const SelectableMenuItemSelect = ({ + entity, + onEntitySelected, + selectedEntity, +}: SelectableMenuItemSelectProps) => { + const { isSelectedItemId } = useSelectableList({ + selectableListId: 'single-entity-select-base-list', + itemId: entity.id, + }); + + return ( + + onEntitySelected(entity)} + text={entity.name} + selected={selectedEntity?.id === entity.id} + hovered={isSelectedItemId} + avatar={ + + } + /> + + ); +}; diff --git a/packages/twenty-front/src/modules/object-record/relation-picker/components/SingleEntitySelect.tsx b/packages/twenty-front/src/modules/object-record/relation-picker/components/SingleEntitySelect.tsx index 4eb6be35a..403044053 100644 --- a/packages/twenty-front/src/modules/object-record/relation-picker/components/SingleEntitySelect.tsx +++ b/packages/twenty-front/src/modules/object-record/relation-picker/components/SingleEntitySelect.tsx @@ -7,21 +7,18 @@ import { useListenClickOutside } from '@/ui/utilities/pointer-event/hooks/useLis import { isDefined } from '~/utils/isDefined'; import { useEntitySelectSearch } from '../hooks/useEntitySelectSearch'; -import { EntityForSelect } from '../types/EntityForSelect'; import { SingleEntitySelectBase, SingleEntitySelectBaseProps, } from './SingleEntitySelectBase'; -export type SingleEntitySelectProps< - CustomEntityForSelect extends EntityForSelect, -> = { +export type SingleEntitySelectProps = { disableBackgroundBlur?: boolean; onCreate?: () => void; width?: number; } & Pick< - SingleEntitySelectBaseProps, + SingleEntitySelectBaseProps, | 'EmptyIcon' | 'emptyLabel' | 'entitiesToSelect' @@ -31,9 +28,7 @@ export type SingleEntitySelectProps< | 'selectedEntity' >; -export const SingleEntitySelect = < - CustomEntityForSelect extends EntityForSelect, ->({ +export const SingleEntitySelect = ({ EmptyIcon, disableBackgroundBlur = false, emptyLabel, @@ -44,7 +39,7 @@ export const SingleEntitySelect = < onEntitySelected, selectedEntity, width = 200, -}: SingleEntitySelectProps) => { +}: SingleEntitySelectProps) => { const containerRef = useRef(null); const { searchFilter, handleSearchFilterChange } = useEntitySelectSearch(); diff --git a/packages/twenty-front/src/modules/object-record/relation-picker/components/SingleEntitySelectBase.tsx b/packages/twenty-front/src/modules/object-record/relation-picker/components/SingleEntitySelectBase.tsx index 440402d9e..b4aff2c0e 100644 --- a/packages/twenty-front/src/modules/object-record/relation-picker/components/SingleEntitySelectBase.tsx +++ b/packages/twenty-front/src/modules/object-record/relation-picker/components/SingleEntitySelectBase.tsx @@ -1,39 +1,31 @@ -/* eslint-disable react-hooks/rules-of-hooks */ import { useRef } from 'react'; import { isNonEmptyString } from '@sniptt/guards'; import { Key } from 'ts-key-enum'; +import { SelectableMenuItemSelect } from '@/object-record/relation-picker/components/SelectableMenuItemSelect'; import { IconPlus } from '@/ui/display/icon'; import { IconComponent } from '@/ui/display/icon/types/IconComponent'; import { DropdownMenuItemsContainer } from '@/ui/layout/dropdown/components/DropdownMenuItemsContainer'; import { DropdownMenuSeparator } from '@/ui/layout/dropdown/components/DropdownMenuSeparator'; -import { SelectableItem } from '@/ui/layout/selectable-list/components/SelectableItem'; import { SelectableList } from '@/ui/layout/selectable-list/components/SelectableList'; -import { useSelectableList } from '@/ui/layout/selectable-list/hooks/useSelectableList'; import { MenuItem } from '@/ui/navigation/menu-item/components/MenuItem'; import { MenuItemSelect } from '@/ui/navigation/menu-item/components/MenuItemSelect'; -import { MenuItemSelectAvatar } from '@/ui/navigation/menu-item/components/MenuItemSelectAvatar'; import { useScopedHotkeys } from '@/ui/utilities/hotkey/hooks/useScopedHotkeys'; -import { Avatar } from '@/users/components/Avatar'; import { assertNotNull } from '~/utils/assert'; import { CreateNewButton } from '../../../ui/input/relation-picker/components/CreateNewButton'; import { DropdownMenuSkeletonItem } from '../../../ui/input/relation-picker/components/skeletons/DropdownMenuSkeletonItem'; -import { CreateButtonId, EmptyButtonId } from '../constants'; -import { useEntitySelectScroll } from '../hooks/useEntitySelectScroll'; import { EntityForSelect } from '../types/EntityForSelect'; import { RelationPickerHotkeyScope } from '../types/RelationPickerHotkeyScope'; -export type SingleEntitySelectBaseProps< - CustomEntityForSelect extends EntityForSelect, -> = { +export type SingleEntitySelectBaseProps = { EmptyIcon?: IconComponent; emptyLabel?: string; - entitiesToSelect: CustomEntityForSelect[]; + entitiesToSelect: EntityForSelect[]; loading?: boolean; onCancel?: () => void; - onEntitySelected: (entity?: CustomEntityForSelect) => void; - selectedEntity?: CustomEntityForSelect; + onEntitySelected: (entity?: EntityForSelect) => void; + selectedEntity?: EntityForSelect; onCreate?: () => void; showCreateButton?: boolean; SelectAllIcon?: IconComponent; @@ -43,9 +35,7 @@ export type SingleEntitySelectBaseProps< onAllEntitySelected?: () => void; }; -export const SingleEntitySelectBase = < - CustomEntityForSelect extends EntityForSelect, ->({ +export const SingleEntitySelectBase = ({ EmptyIcon, emptyLabel, entitiesToSelect, @@ -60,23 +50,14 @@ export const SingleEntitySelectBase = < isAllEntitySelected, isAllEntitySelectShown, onAllEntitySelected, -}: SingleEntitySelectBaseProps) => { +}: SingleEntitySelectBaseProps) => { const containerRef = useRef(null); const entitiesInDropdown = [selectedEntity, ...entitiesToSelect].filter( - (entity): entity is CustomEntityForSelect => + (entity): entity is EntityForSelect => assertNotNull(entity) && isNonEmptyString(entity.name), ); - const { preselectedOptionId } = useEntitySelectScroll({ - selectableOptionIds: [ - EmptyButtonId, - ...entitiesInDropdown.map((item) => item.id), - ...(showCreateButton ? [CreateButtonId] : []), - ], - containerRef, - }); - useScopedHotkeys( Key.Escape, () => { @@ -90,94 +71,72 @@ export const SingleEntitySelectBase = < return (
- - {loading ? ( - - ) : entitiesInDropdown.length === 0 && !isAllEntitySelectShown ? ( - - ) : ( - <> - {isAllEntitySelectShown && - selectAllLabel && - onAllEntitySelected && ( + { + if (showCreateButton) { + onCreate?.(); + } else { + const entity = entitiesInDropdown.findIndex( + (entity) => entity.id === _itemId, + ); + onEntitySelected(entitiesInDropdown[entity]); + } + }} + > + + {loading ? ( + + ) : entitiesInDropdown.length === 0 && !isAllEntitySelectShown ? ( + + ) : ( + <> + {isAllEntitySelectShown && + selectAllLabel && + onAllEntitySelected && ( + onAllEntitySelected()} + LeftIcon={SelectAllIcon} + text={selectAllLabel} + selected={!!isAllEntitySelected} + /> + )} + {emptyLabel && ( onAllEntitySelected()} - LeftIcon={SelectAllIcon} - text={selectAllLabel} - hovered={preselectedOptionId === EmptyButtonId} - selected={!!isAllEntitySelected} + key="select-none" + onClick={() => onEntitySelected()} + LeftIcon={EmptyIcon} + text={emptyLabel} + selected={!selectedEntity} /> )} - {emptyLabel && ( - onEntitySelected()} - LeftIcon={EmptyIcon} - text={emptyLabel} - hovered={preselectedOptionId === EmptyButtonId} - selected={!selectedEntity} + {entitiesInDropdown?.map((entity) => ( + + ))} + + )} + + {showCreateButton && ( + <> + + + - )} - {entitiesInDropdown?.map((entity) => ( - { - if ( - showCreateButton && - preselectedOptionId === CreateButtonId - ) { - onCreate?.(); - } else { - const entity = entitiesInDropdown.findIndex( - (entity) => entity.id === _itemId, - ); - onEntitySelected(entitiesInDropdown[entity]); - } - }} - > - - onEntitySelected(entity)} - text={entity.name} - selected={selectedEntity?.id === entity.id} - hovered={ - useSelectableList({ - selectableListId: 'single-entity-select-base-list', - itemId: entity.id, - }).isSelectedItemId - } - avatar={ - - } - /> - - - ))} + )} - - {showCreateButton && ( - <> - - - - - - )} +
); }; diff --git a/packages/twenty-front/src/modules/object-record/relation-picker/hooks/useEntitySelectScroll.ts b/packages/twenty-front/src/modules/object-record/relation-picker/hooks/useEntitySelectScroll.ts deleted file mode 100644 index b6608b2b5..000000000 --- a/packages/twenty-front/src/modules/object-record/relation-picker/hooks/useEntitySelectScroll.ts +++ /dev/null @@ -1,101 +0,0 @@ -import scrollIntoView from 'scroll-into-view'; -import { Key } from 'ts-key-enum'; - -import { useRelationPicker } from '@/object-record/relation-picker/hooks/useRelationPicker'; -import { useScopedHotkeys } from '@/ui/utilities/hotkey/hooks/useScopedHotkeys'; - -import { CreateButtonId } from '../constants'; -import { RelationPickerHotkeyScope } from '../types/RelationPickerHotkeyScope'; -import { getPreselectedIdIndex } from '../utils/getPreselectedIdIndex'; - -export const useEntitySelectScroll = ({ - containerRef, - selectableOptionIds, -}: { - selectableOptionIds: string[]; - containerRef: React.RefObject; -}) => { - const { relationPickerPreselectedId, setRelationPickerPreselectedId } = - useRelationPicker(); - - const preselectedIdIndex = getPreselectedIdIndex( - selectableOptionIds, - relationPickerPreselectedId ?? '', - ); - - const resetScroll = () => { - setRelationPickerPreselectedId(''); - - const preselectedRef = containerRef.current?.children[0] as HTMLElement; - - scrollIntoView(preselectedRef, { - align: { - top: 0, - }, - isScrollable: (target) => { - return target === containerRef.current; - }, - time: 0, - }); - }; - - useScopedHotkeys( - Key.ArrowUp, - () => { - const previousSelectableIndex = Math.max(preselectedIdIndex - 1, 0); - const previousSelectableId = selectableOptionIds[previousSelectableIndex]; - setRelationPickerPreselectedId(previousSelectableId); - const preselectedRef = containerRef.current?.children[ - previousSelectableIndex - ] as HTMLElement; - - if (preselectedRef) { - scrollIntoView(preselectedRef, { - align: { - top: 0.5, - }, - isScrollable: (target) => { - return target === containerRef.current; - }, - time: 0, - }); - } - }, - RelationPickerHotkeyScope.RelationPicker, - [selectableOptionIds], - ); - - useScopedHotkeys( - Key.ArrowDown, - () => { - const nextSelectableIndex = Math.min( - preselectedIdIndex + 1, - selectableOptionIds?.length - 1, - ); - const nextSelectableId = selectableOptionIds[nextSelectableIndex]; - setRelationPickerPreselectedId(nextSelectableId); - if (nextSelectableId !== CreateButtonId) { - const preselectedRef = containerRef.current?.children[ - nextSelectableIndex - ] as HTMLElement; - - if (preselectedRef) { - scrollIntoView(preselectedRef, { - align: { - top: 0.15, - }, - isScrollable: (target) => target === containerRef.current, - time: 0, - }); - } - } - }, - RelationPickerHotkeyScope.RelationPicker, - [selectableOptionIds], - ); - - return { - preselectedOptionId: relationPickerPreselectedId, - resetScroll, - }; -}; diff --git a/packages/twenty-front/src/modules/ui/input/components/IconPicker.tsx b/packages/twenty-front/src/modules/ui/input/components/IconPicker.tsx index 1d3961d22..d83cb49d7 100644 --- a/packages/twenty-front/src/modules/ui/input/components/IconPicker.tsx +++ b/packages/twenty-front/src/modules/ui/input/components/IconPicker.tsx @@ -138,7 +138,7 @@ export const IconPicker = ({ dropdownComponents={ { onChange({ iconKey, Icon: getIcon(iconKey) }); diff --git a/packages/twenty-front/src/modules/ui/input/relation-picker/components/CreateNewButton.tsx b/packages/twenty-front/src/modules/ui/input/relation-picker/components/CreateNewButton.tsx index a9c7748c8..9cdade903 100644 --- a/packages/twenty-front/src/modules/ui/input/relation-picker/components/CreateNewButton.tsx +++ b/packages/twenty-front/src/modules/ui/input/relation-picker/components/CreateNewButton.tsx @@ -3,7 +3,7 @@ import styled from '@emotion/styled'; import { MenuItem } from '@/ui/navigation/menu-item/components/MenuItem'; -const StyledCreateNewButton = styled(MenuItem)<{ hovered: boolean }>` +const StyledCreateNewButton = styled(MenuItem)<{ hovered?: boolean }>` ${({ hovered, theme }) => hovered && css` diff --git a/packages/twenty-front/src/modules/ui/layout/selectable-list/components/SelectableItem.tsx b/packages/twenty-front/src/modules/ui/layout/selectable-list/components/SelectableItem.tsx index 4f2feccdf..0cc9cb10f 100644 --- a/packages/twenty-front/src/modules/ui/layout/selectable-list/components/SelectableItem.tsx +++ b/packages/twenty-front/src/modules/ui/layout/selectable-list/components/SelectableItem.tsx @@ -1,14 +1,19 @@ -import React, { useEffect, useRef } from 'react'; +import { ReactNode, useEffect, useRef } from 'react'; import { useRecoilValue } from 'recoil'; import { useSelectableListScopedStates } from '@/ui/layout/selectable-list/hooks/internal/useSelectableListScopedStates'; -type SelectableItemProps = { +export type SelectableItemProps = { itemId: string; - children: React.ReactElement; + children: ReactNode; + className?: string; }; -export const SelectableItem = ({ itemId, children }: SelectableItemProps) => { +export const SelectableItem = ({ + itemId, + children, + className, +}: SelectableItemProps) => { const { isSelectedItemIdSelector } = useSelectableListScopedStates({ itemId: itemId, }); @@ -23,5 +28,9 @@ export const SelectableItem = ({ itemId, children }: SelectableItemProps) => { } }, [isSelectedItemId]); - return
{children}
; + return ( +
+ {children} +
+ ); }; diff --git a/packages/twenty-front/src/modules/ui/layout/selectable-list/components/SelectableList.tsx b/packages/twenty-front/src/modules/ui/layout/selectable-list/components/SelectableList.tsx index e65c499c0..df06ccae3 100644 --- a/packages/twenty-front/src/modules/ui/layout/selectable-list/components/SelectableList.tsx +++ b/packages/twenty-front/src/modules/ui/layout/selectable-list/components/SelectableList.tsx @@ -1,28 +1,26 @@ import { ReactNode, useEffect } from 'react'; -import styled from '@emotion/styled'; import { useSelectableListHotKeys } from '@/ui/layout/selectable-list/hooks/internal/useSelectableListHotKeys'; import { useSelectableList } from '@/ui/layout/selectable-list/hooks/useSelectableList'; import { SelectableListScope } from '@/ui/layout/selectable-list/scopes/SelectableListScope'; +import { arrayToChunks } from '~/utils/array/array-to-chunks'; type SelectableListProps = { children: ReactNode; selectableListId: string; - selectableItemIds: string[][]; + selectableItemIdArray?: string[]; + selectableItemIdMatrix?: string[][]; onSelect?: (selected: string) => void; hotkeyScope: string; onEnter?: (itemId: string) => void; }; -const StyledSelectableItemsContainer = styled.div` - width: 100%; -`; - export const SelectableList = ({ children, selectableListId, hotkeyScope, - selectableItemIds, + selectableItemIdArray, + selectableItemIdMatrix, onEnter, }: SelectableListProps) => { useSelectableListHotKeys(selectableListId, hotkeyScope); @@ -36,14 +34,24 @@ export const SelectableList = ({ }, [onEnter, setSelectableListOnEnter]); useEffect(() => { - setSelectableItemIds(selectableItemIds); - }, [selectableItemIds, setSelectableItemIds]); + if (!selectableItemIdArray && !selectableItemIdMatrix) { + throw new Error( + 'Either selectableItemIdArray or selectableItemIdsMatrix must be provided', + ); + } + + if (selectableItemIdMatrix) { + setSelectableItemIds(selectableItemIdMatrix); + } + + if (selectableItemIdArray) { + setSelectableItemIds(arrayToChunks(selectableItemIdArray, 1)); + } + }, [selectableItemIdArray, selectableItemIdMatrix, setSelectableItemIds]); return ( - - {children} - + {children} ); }; diff --git a/packages/twenty-front/src/modules/ui/layout/selectable-list/hooks/internal/useSelectableListHotKeys.tsx b/packages/twenty-front/src/modules/ui/layout/selectable-list/hooks/internal/useSelectableListHotKeys.tsx index 0f75dae3d..a710370de 100644 --- a/packages/twenty-front/src/modules/ui/layout/selectable-list/hooks/internal/useSelectableListHotKeys.tsx +++ b/packages/twenty-front/src/modules/ui/layout/selectable-list/hooks/internal/useSelectableListHotKeys.tsx @@ -16,7 +16,7 @@ export const useSelectableListHotKeys = ( selectedItemId?: string | null, ) => { if (!selectedItemId) { - return { row: 0, col: -1 }; + return; } for (let row = 0; row < selectableItemIds.length; row++) { @@ -25,7 +25,6 @@ export const useSelectableListHotKeys = ( return { row, col }; } } - return { row: 0, col: 0 }; }; const handleSelect = useRecoilCallback( @@ -41,12 +40,15 @@ export const useSelectableListHotKeys = ( selectableItemIdsState, ); - const { row: currentRow, col: currentCol } = findPosition( - selectableItemIds, - selectedItemId, - ); + const currentPosition = findPosition(selectableItemIds, selectedItemId); const computeNextId = (direction: Direction) => { + if (!selectedItemId || !currentPosition) { + return selectableItemIds[0][0]; + } + + const { row: currentRow, col: currentCol } = currentPosition; + if (selectableItemIds.length === 0) { return; } @@ -93,7 +95,7 @@ export const useSelectableListHotKeys = ( const nextId = computeNextId(direction); - if (!selectedItemId || (selectedItemId && selectedItemId !== nextId)) { + if (selectedItemId !== nextId) { if (nextId) { const { isSelectedItemIdSelector } = getSelectableListScopedStates({ selectableListScopeId: scopeId, diff --git a/packages/twenty-front/src/modules/ui/navigation/menu-item/internals/components/StyledMenuItemBase.tsx b/packages/twenty-front/src/modules/ui/navigation/menu-item/internals/components/StyledMenuItemBase.tsx index c67a1c574..1ee462483 100644 --- a/packages/twenty-front/src/modules/ui/navigation/menu-item/internals/components/StyledMenuItemBase.tsx +++ b/packages/twenty-front/src/modules/ui/navigation/menu-item/internals/components/StyledMenuItemBase.tsx @@ -19,7 +19,7 @@ export const StyledMenuItemBase = styled.li` background: ${({ isKeySelected, theme }) => isKeySelected ? theme.background.transparent.light - : theme.background.primary}; + : theme.background.secondary}; border-radius: ${({ theme }) => theme.border.radius.sm}; cursor: pointer;