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.

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

CloneButton should sanitize its rest props #2781

Closed
djhi opened this issue Jan 17, 2019 · 6 comments · Fixed by #2818
Closed

CloneButton should sanitize its rest props #2781

djhi opened this issue Jan 17, 2019 · 6 comments · Fixed by #2818
Assignees
Labels

Comments

@djhi
Copy link
Collaborator

djhi commented Jan 17, 2019

What you were expecting:
Using a <CloneButton /> inside an <Toolbar /> from an Edit view should not trigger any React warnings

What happened instead:
Lots of warnings about unkown props passed by the <Toolbar />

Steps to reproduce:
Using a <CloneButton /> inside an <Toolbar /> from an Edit view .

Related code:
https://github.com/marmelab/react-admin/blob/master/packages/ra-ui-materialui/src/button/CloneButton.js#L25

Environment

  • React-admin version: 2.6.1
  • Last version that did not exhibit the issue (if applicable):
@fzaninotto
Copy link
Member

I'm not sure we should address this case.

<CloneButton> is designed to be used in an edit view Actions component, not inside a Toolbar. The Toolbar is basically for submitting the form, not for going to another resource. If we support the usage of any button inside Toolbar, then we can't pass specific props that are useful to SaveButton only. Or we need to specifically sanitize the props injected by Toolbar, which seems like encouraging a bad pattern.

But since we don't explicit the purpose of Toolbar, let's address it.

@mnlbox
Copy link
Contributor

mnlbox commented Feb 20, 2019

@fzaninotto Please add this as a note to docs. I lost 3 hours until find this issue and fix my problem. 😞

<CloneButton> is designed to be used in an edit view Actions component, not inside a Toolbar. The Toolbar is basically for submitting the form, not for going to another resource.

/CC: @djhi

@djhi
Copy link
Collaborator Author

djhi commented Feb 20, 2019

@mnlbox Waiting for your PR 😉

@mnlbox
Copy link
Contributor

mnlbox commented Feb 20, 2019

I'm a little new in react-admin and I think it's a little soon for me but thanks for your suggestion. 🤔

@djhi
Copy link
Collaborator Author

djhi commented Feb 20, 2019

This is a documentation pull request. The documentation is located here and it seems you already have the text to put in it :)

@mnlbox
Copy link
Contributor

mnlbox commented Feb 20, 2019

@djhi I done it man: #2904 😉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants