bvaughn / react-virtualized-auto-sizer

Standalone version of the AutoSizer component from react-virtualized
MIT License
613 stars 82 forks source link

The container's width (or the resize event) does not update if the parent has a scaling animation #49

Closed anhdd-kuro closed 1 year ago

anhdd-kuro commented 1 year ago

The container width can only be obtained from parent's initial width. If the container is resized using CSS transform scale, the resize event is not be triggered (which may be the expected behavior), resulting in the width not being updated accordingly However, the AutoSizer component of react-virtualized seems to work well in this scenario ( tested with react-base-table which uses react-virtualized ) It is possible that listening to the transitionend_event event may help to address this issue ?

bvaughn commented 1 year ago

However, the AutoSizer component of react-virtualized seems to work well in this scenario

I think this must be a mistake or a coincidence where something else triggered a re-render. There are very few differences between the relevant code in this library and the code in react-virtualized, (both of which are based on this code.

Animations are something I haven't put much thought into for this library. Maybe a "transitioned" event would help (though only resizing at the end of an animation might also not be good enough for some use cases?)

Would you be interested in putting together a PR (or even a proof of concept) for this? I don't have a use case for it (or even a repro).

anhdd-kuro commented 1 year ago

I'll give it a try! I discovered this problem when using this library: https://auto-animate.formkit.com/. The table is rendered by triggering a checkbox ( show / hide ), When it is displayed, the library adds an animation that scales from 90% to 100% , auto sizer stop width at 90%

ivan-palatov commented 1 year ago

We are currently experiencing the same issue in our project if we update react-virtualized-auto-sizer from 1.0.8 to the latest version. On 1.0.7 everything works as expected, on 1.0.8 and above the lists look like this image

Edit: more correct information on versions

bvaughn commented 1 year ago

A repro would certainly be helpful! 😄

ivan-palatov commented 1 year ago

@bvaughn here is a basic reproduction of the bug Codesandbox. Red box should fill the popover, but doesn't.

bvaughn commented 1 year ago

Thanks for the repro!

I've recorded a Replay of both versions:


Looks like, in both cases, the children function is only called with a single set of dimensions– but in 1.0.9 they're smaller than you'd expect given the Box size.

The significant difference seems to be that 1.0.7 reads the parent node's size using offsetWidth and offsetHeight:

var _height = _this._parentNode.offsetHeight || 0;
var _width = _this._parentNode.offsetWidth || 0;

But 1.0.9 uses getBoundingClientRect:

const rect = this._parentNode.getBoundingClientRect();
const height = rect.height || 0;
const width = rect.width || 0;

I think the latter is more accurate – but the trouble is that there's no update fired when scaling changes or finishes.


I think react-virtualized-auto-sizer matches ResizeObserver spec behavior in that:

Observations will not be triggered by CSS transforms.

I wonder what folks normally do for this?

I considered adding an "animationstart" or "transitionstart" listeners and then using a RAF to poll for updates– but that only works if the parent node itself fires the event, and it doesn't in this case (because the transition/animation happens on the parent node, see 7bb5abd1-7752-4c0c-81b5-af2e60efed35).

I could maybe update the children function to accept both width/height and offset width/height, and let the caller choose which to use, but beyond that I'm not sure what to do here.

ivan-palatov commented 1 year ago

I think returning 2 different things could be okay. Or maybe add 2 different component exports: one that uses getBoundingClientRect and the other that uses offsets. Those could be just thin wrappers. But both of these seem hacky tbh.

bvaughn commented 1 year ago

Hmm. I don't think two components/exports seems like a great solution. Either adding some additional props to the children function, or maybe adding a prop to AutoSizer to control the value... but really it seems like figuring out the animation/transition event would be better.

bvaughn commented 1 year ago

I think I'm going to just treat this as a regression and rervert to the prior behavior.

I will add new params to the children function for scaled with and height, but you can ignore those for your use case (and probably for most use cases– except maybe for positioning tooltips or popups).

bvaughn commented 1 year ago

Fix has been published in 1.0.12

Thanks for the repro and the dialog