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

Warning: "Wrapper div has no height or width" #35

Open
alampros opened this issue Jul 12, 2016 · 23 comments
Open

Warning: "Wrapper div has no height or width" #35

alampros opened this issue Jul 12, 2016 · 23 comments
Milestone

Comments

@alampros
Copy link

There doesn't seem to be any way around this warning (it even throws in the example). It is thrown on the first render because the state is not yet updated after componentDidMount.

Any ideas on how to avoid or silence it?

@fkrauthan
Copy link

I have the same issue

@killtheliterate
Copy link

This seems to happen intermittently. On some refreshed, I get a lot of this error and no render.

@zeroasterisk
Copy link

Right... I think this is not a very useful warning... if we end up with 0 for height and width... we know it didn't find the size.

This is a "first run" issue - the wrapper hasn't rendered, so it can't get the size of itself, to update it's own state.

I propose you remove the warning.

alampros added a commit to alampros/react-dimensions that referenced this issue Jul 26, 2016
@beheh
Copy link

beheh commented Aug 11, 2016

Fixed in #43.

@gmaclennan gmaclennan added this to the 2.0.0 milestone Aug 12, 2016
@killtheliterate
Copy link

I get this warning no matter what. Here's a paste that I thought would work, but doesn't:

// vendor
import React from 'react'
import Dimensions from 'react-dimensions'

// lib
import Foo from './foo'

class Wrapper extends React.Component {
    render() {
        return (
            <Foo
                containerWidth={ this.props.containerWidth }
                containerHeight={ this.props.containerHeight }
            />
        )
    }
}

const EnhancedWrapper = Dimensions()(Wrapper)

export default class Bar extends React.Component {
    render() {
        return (
            <div>
                    <div style={{height: '100px'}}>
                        <EnhancedWrapper />
                    </div>
            </div>
        )
    }
}

@killtheliterate
Copy link

I should add that resizing the browser window causes the widths to be calculated, and the component to render.

@jharris4
Copy link
Contributor

I've been noticing this issue too, realized that the width/height state props are undefined on first render because they aren't set in the componentWillMount lifecycle function.

Looks like this is/will be fixed in v2.0 ?

@gmaclennan
Copy link
Member

Yes, should be. If you can test let me know.

@jharris4
Copy link
Contributor

jharris4 commented Sep 1, 2016

2.0.0-alpha1 is working great for me. Had to refactor a little bit to move some styles from the wrapper div to the parent, but that's to be expected, and actually cleaner in my opinion!

@jharris4
Copy link
Contributor

jharris4 commented Sep 1, 2016

Oh, I just realized that the wrapper div is still present in 2.0.0-alpha despite the fact that its style cannot be changed.

the wrapper div currently has this hard-coded style:

{
    overflow: 'visible',
    height: 0,
    width: 0
}

Which is breaking my layout further down the DOM that tries to do width: 100%. I guess I can go and replace it everywhere with width={containerWidth} etc, but I'd be curious to know why you're even using a wrapper div at all in the new version, let alone setting inline styles on it...

@gmaclennan
Copy link
Member

In order to reference the parent div, I need to reference it 'from' somewhere. Because of the way React works I can't reach down into components that are children of react-dimensions to find a div, and then get the parent, so I need to create that wrapping div and then reach through the DOM for its parent. It's the only way, I think. I spent some time seeing if it could be done without the div but impossible. The hard-coded style is an attempt to make it 'invisible' to layout, but I don't think that works. Maybe it either needs to be customizable or there should be a different hard-coded style?

@jharris4
Copy link
Contributor

jharris4 commented Sep 2, 2016

I was just about to edit my comment because after thinking about it, I realized the same thing about needing the DOM reference from somewhere.

I was going to suggest https://facebook.github.io/react/docs/top-level-api.html#reactdom.finddomnode, not sure if you tried that? Once the child component is mounted by the parent HOC it might be pretty easy...

That said, I did work around the width/height being set to 0 on the wrapper div by setting an explicit height=containerHeight etc on the style of the wrapped div, and it seems to be working properly on chrome/safari/firefox/ie11/edge

@jharris4
Copy link
Contributor

jharris4 commented Sep 2, 2016

I just tinkered a bit with eliminating the wrapper div, and was able to get a reference to the parent div directly from the reference to the wrapped component's reference. Basically, I changed the render function to this:

if (containerWidth || containerHeight) {
    return <ComposedComponent
        {...this.state}
        {...this.props}
        updateDimensions={this.updateDimensions}
        ref='wrappedInstance'
      />
}
else {
    return <div style={wrapperStyle} ref='wrappedInstance'></div>;
}

And then changed the componentDidMount function to this:

if (!this.refs.wrappedInstance) {
    throw new Error('Cannot find wrapped instance')
}
this._parent = this.refs.wrappedInstance.parentNode
this.updateDimensionsImmediate()

Seems to be working pretty flawlessly, at least for my use cases. The important things to note though, are that you need a DOM node present to get the ref for it (duh), so I had to return the empty div in the else block of the render for that to work. I used wrapperStyle to hide that empty div, but that should probably be styled to display: hidden or something similar to prevent FOUC type issues. Also, this whole thing will break if the wrapped component returns false from its render function, but I think that's acceptable to document and inform people that they can't do that if they're using this HOC

@joeyfigaro
Copy link

Any updates on this @digidem?

@jharris4
Copy link
Contributor

That reminds me, I have a fork of this project where it's working without any wrapper div (as described in my last comment). Haven't created a PR for it, but anyone interested can find it here: https://github.com/jharris4/react-dimensions

@jdelafon
Copy link

The fix is really worth being backported to 1.x (1.3.1? -> npm).

@odub
Copy link

odub commented May 17, 2017

A 1.3.1 with this fix would be great. I'd be happy to help.

@jharris4
Copy link
Contributor

fwiw, I created a simplified sizer package that I'm using to do what react-dimensions does:

https://github.com/jharris4/react-sizer

@ethankong113
Copy link

Is this library still actively maintained? I could open a PR to remove this message for 1.X versions. Seems like a lot of people are requesting for the same patch.

@gmaclennan
Copy link
Member

I haven't had time to maintain it, we don't actually use it any more, see the README:

DEVELOPMENT STATUS: I'm not really using this any more since for grids/tables in React I've switched from fixed-data-table (which I was using this for) to react-virtualized which includes similar functionality to this with the Autosizer. I'm happy for someone else to take this module on.

I'd be happy to add maintainers, or transfer the repo / npm name.

@sasadaisy
Copy link

this requestion has any solution?

@sasadaisy
Copy link

@gmaclennan

@gmaclennan
Copy link
Member

I don't think so, it's unlikely that this is getting any updates, I'm using https://bvaughn.github.io/react-virtualized/#/components/AutoSizer for this functionality now.

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