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

Redundant repaints, because of ResizeObserver. It send events when when list got hiden with css display property. #823

Open
2 tasks done
jakerdy opened this issue Aug 31, 2024 · 6 comments

Comments

@jakerdy
Copy link

jakerdy commented Aug 31, 2024

Describe the bug

Hi, I have app that has tabbed view with multiple lists. When tab switched all elements in list disapears. After it went back to visible state, list got rerender.

Tab view implemented as such:

<div id="tab_view">
  <div id="tab_bar>...</div>
  <div id="viewport">
    <div id="tab_1" style="display:none">  <VirtualList ... /> </div>
    <div id="tab_2" style="display:block"> <VirtualList ... /> </div>
    <div id="tab_3" style="display:none">  <VirtualList ... /> </div>
  </div>
</div>

Things that happens:

  • Current active tab: #tab_1, 30 elements rendered within <VirtualList>
  • User clicks on tab associated with #tab_2
  • #tab_1 style property changed from block -> none
  • [Bad Thing Here:] <VirtualList/> within #tab_1 notified via resize observer, with event where all sizes === 0
  • All list elements inside #tab_1 got unmounted
  • #tab_2 style property changed from none -> block
  • <VirtualList/> within #tab_2 notified via resize observer with current view size
  • <VirtualList/> within #tab_2 populated with visible elements

The problem with this behaviour is that elements getting rerendered when user switch tabs. In most cases it is not a big problem, but in my case it make lot's of flickering, because list displays elements with images. And for some reason, images couldn't be displaye immediatly. Also it just useless work that browser should redo each time user show/hide lists.

Your minimal, reproducible example

https://stackblitz.com/edit/vitejs-vite-1ucf3g?file=src%2FApp.tsx

Steps to reproduce

  • Open stackblitz example
  • Click on tabs above listview
  • Check repaint counts
  • Check DOM in devtools (list contents specifically)

Expected behavior

It would be great, if handler of ResizeObserver checks if offsetParrent of observed element is not equal to null. It it is, that means that observed element is invisible, and all layouting/rerendering of list should be skipped.

Or at least, there should be some flag that allows to skip such rerenders.

How often does this bug happen?

Every time

Screenshots or Videos

No response

Platform

Windows 10
Chrome 128

tanstack-virtual version

solid-virtual 3.10.6

TypeScript version

5.4.5

Additional context

No response

Terms & Code of Conduct

  • I agree to follow this project's Code of Conduct
  • I understand that if my bug cannot be reliable reproduced in a debuggable environment, it will probably not be fixed and this issue may even be closed.
@piecyk
Copy link
Collaborator

piecyk commented Sep 4, 2024

Hi, yes it's know issue as the RO callback will be called when changing display.

One option could be to pass custom measureElement that will skip those updates

const updateSize = isTabVisible

measureElement: (
  element: TItemElement,
  entry: ResizeObserverEntry | undefined,
  instance: Virtualizer<TScrollElement, TItemElement>,
) => {
  const getSize = () => measureElement(element, entry, instance)

  if (updateSize) {
    return getSize()
  } else {
    const item = instance.measurementsCache[instance.indexFromElement(element)]

    return item ? item.size : getSize()
  }
},

Other using the enabled option, that should be the recommend way but current could be buggy, should be working soonish.

@jakerdy
Copy link
Author

jakerdy commented Sep 4, 2024

What do you mean by enabled option? Where is it and what consequences of using it?

Also, example with custom measureElement, doesn't do a thing. This one should be added to virtualizer? If so, what the logic begind it? It doesn't like it related to viewport, but more like about list elements which aren't resized when RO triggered.

@piecyk
Copy link
Collaborator

piecyk commented Sep 4, 2024

Also, example with custom measureElement, doesn't do a thing.

Ooo sorry you are right, before it was working, need to check it.

@ekoooo
Copy link

ekoooo commented Oct 22, 2024

hi, I encountered the same issue, and here is my solution.

updateActiveTab

const updateActiveTab = useCallback((val: string) => {
  emitter.emit('onBeforeTabChange', val);
  dispatch(updateAppState({currentActiveTab: val}));
  emitter.emit('onAfterTabChange', val);
}, [dispatch]);

emitter.ts

import mitt from 'mitt';

type Events = {
  onBeforeTabChange: string;
  onAfterTabChange: string;
};

const emitter = mitt<Events>();
export default emitter;

useVirtualizerScrollElementAvailable.ts

import { useEffect, useState } from "react";
import { useAppSelector } from "../store/hooks";
import emitter from "../util/emitter";

export function useVirtualizerScrollElementAvailable(tabKey: string) {
  const currentActiveTab = useAppSelector(state => state.app.currentActiveTab);
  const [isScrollContainerAvailable, setIsScrollContainerAvailable] = useState(currentActiveTab === tabKey);

  useEffect(() => {
    const onBeforeTabChangeHandler = (val: string) => {
      val !== tabKey && setIsScrollContainerAvailable(false);
    };
    const onAfterTabChangeHandler = (val: string) => {
      val === tabKey && setTimeout(() => setIsScrollContainerAvailable(true), 0);
    };

    emitter.on('onBeforeTabChange', onBeforeTabChangeHandler);
    emitter.on('onAfterTabChange', onAfterTabChangeHandler);

    return () => {
      emitter.off('onBeforeTabChange', onBeforeTabChangeHandler);
      emitter.off('onAfterTabChange', onAfterTabChangeHandler);
    };
  }, [tabKey]);

  return {
    isScrollContainerAvailable,
  };
}

Usage

const { isScrollContainerAvailable } = useVirtualizerScrollElementAvailable(tabKey);
const virtualizer = useVirtualizer({
  count: visiblePerks.length,
  // The virtual list remains intact when the tab is hidden.
  // Without this handling, the virtual list will be destroyed and re-rendered when the tab is shown again.
  getScrollElement: () => isScrollContainerAvailable ? listRef.current : null,
  estimateSize: () => 180,
  overscan: 1,
});

@piecyk
Copy link
Collaborator

piecyk commented Oct 23, 2024

Other option would be to skip updates of RO, something like this

const updateSizeRef = useLatestRef(updateSize ?? true)

const virtualizer = useVirtualizer({
  measureElement: (element, entry, instance) => {
    if (updateSizeRef.current) {
      return measureElement(element, entry, instance)
    } else {
      return notUndefined(instance.measurementsCache[instance.indexFromElement(element)]).size
    }
  },
  observeElementRect: (instance, cb) => {
    return observeElementRect(instance, rect => {
      if (updateSizeRef.current) {
        cb(rect)
      } else {
        cb(instance.scrollRect ?? rect)
      }
    })
  },
  ...options,
})

Maybe it should be build in into library?

@jakerdy
Copy link
Author

jakerdy commented Oct 27, 2024

Maybe it should be build in into library?

It would be great

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

No branches or pull requests

3 participants