[Bug]: Listbox Virtualizer is not actually virtualizing the list

[wonderbeel]

Environment

Developement/Production OS: Mac OS 14.6.1 (23G93)
Node version: 18.20.3 && 20.16.0
Package manager: [email protected] && 9.9.0
Radix Vue version: 1.9.5
Vue version: 3.5.3
Client OS: Mac OS 14.6.1 (23G93)
Browser: Chrome 128.0.6613.120

Link to minimal reproduction

https://stackblitz.com/~/github.com/wonderbeel/listbox-virtualizer-reproduction

Steps to reproduce

Open the reproduction and look at the number of items rendered in the virtual lists on the right (or with the browser inspector)

Describe the bug

We noticed that a component in our codebase was extremely slow despite using the virtualizer, as if it were not actually virtualizing the list: upon investigation, it was confirmed that this was indeed the case.

Digging into the Radix Vue codebase, it seems that a height is provided to the virtualizer (see Radix Vue ListboxVirtualizer.vue); however, looking at the TanStack virtual example, they also provide a fixed height to the element that wraps the virtualizer itself (see TanStack RowVirtualizerFixed.vue), also their documentation always explicitly mentions this requirement (see TanStack Virtual Documentation) whereas Radix does not mention it at all (see Radix Vue Listbox Documentation).

I am not sure if this is truly a bug because if we set a height on ListboxContent or ListboxGroup the virtualizer works as expected; however, there is no explanation of this in the documentation so in my opinion we have two potential fixes:

1. Update the documentation and examples to explicitly state that the wrapping element must provide a height
2. Update the virtualizer to actually wrap the component and provide a height

Let me know if you have other questions or need extra details 😃 .

Expected behavior

Listbox virtualizer should render 10~20 items not 1000

Context & Screenshots (if applicable)

No response

[zernonia]

Thanks for the ticket @wonderbeel . The ListboxVirtualizer is designed for dev to set it's own height, because there's multiple way to do so.

I think updating docs should avoid this mistake right? 😁

[wonderbeel]

Updating docs would be enough indeed, another option could be to try to solve it directly in the ListboxVirtualizer component adding an extra div and a property to decide the number of visible elements to force an height (we already have estimateSize so this option too could work).

Let me know which solution sounds more reasonable to you guys and I will try to open a MR today or tomorrow.

[zernonia]

Can you share more info about the latter solution? 😁

[wonderbeel]

My idea was something like this

<script lang="ts">
export interface ListboxVirtualizerProps<T extends AcceptableValue = AcceptableValue> {
  /** List of items */
  options: T[]
  /** Estimated size (in px) of each item */
  estimateSize?: number
  /** text content for each item to achieve type-ahead feature */
  textContent?: (option: T) => string
  /** Number of visible items **/
 visibleItems?: number 
}
</script>

<script setup lang="ts" generic="T extends AcceptableValue = AcceptableValue">
...
const visibleItems = computed(() => props.visibleItems ?? 10)
const estimateSize = computed() => props.estimateSize ?? 28)
...
</script>

<template>
  <div :style="{ height: `${estimateSize * visibleItems}px`, width: '100%', position: 'relative', overflow: 'auto' }">
    <div
      data-radix-vue-virtualizer
      :style="{
        position: 'relative',
        width: '100%',
        height: `${virtualizer.getTotalSize()}px`,
      }"
    >
      <component
        :is="is"
        v-for="{ is, item } in virtualizedItems"
        :key="item.index"
      />
    </div>
  </div>
</template>

(haven't tested it just a thing that I wrote on the fly looking at the component code)

[zernonia]

Thanks for the example @wonderbeel !

However since the estimateSize is just an estimate value, I believe giving dev the ability to set the height will be better.

[zernonia]

Added some docs on v2, can you help check and see if it make sense? 😁

[wonderbeel]

Hey @zernonia let me apologize actually, I wanted to open a PR for that but between COVID and other personal stuff I didn't find time to do it.
It totally makes sense what you wrote (new documentation looks amazing BTW), the only thing that I would do maybe is to make it more evident (maybe like here?) because I feel that a comment can be easily missed and debugging this problem can not be so immediate.