Fix scroll offset popping around during image load by putting explicit height back on images

[ara4n]

No description provided.

[ara4n]

fixes element-hq/element-web#1231

[ara4n]

@richvdh ptal

[richvdh]

This really shouldn't be necessary. Let me have a look at what's going on here.

[richvdh]

what's going on here is I'm an idiot.

[ara4n]

richvdh: i'm dubious about switching the closure refs to the null-guarded string refs in the popping fix - surely if the ref fails because the image doesn't exist on the initial pass, we'll still get the popping because the initial layout won't have a pinned image height?
or did you check that it didn't introduce popping?
The switch to string refs doesn't affect that though.
richvdh: howso? the ref is always null on componentDidMount, so surely the height fixup will return too late
whereas empirically the closure ref was set by the time componentDidMount ran

[richvdh]

The ref should not be null in componentdidmount.

On 24 March 2016 18:22:23 GMT+00:00, Matthew Hodgson [email protected] wrote:

richvdh: i'm dubious about switching the closure refs to the
null-guarded string refs in the popping fix - surely if the ref fails
because the image doesn't exist on the initial pass, we'll still get
the popping because the initial layout won't have a pinned image
height?
or did you check that it didn't introduce popping?
The switch to string refs doesn't affect that though.
richvdh: howso? the ref is always null on componentDidMount,
so surely the height fixup will return too late
whereas empirically the closure ref was set by the time
componentDidMount ran

---

You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub:
#248 (comment)

[ara4n]

ah ha! i see what was happening - because i was firing fixupHeight from componentDidUpdate it was firing seemingly before componentDidMount(?!), hence null ref. Have added warning logging to catch if the ref is ever null, removed some extraneous console logging, but i think we're good to go now. I'm going to declare this reviewed somewhere between the vdh/moi joint effort here...