[Feature Request] add scrollElement to useWindowScroll

[acapro]

This is by far one of the best windowing tools I have tried, and would love to be able to use it, but there is one feature that is sadly still missing and I have not yet been able to find a work around this yet.

I have a more or less complex layout with multiple overflowing divs and scrollbars, and it would be an awesome feature if the useWindowScroll could be extended to support other wrapping scrollable (overflow) elements besides the window.

In react-virtualized there is a WindowScroller component that does just this, it takes an html element parameter called scrollElement (https://github.com/bvaughn/react-virtualized/blob/master/docs/WindowScroller.md)

Here is what I am trying to do:

[petyosi]

Hey,

What you explain is fairly simple to achieve. You can take a look at the commit which introduced the window scrolling behavior and extend it further. I am happy to accept a PR for that.

[maxhelsel]

I second this. My only issue with Virtuoso is the fact it doesn’t have support for assigning a custom parent scroll element like React Virtualized WindowScroller.

My use case is within modals and containers that have other elements within them; RV WindowScroller makes it easy and convenient to inject a scrollElement ref and carry on.

I forked this package and spent a few hours with it, but I’m not experienced whatsoever with packages and just couldn’t get a handle on everything going on within the src file. Unfortunately, I don’t have the time to familiarize myself enough to see it through.

If @petyosi or anyone else capable of implementing this feature stumbles upon this, here’s the link to where the window scrolling feature was introduced to Virtuoso: https://github.com/petyosi/react-virtuoso/compare/v1.2.7...v1.3.0.

My thinking was to provide an optional scrollElement prop for Virtuoso component, to be swapped within buildWindowScroller if the scrollElement props exists.

• Uses window (via buildWindowScroller within src/List)

  <Virtuoso 
  useWindowScroll={true} 
  />

• Uses custom scrollElement (via buildWindowScroller within src/List - swaps the customRef for the window). Figured it would be easiest just to try to slip the ref within the buildWindowScroller function and recycle all that logic.

  <Virtuoso 
  useWindowScroll={true}
  scrollElement={myCustomRef} 
  />

Then below is the function to build the Scroller element when useWindowScoll={true} - this code is copied and unchanged from src/List. I think here would be the place to substitute the scrollElement for the window if the scrollElement prop exists. You'd also need to adjust the useScrollTop and the viewport functions too, I believe (maybe some others?)

buildWindowScroller({ usePublisher, useEmitter, useEmitterValue }: Hooks) {
  const Scroller: Components['Scroller'] = React.memo(function VirtuosoWindowScroller({ style, children, ...props }) {
    const scrollTopCallback = usePublisher('windowScrollTop')
    const ScrollerComponent = useEmitterValue('ScrollerComponent')!
    const smoothScrollTargetReached = usePublisher('smoothScrollTargetReached')
    const totalListHeight = useEmitterValue('totalListHeight')
    const { scrollerRef, scrollByCallback, scrollToCallback } = useScrollTop(
      scrollTopCallback,
      smoothScrollTargetReached,
      ScrollerComponent
    )

    useIsomorphicLayoutEffect(() => {
      scrollerRef.current = window
      return () => {
        scrollerRef.current = null
      }
    }, [scrollerRef])

    useEmitter('windowScrollTo', scrollToCallback)
    useEmitter('scrollBy', scrollByCallback)
    return createElement(
      ScrollerComponent,
      {
        style: { position: 'relative', ...style, ...(totalListHeight !== 0 ? { height: totalListHeight } : {}) },
        ...props,
      },
      children
    )
  })
  return Scroller
}

Not sure if this is even a feasible approach, but seemed less invasive than building out an entirely new buildScroller function for the custom scrollElement ref.

So, that's where my heads at. Like I said, I don't have the experience with packages or time to spend to try to get this working, but I figured I could contribute here to help see this feature realized. It's the only thing Virtuoso lacks compared to React Virtualized, and prevents it (at least for me) from being the complete windowing solution

[Guiw5]

Im looking for the same feature here! would love to hear any news about this

Meantime @acapro how did you figure it out? I have the exact same use case, Im thinking wrapping this with the window-scroller and setting it but im not sure if there is any api to handle scroll externally and use that as onScroll prop or whatever.. have you found a workaround?

pd: great package absolutely!

[Guiw5]

@maxhelsel Im not familiarized whit rxjs, but this is the where the scroller is pointing to window component

what about setting it like this?

useIsomorphicLayoutEffect(() => {
      scrollerRef.current =  window;
      return () => {
        scrollerRef.current = null
      }
    }, [scrollerRef, scrollElement])

// and maybe setting it like::
useIsomorphicLayoutEffect(() => {
      scrollerRef.current =  scrollElement ? scrollElement.current : window;
      return () => {
        scrollerRef.current = null
      }
    }, [scrollerRef, scrollElement])

this would be more reusable way with a new ElementScroller based on the prop

const OuterScroll = scrollElement ? ElementScroller : WindowScroller
const TheScroller = useWindowScroll ?  OuterScroll : Scroller
const TheViewport = useWindowScroll ? WindowViewport : Viewport

// and renaming useWindowScroll prop to useOuterScroll or just adding it to not break the current API

In this scenario the ElementScroller should be a almost a copy from the buildWindowScroller Im not aware for the specific changes but conceptually the element should be the main case and would point to the window in the particular case where there is no element to use.

[maxhelsel]

@Guiw5 Yeah, I think that's very similar to how I tried it a few months ago:

useIsomorphicLayoutEffect(() => {
      scrollerRef.current =  scrollElement ? scrollElement.current : window;
      ...

Furthermore, like you said, I figured declaring the "ElementScroller" variable prior to running it though those buildWindowScroller functions was the way to go.

something like:

const ScrollingContainer = scrollElement && scrollElement.current ? scrollElement.current : window;

If I recall correctly, that wasn't enough to get it working on its own. There might have been other window-specific functions going on throughout the code that complicated things. It was a while ago so I can't be certain.

I really hope this becomes a feature because it's just about the only thing keeping this package from becoming the absolute best virtual solution.

@petyosi and thoughts? You had said earlier in this thread back in March that this feature would be "fairly simple to achieve" so I'm just wondering if you'd have the time to take a stab at it since you're completely familiar with the inner workings of the package.

[petyosi]

I'm afraid I'm very short on time recently. Also, it's hard to prioritize this since it is not related to the use cases of my day job.

[Guiw5]

thanks anyway, maybe someone else @stephenh @cryptoethic @exneval?

[Guiw5]

any news?

[maxhelsel]

Even though @petyosi doesn't have the time to make it happen, it's a good sign he's referencing this feature req. There's clearly a level of demand for it, and we're finally getting some traction and traffic here. It would be a godsend if someone familiar with this package would try to get something going on this

[bhargavshah]

This is much needed. If possible please prioritize!

[petyosi]

@bhargavshah are you willing to sponsor the development of the feature?

[cvanem]

@petyosi Do you still need a PR for this? I have something working but I noticed you have been working on a custom scroller example recently?

[petyosi]

Yes, a PR would be welcome.

[petyosi]

@maxhelsel @Guiw5 @acapro - please take a look at the #556 from @cvanem. The examples look right to me.

[petyosi]

Implemented in #556