Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
import { useEffect } from 'react';
import { useContext, useEffect } from 'react';

import { type FieldMetadataItem } from '@/object-metadata/types/FieldMetadataItem';
import { TimelineActivityContext } from '@/activities/timeline-activities/contexts/TimelineActivityContext';
import { type EnrichedObjectMetadataItem } from '@/object-metadata/types/EnrichedObjectMetadataItem';
import { type FieldMetadataItem } from '@/object-metadata/types/FieldMetadataItem';
import { type FieldFilesValue } from '@/object-record/record-field/ui/types/FieldMetadata';
import { recordStoreFamilyState } from '@/object-record/record-store/states/recordStoreFamilyState';
import { useAtomFamilyStateValue } from '@/ui/utilities/state/jotai/hooks/useAtomFamilyStateValue';
import { useSetAtomFamilyState } from '@/ui/utilities/state/jotai/hooks/useSetAtomFamilyState';
import { FieldMetadataType } from 'twenty-shared/types';
import { isDefined } from 'twenty-shared/utils';

export const EventFieldDiffValueEffect = ({
Expand All @@ -22,22 +26,50 @@ export const EventFieldDiffValueEffect = ({
diffArtificialRecordStoreId,
);

const { recordId } = useContext(TimelineActivityContext);
const recordStore = useAtomFamilyStateValue(recordStoreFamilyState, recordId);

useEffect(() => {
if (!isDefined(diffRecord)) return;

let fieldValue: typeof diffRecord = diffRecord;

if (
fieldMetadataItem.type === FieldMetadataType.FILES &&
isDefined(recordStore) &&
Array.isArray(diffRecord)
) {
Comment on lines +35 to +41
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code path handles Array.isArray(diffRecord) (FILES diffs), but diffRecord is still typed as Record<string, any> | undefined, which doesn’t reflect arrays and forces repeated casts. Consider widening the diffRecord prop type (e.g., unknown/any or a union including FieldFilesValue[]) so TypeScript matches the shapes this effect supports.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was previous-exsiting. It depends on maintainer.

const currentFiles = Array.isArray(recordStore[fieldMetadataItem.name])
? (recordStore[fieldMetadataItem.name] as FieldFilesValue[])
: [];
const currentFileMap = new Map(
currentFiles.map((file) => [file.fileId, file]),
);

fieldValue = (diffRecord as FieldFilesValue[]).map((file) => {
const currentFile = currentFileMap.get(file.fileId);
if (isDefined(currentFile)) {
return currentFile;
}
return { ...file, isDeleted: true, url: undefined };
});
}

const forgedObjectRecord = {
__typename: mainObjectMetadataItem.nameSingular,
id: diffArtificialRecordStoreId,
[fieldMetadataItem.name]: diffRecord,
[fieldMetadataItem.name]: fieldValue,
};

setRecordStore(forgedObjectRecord);
}, [
diffRecord,
diffArtificialRecordStoreId,
fieldMetadataItem.name,
fieldMetadataItem.type,
mainObjectMetadataItem.nameSingular,
setRecordStore,
recordStore,
]);
Comment on lines +37 to 73
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The effect depends on the entire recordStore object, so any change to the record (even unrelated fields) will re-run the effect and call setRecordStore again. This can cause extra renders in the timeline. Consider narrowing the dependency to just the files field value you read (e.g., recordStore?.[fieldMetadataItem.name]) or memoizing the computed currentFileMap from that field only.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It doesn't make sense. After I debug it, if we changed some fields (like city/link), they all go to useEffect(). even though the dependency changes from recordStore to recordStore?.[fieldMetadataItem.name]. I think there is no difference for them.


return <></>;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -335,4 +335,5 @@ export type FieldFilesValue = {
extension?: string;
url?: string;
fileCategory?: FileCategory;
isDeleted?: boolean;
};
Original file line number Diff line number Diff line change
@@ -1,18 +1,37 @@
import { styled } from '@linaria/react';
import { t } from '@lingui/core/macro';
import { useId, useState } from 'react';
import { createPortal } from 'react-dom';

import { FileIcon } from '@/file/components/FileIcon';
import { type FieldFilesValue } from '@/object-record/record-field/ui/types/FieldMetadata';
import { getFileCategoryFromExtension } from '@/object-record/record-field/ui/utils/getFileCategoryFromExtension';
import { Chip, ChipVariant } from 'twenty-ui/components';
import { AppTooltip, TooltipDelay, TooltipPosition } from 'twenty-ui/display';
import { themeCssVariables } from 'twenty-ui/theme-constants';

const MAX_WIDTH = 120;

const StyledClickableContainer = styled.div<{ clickable: boolean }>`
cursor: ${({ clickable }) => (clickable ? 'pointer' : 'inherit')};
const StyledClickableContainer = styled.div<{
clickable: boolean;
isDeleted: boolean;
}>`
cursor: ${({ clickable, isDeleted }) =>
isDeleted ? 'not-allowed' : clickable ? 'pointer' : 'inherit'};
display: inline-flex;
min-width: 0;
`;

const StyledDeletedLabel = styled.span`
color: ${themeCssVariables.font.color.secondary};
font-size: inherit;
max-width: ${MAX_WIDTH}px;
overflow: hidden;
text-decoration: line-through;
text-overflow: ellipsis;
white-space: nowrap;
`;

type FileChipProps = {
file: FieldFilesValue;
onClick: (file: FieldFilesValue) => void;
Expand All @@ -24,7 +43,11 @@ export const FileChip = ({
onClick,
forceDisableClick,
}: FileChipProps) => {
const isClickable = forceDisableClick !== true;
const isDeleted = file.isDeleted === true;
const isClickable = forceDisableClick !== true && !isDeleted;
const tooltipAnchorId = useId();
const tooltipAnchorSelect = `[data-tooltip-anchor="${tooltipAnchorId}"]`;
const [shouldShowTooltip, setShouldShowTooltip] = useState(false);

const handleMouseDown = (event: React.MouseEvent): void => {
if (!isClickable) {
Expand All @@ -45,17 +68,43 @@ export const FileChip = ({
);

return (
<StyledClickableContainer
clickable={isClickable}
onMouseDown={handleMouseDown}
>
<Chip
label={file.label ?? ''}
maxWidth={MAX_WIDTH}
leftComponent={fileIcon}
variant={ChipVariant.Highlighted}
<>
<StyledClickableContainer
data-tooltip-anchor={tooltipAnchorId}
clickable={isClickable}
/>
</StyledClickableContainer>
isDeleted={isDeleted}
onMouseDown={handleMouseDown}
onMouseEnter={() => isDeleted && setShouldShowTooltip(true)}
onMouseLeave={() => setShouldShowTooltip(false)}
>
<Chip
label={isDeleted ? '' : (file.label ?? '')}
isLabelHidden={isDeleted}
maxWidth={isDeleted ? undefined : MAX_WIDTH}
leftComponent={fileIcon}
rightComponent={
isDeleted ? (
<StyledDeletedLabel>{file.label}</StyledDeletedLabel>
) : undefined
}
variant={ChipVariant.Highlighted}
clickable={isClickable}
/>
</StyledClickableContainer>
{isDeleted &&
shouldShowTooltip &&
createPortal(
<AppTooltip
anchorSelect={tooltipAnchorSelect}
content={t`File no longer exists`}
delay={TooltipDelay.shortDelay}
isOpen={true}
noArrow
place={TooltipPosition.Top}
positionStrategy="fixed"
/>,
document.body,
)}
Comment on lines +94 to +107
Copy link

Copilot AI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AppTooltip is only mounted after onMouseEnter sets shouldShowTooltip, but no isOpen prop is provided. With react-tooltip, mounting the tooltip after the hover event often means it won’t show on the first hover because listeners are attached after the event. Consider either rendering the tooltip whenever isDeleted is true (letting react-tooltip manage hover), or keep the conditional render but pass isOpen={shouldShowTooltip} (similar to OverflowingTextWithTooltip) so it reliably appears on first hover.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sense. Based on my testing, it does indeed display less promptly when I move the mouse quicly.

</>
);
};
Loading