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

Layout warns when wrapping Panel/NavDrawer/Sidebar in custom component #641

Closed
InfernoZeus opened this issue Jul 14, 2016 · 5 comments
Closed

Comments

@InfernoZeus
Copy link
Contributor

I'm trying to wrap the NavDrawer in a custom component which accepts a list of menu items. When doing this, Layout warns that it only accepts Panel as a child, with optional NavDrawer and Sidebar:

Warning: Failed prop type: Layout should have a Panel for a child, optionally preceded by a NavDrawer and/or followed by a Sidebar.
in Layout (created by Themed Layout)
in Themed Layout (created by App)
in div (created by App)
in App (created by RouterContext)
in RouterContext (created by Router)
in Router

My layout has a NavMenu component, which is causing the issue:

...
<Layout>
  <NavMenu items={menuItems}></NavMenu>
...
class NavMenu extends React.Component {
  render () {
    return <NavDrawer ...>{generatedMenuItems}</NavDrawer>;
  }
}

Is this likely to cause issues? If not, is there be some way to disable the warning?
@InfernoZeus InfernoZeus changed the title Layout warns when wrapping Panel/NavDrawer/Sidebar Layout warns when wrapping Panel/NavDrawer/Sidebar in custom component Jul 14, 2016
@tsega
Copy link

tsega commented Jul 21, 2016

I just had the same kind of error. I believe react-toolbox is explicitly expecting either a NavDrawer or Sidebar to precede a Panel, any other component would output that warning. You would also get a warning if you changed the order of the components, to something like Panel, Sidebar & then NavDrawer.

Note: Explicitly importing the components as follows:

import Layout from 'react-toolbox/lib/layout/layout';
import NavDrawer from 'react-toolbox/lib/drawer';
import Panel from 'react-toolbox/lib/layout/panel';

as opposed to importing them as in the React-Toolbox example:

import { Layout, NavDrawer, Panel, Sidebar } from 'react-toolbox';

would also give you the same warning.

@InfernoZeus
Copy link
Contributor Author

Seems to me that this error message is overly aggressive, treating the developer as a bit of an idiot.

@asprouse
Copy link
Contributor

I just ran into this message. There should be a better way than checking the component name:

https://github.com/react-toolbox/react-toolbox/blob/dev/components/layout/Layout.js#L12

One way to work around it would be to set your displayName to NavDrawer or Sidebar. However this becomes tricky if you are using any sort of higher order components such as redux connect. It also defeats the debugging utility of component names. I would propose removing this warning all together it is pretty visually obvious when the components are in the wrong order.

What do you think? I can put together a PR if you guys agree.

@InfernoZeus
Copy link
Contributor Author

I'm leaning towards removing the warning entirely. You should be able to
notice if your layout isn't working properly.

On 24 Aug 2016 3:48 pm, "Andrew Sprouse" notifications@github.com wrote:

I just ran into this message. There should be a better way than checking
the component name:

https://github.com/react-toolbox/react-toolbox/blob/
dev/components/layout/Layout.js#L12

One way to work around it would be to set your displayName to NavDrawer or
Sidebar. However this becomes tricky if you are using any sort of higher
order components such as redux connect. It also defeats the debugging
utility of component names. I would propose removing this warning all
together it is pretty visually obvious when the components are in the wrong
order.

What do you think? I can put together a PR if you guys agree.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#641 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AARUHBOA_PPFVaKbIVNVtPLldeIowKPDks5qjEu9gaJpZM4JMbgP
.

@javivelasco
Copy link
Member

Agree, if the warning it's annoying we can just remove it. It's a matter of styling and you may guess it's wrong as @InfernoZeus Enough warning in the docs IMO

asprouse added a commit to asprouse/react-toolbox that referenced this issue Aug 25, 2016
javivelasco added a commit that referenced this issue Aug 28, 2016
Remove overly strict children propType validation. fixes #641
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants