search

nkbt / react-height

Component-wrapper to determine and report children elements height
MIT License
180 stars 27 forks source link

Invalid initial height in demos / first render #23

Closed tleunen closed 7 years ago

tleunen commented 7 years ago

This page http://nkbt.github.io/react-height/example/ shows an initial height of 18px. screen shot 2017-01-25 at 8 36 46 pm

But in reality, the content height for a paragraph is 70px, while the block is 152px.

The onHeightReady doesn't seem to be called on the first render, and I'm having the same issue in my app as well...

torleifhalseth commented 7 years ago

It looks like the const getElementHeight = el => el.clientHeight; ignores both padding and margin specified in css on the initial render.

Have you experienced this during the update of react-collapse @nkbt ? I see you removed react-height as dependency in your upcoming version. But you are still using clientHeight here.

nkbt commented 7 years ago

Yep it foes ignore margins and paddings. That is intentional.

Collapse does not use react-height since v3 anymore.

Example was not upgraded yet

torleifhalseth commented 7 years ago

Thank you for answering so fast @nkbt! 👍 Does react-collapse v3 include paddings and margin on initial render? From what i can see it uses the same logic here .

I was wondering if you ever tested performance of the collapse solution without react-motion only specifying height and using css transition?

Best regards

nkbt commented 7 years ago

Nope it does not do it either. By choice again. I dont want to solve css height problems at all. If you use collapse, make sure tour element is not affected by padd/marg. Style children elements or containers instead.

Css should be faster, tho never tested, but it does not solve give enough flexibility