Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add isScrollingResetDelay option to Virtualizer #719

Merged
merged 1 commit into from
Apr 29, 2024

Conversation

piecyk
Copy link
Collaborator

@piecyk piecyk commented Apr 29, 2024

No description provided.

Copy link

nx-cloud bot commented Apr 29, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit d7d7111. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@piecyk piecyk force-pushed the feat/isScrollingResetDelay branch from 22dfe54 to ecd788b Compare April 29, 2024 08:35
@piecyk piecyk force-pushed the feat/isScrollingResetDelay branch from ecd788b to d7d7111 Compare April 29, 2024 09:03
@piecyk piecyk merged commit ab6ede8 into TanStack:main Apr 29, 2024
2 checks passed
@piecyk piecyk deleted the feat/isScrollingResetDelay branch April 29, 2024 09:08
@kieranm
Copy link

kieranm commented Apr 29, 2024

Thanks for this!

@kieranm
Copy link

kieranm commented Apr 29, 2024

Hey @piecyk

The code below doesn't seem to work because isScrollingResetDelay does not get updated with the new value after the effect runs (the virtualizer still thinks it's undefined)

  // State to detect if the current browser is Safari
  const [safari, setSafari] = useState(false)

  // Effect to check if the browser is Safari upon component mount
  useEffect(() => {
    setSafari(
      typeof window !== "undefined" &&
        Boolean((window as unknown as { safari: object }).safari),
    )
  }, [])

  const virtualizer = useVirtualizer({
    count: COUNT,
    getScrollElement,
    estimateSize: useCallback((index) => sizes[index], [sizes]),
    horizontal: true,
    initialOffset: 0,
    initialRect: { width: 2500, height: 2500 },
    overscan: 3,
    isScrollingResetDelay: safari ? 1100 : undefined,
  })

This code does work but it causes the whole virtualizer to re-initialize which causes a flicker on and off.


  const virtualizer = useVirtualizer({
    count: COUNT,
    getScrollElement,
    estimateSize: useCallback((index) => sizes[index], [sizes]),
    horizontal: true,
    initialOffset: 0,
    initialRect: { width: 2500, height: 2500 },
    overscan: 3,
  })

  // Effect to check if the browser is Safari upon component mount
  useEffect(() => {
    const isSafari =
      typeof window !== "undefined" &&
      Boolean((window as unknown as { safari: object }).safari)
    if (isSafari) {
      virtualizer.setOptions({
        ...virtualizer.options,
        isScrollingResetDelay: 1000,
      })
    }
  }, [])

Do you know how I can get the virtualizer to get the correct value without re-initializing? Thanks!

@piecyk
Copy link
Collaborator Author

piecyk commented Apr 29, 2024

Hey @piecyk

The code below doesn't seem to work because isScrollingResetDelay does not get updated with the new value after the effect runs (the virtualizer still thinks it's undefined)

  // State to detect if the current browser is Safari
  const [safari, setSafari] = useState(false)

  // Effect to check if the browser is Safari upon component mount
  useEffect(() => {
    setSafari(
      typeof window !== "undefined" &&
        Boolean((window as unknown as { safari: object }).safari),
    )
  }, [])

  const virtualizer = useVirtualizer({
    count: COUNT,
    getScrollElement,
    estimateSize: useCallback((index) => sizes[index], [sizes]),
    horizontal: true,
    initialOffset: 0,
    initialRect: { width: 2500, height: 2500 },
    overscan: 3,
    isScrollingResetDelay: safari ? 1100 : undefined,
  })

This code does work but it causes the whole virtualizer to re-initialize which causes a flicker on and off.


  const virtualizer = useVirtualizer({
    count: COUNT,
    getScrollElement,
    estimateSize: useCallback((index) => sizes[index], [sizes]),
    horizontal: true,
    initialOffset: 0,
    initialRect: { width: 2500, height: 2500 },
    overscan: 3,
  })

  // Effect to check if the browser is Safari upon component mount
  useEffect(() => {
    const isSafari =
      typeof window !== "undefined" &&
      Boolean((window as unknown as { safari: object }).safari)
    if (isSafari) {
      virtualizer.setOptions({
        ...virtualizer.options,
        isScrollingResetDelay: 1000,
      })
    }
  }, [])

Do you know how I can get the virtualizer to get the correct value without re-initializing? Thanks!

I see the issue, i think you can lift the safari up to global context as a method, then you don't really need an effect and ssr hydration should also work correctly

const isSafari = () =>
  typeof window !== 'undefined' &&
  Boolean((window as unknown as { safari: object }).safari)

const Foo () => {
  const virtualizer = useVirtualizer({
    count: COUNT,
    getScrollElement,
    estimateSize: useCallback((index) => sizes[index], [sizes]),
    horizontal: true,
    initialOffset: 0,
    initialRect: { width: 2500, height: 2500 },
    overscan: 3,
    isScrollingResetDelay: isSafari() ? 1100 : undefined,
  })
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants