Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix console errors #10111

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
+13 −18
Draft

Fix console errors #10111

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4fc79af
Remove forwardRef console error
martmull Feb 10, 2025
305e8fa
Fix motion() deprecated
martmull Feb 10, 2025
b6980aa
Remove wrong color setting css
martmull Feb 10, 2025
b862fcd
Remove missing field when updating cache
martmull Feb 11, 2025
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
14 changes: 6 additions & 8 deletions packages/twenty-front/src/modules/object-record/cache/utils/getRecordNodeFromRecord.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,16 +45,14 @@ export const getRecordNodeFromRecord = <T extends ObjectRecord>({
}

const nestedRecord = Object.fromEntries(
Object.entries(record)
.map(([fieldName, value]) => {
objectMetadataItem.fields
Copy link
Member

Choose a reason for hiding this comment

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

agree that we should pilot the utils by the metadata and not the record sturcture but might have a lot of side effects in this app, we need to test everything then

.map((field) => {
const fieldName = field.name;
const value = record[fieldName];
if (isDefined(recordGqlFields) && !recordGqlFields[fieldName]) {
return undefined;
}

const field = objectMetadataItem.fields.find(
(field) => field.name === fieldName,
);

if (isUndefined(field)) {
return undefined;
}
Expand All @@ -79,7 +77,7 @@ export const getRecordNodeFromRecord = <T extends ObjectRecord>({
getRecordConnectionFromRecords({
objectMetadataItems,
objectMetadataItem: oneToManyObjectMetadataItem,
records: value as ObjectRecord[],
records: (value || []) as ObjectRecord[],
Copy link
Member

Choose a reason for hiding this comment

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

i understand the changes too, I'm not agaist them but we should add tests on these

recordGqlFields:
recordGqlFields?.[fieldName] === true ||
isUndefined(recordGqlFields?.[fieldName])
Expand Down Expand Up @@ -144,7 +142,7 @@ export const getRecordNodeFromRecord = <T extends ObjectRecord>({
];
}
default: {
return [fieldName, value];
return [fieldName, value || null];
Copy link
Member

Choose a reason for hiding this comment

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

same here. And we should make sure that {} ends up being returned as null

}
}
})
Expand Down
2 changes: 1 addition & 1 deletion ...cord/record-show/record-detail-section/components/RecordDetailRelationRecordsListItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ const StyledClickableZone = styled.div`
justify-content: flex-end;
`;

const MotionIconChevronDown = motion(IconChevronDown);
const MotionIconChevronDown = motion.create(IconChevronDown);

type RecordDetailRelationRecordsListItemProps = {
isExpanded: boolean;
Expand Down
4 changes: 0 additions & 4 deletions packages/twenty-front/src/modules/object-record/record-table/components/RecordTable.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,6 @@ const StyledTable = styled.table`
border-spacing: 0;
table-layout: fixed;
width: 100%;

.footer-sticky tr:nth-last-child(2) td {
border-bottom-color: ${({ theme }) => theme.background.transparent};
}
`;

export const RecordTable = () => {
Expand Down
11 changes: 6 additions & 5 deletions packages/twenty-front/src/modules/ui/input/components/TextInputV2.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,9 +277,10 @@ const TextInputV2Component = (
);
};

const TextInputV2WithAutoGrowWrapper = (
props: TextInputV2WithAutoGrowWrapperProps,
) => (
const TextInputV2WithAutoGrowWrapper = forwardRef<
HTMLInputElement,
TextInputV2WithAutoGrowWrapperProps
>((props, _) => (
<>
{props.autoGrow ? (
<ComputeNodeDimensions node={props.value || props.placeholder}>
Expand All @@ -293,6 +294,6 @@ const TextInputV2WithAutoGrowWrapper = (
<TextInputV2Component {...props} />
)}
</>
);
));

export const TextInputV2 = forwardRef(TextInputV2WithAutoGrowWrapper);
export const TextInputV2 = TextInputV2WithAutoGrowWrapper;
Copy link
Member

Choose a reason for hiding this comment

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

don't we need the forwardRef anymore in some case? Not sure why we used it before?

Copy link
Member

Choose a reason for hiding this comment

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

I can see that AddressInput is passing a ref for example to handle the focus direclty

@bosiraphael has change the component to enable autogrow but this ref should be forwared into the subcomponent TextInputV2Component

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@charlesBochet so you think I should remove the forwardRef completely?

Loading