-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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.
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
base: main
Are you sure you want to change the base?
Fix console errors #10111
Conversation
753c0f2
to
b6980aa
Compare
|
||
export const TextInputV2 = forwardRef(TextInputV2WithAutoGrowWrapper); | ||
export const TextInputV2 = TextInputV2WithAutoGrowWrapper; |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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?
Thank you! |
@@ -45,16 +45,14 @@ export const getRecordNodeFromRecord = <T extends ObjectRecord>({ | |||
} | |||
|
|||
const nestedRecord = Object.fromEntries( | |||
Object.entries(record) | |||
.map(([fieldName, value]) => { | |||
objectMetadataItem.fields |
There was a problem hiding this comment.
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
@@ -79,7 +77,7 @@ export const getRecordNodeFromRecord = <T extends ObjectRecord>({ | |||
getRecordConnectionFromRecords({ | |||
objectMetadataItems, | |||
objectMetadataItem: oneToManyObjectMetadataItem, | |||
records: value as ObjectRecord[], | |||
records: (value || []) as ObjectRecord[], |
There was a problem hiding this comment.
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
@@ -144,7 +142,7 @@ export const getRecordNodeFromRecord = <T extends ObjectRecord>({ | |||
]; | |||
} | |||
default: { | |||
return [fieldName, value]; | |||
return [fieldName, value || null]; |
There was a problem hiding this comment.
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
Should we change this from draft to open? |
Solves: