-
-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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 ArrayInput issue with initialValue #6932
Conversation
f431898
to
ab27b0e
Compare
const { input, meta } = useFinalFormField(finalName, { | ||
initialValue, | ||
initialValue: get(record, source, initialValue), |
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.
If I may, this makes sense to me.
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.
Yes, we were actually lucky it worked before XD
@@ -67,6 +67,7 @@ const FormWithRedirect = ({ | |||
sanitizeEmptyValues: shouldSanitizeEmptyValues = true, | |||
...props | |||
}: FormWithRedirectProps) => { | |||
const record = useRecordContext(props); |
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.
it feels very weird that you read a context to create it again.
What I understand is that you want to wrap a RecordContext if a record param is passed. That's what OptionalRecordContextProvider does in next. Maybe the right fix is to use that new component?
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.
That would be the right fix but I didn't want to introduce a new component
); | ||
}} | ||
/> | ||
<RecordContextProvider value={{ id: 1, views: 0 }}> |
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 don't understand why you need that change as you're adding a context in FormWithRedirect
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.
Forgot to fix this test after introducing the RecordContext in FormWithRedirect
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.
Not really important though
This pull request is being automatically deployed with Vercel (learn more). react-admin – ./examples/simple🔍 Inspect: https://vercel.com/marmelab/react-admin/4BnweNZvzLaF4pnu6A32YioLMppD react-admin-storybook – ./🔍 Inspect: https://vercel.com/marmelab/react-admin-storybook/GhcAjpDj7WjW12wYnSkwv6ijbRPz |
f63b6b9
to
d58152c
Compare
This issue happens because of how final-form handles the initialValue prop. It takes precedence over the form level initialValue that we set to the record when editing one. However, as ArrayInput dynamically adds new final-form fields, their initialValue overrides the value they got from the form.
The solution is to update the useInput hook so that it ignores the initialValue if a record is available through the RecordContext and has a value for this field.
Closes #6908