Adds a new updateOffsetOnChange prop

okonet / react-container-dimensions

Wrapper component that detects element resize and passes new dimensions down the tree. Based on https://github.com/wnr/element-resize-detector

MIT License

229 stars 26 forks source link

Adds a new updateOffsetOnChange prop #26

Closed alexanderwallin closed 5 years ago

alexanderwallin commented 7 years ago

As discussed in #24, the top and left passed on to children components are not updated when they are changed, for instance when scrolling the page.

This is a quite naive solution, where we basically check getBoundingClientRect() on every frame. I couldn't come up with a reasonable alternative, since there are a ton of ways an element's offset can be changed. If you have an idea, that'd be great!

If we implement this, it should probably come with a friendly warning about the implementation.

alexanderwallin commented 7 years ago

Oh, and I didn't intend to address linting issues or indentations, but was enforced by lint-staged. 😊

okonet commented 7 years ago

I don't really like doing this on every frame. This will trash the layout and will probably have a severe impact on performance. Did you consider using something like https://developer.mozilla.org/en-US/docs/Web/API/MutationObserver?

The more I think about this, the lesser I think this should be a part of this library, especially if this might have a performance drawbacks. If this issue can't be solved in a performant manner, I'll probably decline this PR. What do you think of it?

alexanderwallin commented 7 years ago

I don't like this either. This solution fits better in a separate package. I'll have a look at MutationObserver tho!

alexanderwallin commented 7 years ago

Okay, so neither MutationObserver or any of the other newer *Observer APIs work in this situation, since they watch DOM attributes or objects, whereas we are spying on the result of a function call.

We could do this in a couple of other ways:

Provide a scrollTargets prop, where each target gets a scroll event listener that calls onResize().
Expose a new component (<ContainerDimensionsWithScrollUpdates>, or perhaps something less verbose) that has the scrollTargets functionality above, but renders <ContainerDimesions>, stores a ref on it and calls this.storedRef.onResize().
Expose a prop that simply triggers a call onResize() whenever it changes.

If none of these methods make sense here, I'm happy to just create a new lil lib and provide it through there. 😊

okonet commented 7 years ago

To be honest, I think the best option would be to have a separate HoC or HoF that is only concerned about scroll position. Based on that it's easy to calculate the realt top/left offsets. What do you think?