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

[CardTitle] Unwanted warnings on CardTitle with actAsExpander #4761

Closed
hugocaillard opened this issue Jul 21, 2016 · 9 comments
Closed

[CardTitle] Unwanted warnings on CardTitle with actAsExpander #4761

hugocaillard opened this issue Jul 21, 2016 · 9 comments
Labels
bug 🐛 Something doesn't work component: card This is the name of the generic UI component, not the React module!

Comments

@hugocaillard
Copy link

Problem description

When using CardTitle as a card expander I get this React warning in the console:

Warning: Unknown props `actAsExpander`, `showExpandableButton` on <div> tag. Remove these props from the element. For details, see https://fb.me/react-unknown-prop
    in div (created by CardTitle)
    in CardTitle

And also on expandable CardText:

Warning.js:44 Warning: Unknown prop `expandable` on <div> tag. Remove this prop from the element. For details, see https://fb.me/react-unknown-prop
    in div (created by CardText)
    in CardText

It works just fine though.

Steps to reproduce

<Card>
  <CardTitle
    title='...'
    actAsExpander
    showExpandableButton
  />
  <CardText expandable>
  </CardText>
</Card>

Versions

  • Material-UI: material-ui@0.15.2
  • React: react@15.2.1
  • Browser: Chrome Version 51.0.2704.106
@ztittle
Copy link

ztittle commented Jul 22, 2016

I'm having this issue as well. Very annoying as we are trying to stomp out all react warnings.

@jsamr
Copy link

jsamr commented Jul 22, 2016

Same here ! material ui 0.15.2, chromium 51.0.2704.79, react 0.15.2

@arunkarottu
Copy link

I'm using a temporary fix that seems to be working for the time being. To fix the error on CardText, for instance, open node_modules/material-ui/Card/CardText.js.

Change line 59 from this:

_extends({}, this.props, { style: prepareStyles(rootStyle) }),

to this:

_extends({}, { style: prepareStyles(rootStyle) }),

@hugocaillard
Copy link
Author

hugocaillard commented Jul 25, 2016

Would you do a PR with that ?

EDIT: well, it actually doesn't really solve the problem, it's really temporary indeed

@siemato
Copy link

siemato commented Jul 30, 2016

From React documentation:
However, React currently strips all unknown attributes, so specifying them in your React app will not cause them to be rendered.

So it should throw the warning but the custom attribute shouldn't get into the DOM during production mode.

@arunkarottu: Removing the this.props not only gets rid of the custom DOM attributes but all of the valid DOM attributes except for the styles as well, which you might want to inject. As I see it, Material UI uses the property on the JS side but then we are injecting all props into the DOM, which is indeed not needed.

A better temporary solution would be a tempObject, get rid of the Key since it isnt't doing anything and pass the tempObject to the DOM.

I'll get into this tomorrow.

@siemato
Copy link

siemato commented Jul 30, 2016

okay, this did the job for me:

function omit(currentProps) {
    var propsCopy = Object.assign({}, currentProps);
    if (propsCopy.expandable != undefined){delete propsCopy.expandable;}
    return propsCopy;
}

and

_extends({}, omit(this.props), { style: prepareStyles(rootStyle) }),

this solution still is kind of ugly but it is less intrusive

@hugocaillard
Copy link
Author

It's just a solution to remove the warnings, and we have to do it manually for each props, i did that in CardTitle :

function omit(currentProps) {
  var propsCopy = Object.assign({}, currentProps);
  ['actAsExpander', 'showExpandableButton'].forEach(p => {
    delete propsCopy[p]
  })
  return propsCopy;
}

Still, it would be great to have a true fix

@siemato
Copy link

siemato commented Aug 1, 2016

this problem is linked to #4594 as well.

@mpontikes mpontikes mentioned this issue Aug 5, 2016
13 tasks
@oliviertassinari oliviertassinari added the component: card This is the name of the generic UI component, not the React module! label Dec 18, 2016
@oliviertassinari oliviertassinari changed the title Unwanted warnings on CardTitle with actAsExpander [CardTitle] Unwanted warnings on CardTitle with actAsExpander Dec 18, 2016
@oliviertassinari
Copy link
Member

That issue should have been fixed by #4675 and #4603. Thanks for the help.

@oliviertassinari oliviertassinari added the bug 🐛 Something doesn't work label Dec 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: card This is the name of the generic UI component, not the React module!
Projects
None yet
Development

No branches or pull requests

6 participants