fix(web): invalidate position on ancestor scroll

[motiz88]

Summary:

Fixes #471.

As there is no native way to listen for changes to getBoundingClientRect(), the web implementation of Slider must cache and try to correctly invalidate the measurements. The code already does this on layout and when the viewport is resized, but not when the slider's bounding rect changes due to scrolling. This PR registers a capturing scroll event handler on document and invalidates the cache when an ancestor of the slider scrolls. This is probably not airtight (see this StackOverflow discussion) but is AFAICT correct.

For performance, it may be worth experimenting with unregistering the scroll (and resize) listener once the cache has been invalidated - this would avoid calling O(sliders on page) handlers on every frame while scrolling. This is a potentially more invasive change to Slider so I am publishing the PR without it.

Test Plan:

Added an example to example-web and tested it manually (see video).

Screen.Recording.2022-12-27.at.20.53.08.mov

[jspizziri]

@motiz88 you should probably subscribe to resize events the react-native way:

https://necolas.github.io/react-native-web/docs/use-window-dimensions/

[motiz88]

@jspizziri

1. This use of the resize event was pre-existing and I had no intention of changing it as part of this bugfix. I merely renamed onResize to invalidateContainerPosition to better communicate what it does (and reuse it in the onScroll handler).
2. IMO, given this is platform-specific code, use of DOM primitives is completely fine. It's also not directly interchangeable with useWindowDimensions (which would tie us to the React component lifecycle in a way that might not be the most correct/efficient for this use case)

[jspizziri]

If there's no way to do this in a more react-native idiomatic way, my opinion would be to do something like the following at the top of the file:

// @ts-ignore
const { addEventListener, removeEventListener } = document;

...
const RCTSliderWebComponent = React.forwardRef(
...
);

That would reduce the overall number of @ts-ignore comments in the code, which aren't ideal.

Perhaps it would be even better to create a unique hook (e.g. useOnDocumentScroll that receives a callback or something), that abstracts this functionality.

[motiz88]

That would reduce the overall number of @ts-ignore comments in the code, which aren't ideal.

I think the @ts-ignore issue (which, again, is pre-existing here) would be best solved by configuring TypeScript to properly typecheck uses of DOM APIs in .web.tsx files. This might be solved by adding /// <reference lib="dom" /> to the top of the file, but I haven't tried it.

[motiz88]

@jspizziri I've put up #475 to deal with the @ts-ignores comprehensively.

[BartoszKlonowski]

To me it looks very good, I'm happy to accept that. Also thanks so much for adding another example showing this specific scenario! 👍

@jspizziri Would it be possible for you to let us know if @motiz88's answers address your remarks? I don't want to merge something that still has some potentially open discussions.

[jspizziri]

@BartoszKlonowski LGTM!