digidem / react-dimensions

[Looking for maintainers]
http://lab.digital-democracy.org/react-dimensions/
450 stars 77 forks source link

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

Open alampros opened 8 years ago

alampros commented 8 years ago

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 commented 8 years ago

I have the same issue

killtheliterate commented 8 years ago

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

zeroasterisk commented 8 years ago

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.

beheh commented 8 years ago

Fixed in #43.

killtheliterate commented 8 years ago

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 commented 8 years ago

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

jharris4 commented 8 years ago

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 commented 8 years ago

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

jharris4 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

Any updates on this @digidem?

jharris4 commented 8 years ago

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 commented 8 years ago

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

odub commented 7 years ago

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

jharris4 commented 7 years ago

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

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

ethankong113 commented 7 years ago

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 commented 7 years ago

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 commented 4 years ago

this requestion has any solution?

sasadaisy commented 4 years ago

@gmaclennan

gmaclennan commented 4 years ago

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.