Skip to content

Commit

Permalink
Fixed sync between record value context selector and record store (tw…
Browse files Browse the repository at this point in the history
…entyhq#5517)

This PR introduces many improvements over the new profiling story
feature, with new tests and some refactor with main :
- Added use-context-selector for getting value faster in display fields
and created useRecordFieldValue() hook and RecordValueSetterEffect to
synchronize states
- Added performance test command in CI
- Refactored ExpandableList drill-downs with FieldFocusContext
- Refactored field button icon logic into getFieldButtonIcon util
- Added RelationFieldDisplay perf story
- Added RecordTableCell perf story
- First split test of useField.. hook with useRelationFieldDisplay()
- Fixed problem with set cell soft focus
- Isolated logic between display / soft focus and edit mode in the
related components to optimize performances for display mode.
- Added warmupRound config for performance story decorator
- Added variance in test reporting
  • Loading branch information
lucasbordeau authored May 24, 2024
1 parent 82ec30c commit de9321d
Show file tree
Hide file tree
Showing 47 changed files with 2,042 additions and 553 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,7 @@
"tsup": "^8.0.1",
"type-fest": "4.10.1",
"typeorm": "^0.3.17",
"use-context-selector": "^2.0.0",
"use-debounce": "^10.0.0",
"uuid": "^9.0.0",
"vite-tsconfig-paths": "^4.2.1",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,12 @@ export const ActivityTargetsInlineCell = ({
/>
}
label="Relations"
displayModeContent={() => (
displayModeContent={
<ActivityTargetChips
activityTargetObjectRecords={activityTargetObjectRecords}
maxWidth={maxWidth}
/>
)}
}
isDisplayModeContentEmpty={activityTargetObjectRecords.length === 0}
/>
</RecordFieldInputScope>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { ObjectMetadataItem } from '@/object-metadata/types/ObjectMetadataItem';
import { parseFieldRelationType } from '@/object-metadata/utils/parseFieldRelationType';
import { FieldDefinition } from '@/object-record/record-field/types/FieldDefinition';
import { FieldMetadata } from '@/object-record/record-field/types/FieldMetadata';
import { getFieldButtonIcon } from '@/object-record/record-field/utils/getFieldButtonIcon';

import { FieldMetadataItem } from '../types/FieldMetadataItem';

Expand All @@ -15,7 +18,7 @@ export const formatFieldMetadataItemAsFieldDefinition = ({
objectMetadataItem,
showLabel,
labelWidth,
}: FieldMetadataItemAsFieldDefinitionProps) => {
}: FieldMetadataItemAsFieldDefinitionProps): FieldDefinition<FieldMetadata> => {
const relationObjectMetadataItem =
field.toRelationMetadata?.fromObjectMetadata ||
field.fromRelationMetadata?.toObjectMetadata;
Expand All @@ -24,25 +27,31 @@ export const formatFieldMetadataItemAsFieldDefinition = ({
field.toRelationMetadata?.fromFieldMetadataId ||
field.fromRelationMetadata?.toFieldMetadataId;

const fieldDefintionMetadata = {
fieldName: field.name,
placeHolder: field.label,
relationType: parseFieldRelationType(field),
relationFieldMetadataId,
relationObjectMetadataNameSingular:
relationObjectMetadataItem?.nameSingular ?? '',
relationObjectMetadataNamePlural:
relationObjectMetadataItem?.namePlural ?? '',
objectMetadataNameSingular: objectMetadataItem.nameSingular ?? '',
options: field.options,
};

return {
fieldMetadataId: field.id,
label: field.label,
showLabel,
labelWidth,
type: field.type,
metadata: {
fieldName: field.name,
placeHolder: field.label,
relationType: parseFieldRelationType(field),
relationFieldMetadataId,
relationObjectMetadataNameSingular:
relationObjectMetadataItem?.nameSingular ?? '',
relationObjectMetadataNamePlural:
relationObjectMetadataItem?.namePlural ?? '',
objectMetadataNameSingular: objectMetadataItem.nameSingular ?? '',
options: field.options,
},
metadata: fieldDefintionMetadata,
iconName: field.icon ?? 'Icon123',
defaultValue: field.defaultValue,
editButtonIcon: getFieldButtonIcon({
metadata: fieldDefintionMetadata,
type: field.type,
}),
};
};
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@ import {
RecordUpdateHook,
RecordUpdateHookParams,
} from '@/object-record/record-field/contexts/FieldContext';
import { getFieldButtonIcon } from '@/object-record/record-field/utils/getFieldButtonIcon';
import { RecordInlineCell } from '@/object-record/record-inline-cell/components/RecordInlineCell';
import { InlineCellHotkeyScope } from '@/object-record/record-inline-cell/types/InlineCellHotkeyScope';
import { RecordValueSetterEffect } from '@/object-record/record-store/components/RecordValueSetterEffect';
import { recordStoreFamilyState } from '@/object-record/record-store/states/recordStoreFamilyState';
import { LightIconButton } from '@/ui/input/button/components/LightIconButton';
import { Checkbox, CheckboxVariant } from '@/ui/input/components/Checkbox';
Expand Down Expand Up @@ -209,6 +211,7 @@ export const RecordBoardCard = () => {

return (
<StyledBoardCardWrapper onContextMenu={handleContextMenu}>
<RecordValueSetterEffect recordId={recordId} />
<StyledBoardCard
ref={cardRef}
selected={isCurrentCardSelected}
Expand Down Expand Up @@ -266,6 +269,10 @@ export const RecordBoardCard = () => {
type: fieldDefinition.type,
metadata: fieldDefinition.metadata,
defaultValue: fieldDefinition.defaultValue,
editButtonIcon: getFieldButtonIcon({
metadata: fieldDefinition.metadata,
type: fieldDefinition.type,
}),
},
useUpdateRecord: useUpdateOneRecordHook,
hotkeyScope: InlineCellHotkeyScope.InlineCell,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,17 +37,7 @@ import { isFieldSelect } from '../types/guards/isFieldSelect';
import { isFieldText } from '../types/guards/isFieldText';
import { isFieldUuid } from '../types/guards/isFieldUuid';

type FieldDisplayProps = {
isCellSoftFocused?: boolean;
cellElement?: HTMLElement;
fromTableCell?: boolean;
};

export const FieldDisplay = ({
isCellSoftFocused,
cellElement,
fromTableCell,
}: FieldDisplayProps) => {
export const FieldDisplay = () => {
const { fieldDefinition, isLabelIdentifier } = useContext(FieldContext);

const isChipDisplay =
Expand Down Expand Up @@ -78,22 +68,15 @@ export const FieldDisplay = ({
) : isFieldLink(fieldDefinition) ? (
<LinkFieldDisplay />
) : isFieldLinks(fieldDefinition) ? (
<LinksFieldDisplay
isCellSoftFocused={isCellSoftFocused}
fromTableCell={fromTableCell}
/>
<LinksFieldDisplay />
) : isFieldCurrency(fieldDefinition) ? (
<CurrencyFieldDisplay />
) : isFieldFullName(fieldDefinition) ? (
<FullNameFieldDisplay />
) : isFieldSelect(fieldDefinition) ? (
<SelectFieldDisplay />
) : isFieldMultiSelect(fieldDefinition) ? (
<MultiSelectFieldDisplay
isCellSoftFocused={isCellSoftFocused}
cellElement={cellElement}
fromTableCell={fromTableCell}
/>
<MultiSelectFieldDisplay />
) : isFieldAddress(fieldDefinition) ? (
<AddressFieldDisplay />
) : isFieldRawJson(fieldDefinition) ? (
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { createContext } from 'react';

export type FieldFocusContextType = {
isFocused: boolean;
setIsFocused: (isFocused: boolean) => void;
};

export const FieldFocusContext = createContext<FieldFocusContextType>(
{} as FieldFocusContextType,
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import { useState } from 'react';

import { FieldFocusContext } from '@/object-record/record-field/contexts/FieldFocusContext';

export const FieldFocusContextProvider = ({ children }: any) => {
const [isFocused, setIsFocused] = useState(false);

return (
<FieldFocusContext.Provider
value={{
isFocused,
setIsFocused,
}}
>
{children}
</FieldFocusContext.Provider>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { useContext } from 'react';

import { FieldFocusContext } from '@/object-record/record-field/contexts/FieldFocusContext';

export const useFieldFocus = () => {
const { isFocused, setIsFocused } = useContext(FieldFocusContext);

return {
isFocused,
setIsFocused,
};
};
Original file line number Diff line number Diff line change
@@ -1,33 +1,12 @@
import { useContext } from 'react';
import { IconComponent, IconPencil } from 'twenty-ui';
import { IconComponent } from 'twenty-ui';

import { isFieldDisplayedAsPhone } from '@/object-record/record-field/types/guards/isFieldDisplayedAsPhone';
import { isFieldLinks } from '@/object-record/record-field/types/guards/isFieldLinks';
import { isFieldMultiSelect } from '@/object-record/record-field/types/guards/isFieldMultiSelect';
import { isFieldRelation } from '@/object-record/record-field/types/guards/isFieldRelation';
import { isUndefinedOrNull } from '~/utils/isUndefinedOrNull';
import { getFieldButtonIcon } from '@/object-record/record-field/utils/getFieldButtonIcon';

import { FieldContext } from '../contexts/FieldContext';
import { isFieldEmail } from '../types/guards/isFieldEmail';
import { isFieldLink } from '../types/guards/isFieldLink';
import { isFieldPhone } from '../types/guards/isFieldPhone';

export const useGetButtonIcon = (): IconComponent | undefined => {
const { fieldDefinition } = useContext(FieldContext);

if (isUndefinedOrNull(fieldDefinition)) return undefined;

if (
isFieldLink(fieldDefinition) ||
isFieldEmail(fieldDefinition) ||
isFieldPhone(fieldDefinition) ||
isFieldDisplayedAsPhone(fieldDefinition) ||
isFieldMultiSelect(fieldDefinition) ||
(isFieldRelation(fieldDefinition) &&
fieldDefinition.metadata.relationObjectMetadataNameSingular !==
'workspaceMember') ||
isFieldLinks(fieldDefinition)
) {
return IconPencil;
}
return getFieldButtonIcon(fieldDefinition);
};
Original file line number Diff line number Diff line change
@@ -1,18 +1,16 @@
import { useContext } from 'react';
import { useRecoilValue } from 'recoil';

import { isFieldValueEmpty } from '@/object-record/record-field/utils/isFieldValueEmpty';
import { recordStoreFamilySelector } from '@/object-record/record-store/states/selectors/recordStoreFamilySelector';
import { useRecordFieldValue } from '@/object-record/record-store/contexts/RecordFieldValueSelectorContext';

import { FieldContext } from '../contexts/FieldContext';

export const useIsFieldEmpty = () => {
const { entityId, fieldDefinition } = useContext(FieldContext);
const fieldValue = useRecoilValue(
recordStoreFamilySelector({
fieldName: fieldDefinition.metadata.fieldName,
recordId: entityId,
}),

const fieldValue = useRecordFieldValue(
entityId,
fieldDefinition.metadata.fieldName,
);

return isFieldValueEmpty({
Expand Down
Original file line number Diff line number Diff line change
@@ -1,22 +1,11 @@
import { useFieldFocus } from '@/object-record/record-field/hooks/useFieldFocus';
import { useLinksField } from '@/object-record/record-field/meta-types/hooks/useLinksField';
import { LinksDisplay } from '@/ui/field/display/components/LinksDisplay';

type LinksFieldDisplayProps = {
isCellSoftFocused?: boolean;
fromTableCell?: boolean;
};

export const LinksFieldDisplay = ({
isCellSoftFocused,
fromTableCell,
}: LinksFieldDisplayProps) => {
export const LinksFieldDisplay = () => {
const { fieldValue } = useLinksField();

return (
<LinksDisplay
value={fieldValue}
isChipCountDisplayed={isCellSoftFocused}
withExpandedListBorder={fromTableCell}
/>
);
const { isFocused } = useFieldFocus();

return <LinksDisplay value={fieldValue} isFocused={isFocused} />;
};
Original file line number Diff line number Diff line change
@@ -1,20 +1,14 @@
import { Tag } from 'twenty-ui';

import { useFieldFocus } from '@/object-record/record-field/hooks/useFieldFocus';
import { useMultiSelectField } from '@/object-record/record-field/meta-types/hooks/useMultiSelectField';
import { ExpandableList } from '@/ui/layout/expandable-list/components/ExpandableList';

type MultiSelectFieldDisplayProps = {
isCellSoftFocused?: boolean;
cellElement?: HTMLElement;
fromTableCell?: boolean;
};

export const MultiSelectFieldDisplay = ({
isCellSoftFocused,
fromTableCell,
}: MultiSelectFieldDisplayProps) => {
export const MultiSelectFieldDisplay = () => {
const { fieldValues, fieldDefinition } = useMultiSelectField();

const { isFocused } = useFieldFocus();

const selectedOptions = fieldValues
? fieldDefinition.metadata.options?.filter((option) =>
fieldValues.includes(option.value),
Expand All @@ -24,10 +18,7 @@ export const MultiSelectFieldDisplay = ({
if (!selectedOptions) return null;

return (
<ExpandableList
isChipCountDisplayed={isCellSoftFocused}
withExpandedListBorder={fromTableCell}
>
<ExpandableList isChipCountDisplayed={isFocused}>
{selectedOptions.map((selectedOption, index) => (
<Tag
key={index}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,16 @@
import { RecordChip } from '@/object-record/components/RecordChip';
import { useRelationFieldDisplay } from '@/object-record/record-field/meta-types/hooks/useRelationFieldDisplay';
import { ObjectRecord } from '@/object-record/types/ObjectRecord';

import { useRelationField } from '../../hooks/useRelationField';

export const RelationFieldDisplay = () => {
const { fieldValue, fieldDefinition, maxWidth } = useRelationField();
const { fieldValue, fieldDefinition, maxWidth } = useRelationFieldDisplay();

if (
!fieldValue ||
!fieldDefinition?.metadata.relationObjectMetadataNameSingular
)
) {
return null;
}

return (
<RecordChip
Expand Down
Loading

0 comments on commit de9321d

Please sign in to comment.