-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
[Regression] CreateButton and ShowButton don't update on 'to' prop change #8671
Comments
Hi, and thank you for this very detailed report. I believe the idea of the If you need more control over the redirection, I believe the recommended way (which is actually not much more complex) would be to use a MUI Button directly instead. So IMO I don't think it would be desirable to have the That being said, I would like to have @fzaninotto or @djhi 's opinion to back me up on this one. |
Actually we even document this use-case in the docs here: https://marmelab.com/react-admin/Create.html#prefilling-the-form |
Thank you, that worked. However, those examples refer to MUI button which doesn't have neither |
@rkfg that's wierd because we use this in our own demos (which have TS enabled) without running into any issues. |
Oh okay, it was because the state: {
record: {
dept_id: filterValues?.dept_id,
},
}, looks better than this: search: `filter=${JSON.stringify({ dept_id: filterValues?.dept_id })}`, while both are functionally identical in this case. The former doesn't change the button link and less error-prone imo. But it only works with RA's button, not the MUI's. I honestly preferred the former behavior because I could customize just one aspect of this button without recreating it from scratch (label, icon, color, size etc.). |
I tend to agree. <Button
component={Link}
to={{
pathname: '/comments/create',
}}
state={{ record: { post_id: 123 } }}
>
Write a comment for that post
</Button> What is your react-router-dom version? We are on |
Ok, got it. I was passing
|
@rkfg Okay, so let's add back the |
Should be easy to add but I can't take a closer look until next week (or a week after that). If anyone's up to it feel free to fix it, or wait until then. |
Not sure if related at all but since |
@IAmVisco I do not see this warning in the simple example, although we are using the |
I thought it was an common issue seeing it was from MUI... Sorry for the confusion. I will prepare a reproducible sandbox and open a proper issue later. |
What you were expecting:
See #5014 and #5483 where it was fixed, the
CreateButton
component should rerender if theto
prop changes, it was fixed and then this check (prevProps.to === nextProps.to
) was removed in 4c9bfb4. If this is not officially supported now (the docs link mentioned in #5008 (comment) returns 404 now) how do I prefill forms using Create button?defaultValues
forSimpleForm
don't work here because I need those values from the List context (such as filter values) which isn't available in Create/Show/Edit views. Lifting them up seems like an overkill for such a trivial task. I realize thatto
is an object so it may cause excessive rerenders due to referential inequality if it's not memoized by itself, maybe it's better to compare its individual fields instead.What happened instead:
I'm not sure if it was intentional or by mistake, now I can't reliably prefill the form from filter values as described in #5008 (comment) by myself. The button "lags" by one render, i.e. it prefills the previous values and when I go back and forward it sets the actual value.
Steps to reproduce:
https://codesandbox.io/s/nervous-driscoll-1oetd4
The text was updated successfully, but these errors were encountered: