Closed michalczaplinski closed 6 years ago
@michalczaplinski, thank you for this report.
It is not always obvious how to run your reproduction code, I never tested things in Mocha/JSDom. I tried to set up these, but ended up with different errors and I today have no time to dive there. (ERROR: It looks like you called mount() without a global document being loaded.
)
Could you please make a little demo repository for me?
@ZitRos I have created a reproduction here: https://github.com/michalczaplinski/repro-xmasonry-bug
I based it off create-react-app
. I've been using react@15.6
in my own code, but I can also see the issue in the reproduction which is using react@16.1
.
Let me know if you need any more information!
Thank you very much!
So the XMasonry, being in "browser" environment, renders 2 times before rendering the actual result, see this section in docs.
This is exactly the reason why XMasonry have no children at the very beginning.
Here's how it can pass your test:
// frame 0: XMasonry measures width to determine width per block and number of columns (0 children)
requestAnimationFrame(() => // frame 1: XMasonry measures heights of containers (2 children)
requestAnimationFrame(() => // frame 2: actual result (2 children)
expect(masonry.children().length).toEqual(2)
));
In the "worst" case, XMasonry rendering may even take 5 frames, and here's why:
On frame 3 the friendly scrollbar may appear, and hence container width changes. So XMasonry has nothing but to repeat 2-3, putting a limit at new width not to enter the infinite loop (render - scrollbar disappears - render - it appears again - and so on).
But for any tests, scrollbar doesn't matter much, so, usually, 3 frames is OK. I believe that in React 16, with their "delayed rendering" things may change (but I think, unlikely).
I put a hard work towards making XMasonry as easy for use as possible. Many other masonry implementations require block widths/heights to be pre-defined, but XMasonry does not, while providing a way to create blocks of different width (in columns). Resulting use case is easy indeed, but it has a couple of technical features I mentioned in "Under the Hood" section 😃
Let me know if this helps!
@michalczaplinski, any feedback? Thanks!
hey @ZitRos ! I've written you a response a while back, but it must have not been posted... 🤷♂️
That all makes a lot of sense and thanks for the detailed explaination! 👍
My only feedback would be to perhaps make it a bit more explicit in the docs in case people like me are not able to deduce the above from the "Under the Hood" section 😄
Good point, thanks! Were you been able to set up your tests? Please close the issue if so, thank you!
I have not finished setting up the tests, unfortunately, but I your explanation closes this issue, I believe!
Thanks Michael, it does, indeed: I've tested my code at your example :)
Happy New Year!
Reproduction:
The output (I run the tests with jest):
As you can see I am also trying to log the rendered tree to the console, but it looks like this:
and does not render the
<XBlock>
components or the children