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

[Bug]: Performance issues with Select Custom Dropdown on big lists #474

Closed
Tracked by #111
khuang7 opened this issue May 4, 2021 · 27 comments
Closed
Tracked by #111

[Bug]: Performance issues with Select Custom Dropdown on big lists #474

khuang7 opened this issue May 4, 2021 · 27 comments
Assignees

Comments

@khuang7
Copy link

khuang7 commented May 4, 2021

What package within Headless UI are you using?

@headlessui/react

What version of that package are you using?

v1.1.1

What browser are you using?

Firefox

Reproduction repository

https://github.com/khuang7/headlessui-listbox-issue (yarn install then yarn start)

Describe your issue

I am trying to populate a pretty big list (500 elements) into the Simple Custom component found in TailwindUI for my project. The hover behavior when trying to select an option becomes quite slow when you try to move the mouse around on all the options. Also generating the list can sometimes be slow as well.

My workaround this problem is to replace Listbox.Options and Listbox.Option to ul and li html elements so that I can just call 'hover:' on the li elements which improves performance significantly. But I would prefer to keep using the HeadlessUI instead of HTML because I would lose a lot of the ARIA accessibility features.

I was wondering if there is a way to deal with the performance issues in the HeadlessUI. I could only pinpoint the issue being related to ListboxOptions and ListboxOption.

@mseele
Copy link

mseele commented May 5, 2021

Same for @headlessui/vue. It takes ~ 5 seconds to open a popup with around 1000 items.

Good solution would be to lazy load entries (like vuetify does), but i don't know if this is possible with "headless"

@craxrev
Copy link

craxrev commented May 10, 2021

Yes same for me, but I do also notice some jitters when scrolling (only when moving mouse over list items and triggering the active/selected states), I'm currently using it as a list of countries (249)

@ErickTamayo
Copy link

I managed to virtualize the options by using https://github.com/bvaughn/react-window, but then the accessibility (pressing up and down arrows) gets messed up.

hui-virtualized

@rovansteen
Copy link

Yeah currently all ListBox.Option values needs to be mounted because they register themselves within the context. Would be great if that could be changed so they are lazy so it can work together with virtualisation libraries.

@bezreyhan
Copy link

Yea this is a problem for us as well. It looks like all of the options are re-rendered when the user hovers over each option.

@adrianrudnik
Copy link

Just experienced the same with a language drop-down of 150 values, getting unusable results on mobile.

@doutatsu
Copy link

I thought it was an issue on my end, but looks like the component is just not well optimised. I need to render lists up to 1000 entries, which is unusable with this select

@RomainLanz
Copy link

RomainLanz commented Feb 8, 2022

Bumping this issue.

Having a country list of 240 items makes it nearly unusable (Using the Vue version).

@RobinMalfait RobinMalfait self-assigned this Feb 21, 2022
@RobinMalfait
Copy link
Collaborator

Hey! Thank you for your bug report!
Much appreciated! 🙏

Currently our components don't implement windowing / virtualisation so the performance issues are currently expected if you throw a lot of items at them. I took note of it to explore using Headless UI with tools like react-window as @ErickTamayo explored and once we have that figured out we will document it.

@Miladiir
Copy link

Miladiir commented Feb 22, 2022

I don't quite understand the reasoning behind closing this issue when this issue that is perceived as a bug by developers using headlessui is not remedied. Is there a public roadmap somewhere, where this issue could at least be linked so we can keep track of progress in any other way than subscribing to this issue?

I don't mean to sound rude or anything. We are quite happy using headlessui and so far, have not run into any trouble other than lack of performance. We are quite happy that these formidable components are provided to use.

@Zertz
Copy link
Contributor

Zertz commented Mar 21, 2022

On the React side at least, most of the slow-ness comes from compareDocumentPosition:

image

This is about 2000 items, and takes approximatively 8 seconds on this Core i7 laptop.

Digging a bit deeper, this function is indirectly used by <Listbox> through:

  1. adjustOrderedState
  2. sortByDomNode

I'm not an expert with algorithms but I'm pretty sure that sort is triggering a lot of expensive and duplicate work on the DOM.

@RobinMalfait I'm not familiar with what the code is trying to accomplish but it seems likely performance can be improved somehow before going down the windowing route?

Edit: On my M1 Pro, loading 2000 items "only" takes two seconds but the bottleneck appears to be the same. I ran the playground and changed this line to this:

let people = [...Array(2000)].map(() => `${Math.random()}`)

It does seem like it is exponentially slow: 3000 items takes... a long time and 4000 items causes Edge to hang.

Of course this is not a viable solution but if I return 0; in sortByDomNode, performance isn't great (it's 2000 DOM elements after all!) but it's acceptable.

👀 At least part of the the problem seems to have been introduced in this PR. So it seems like you were aware of the performance drawbacks. If windowing is indeed the only viable solution, would you accept a PR to do it internally or do you think it's best to do it in user-space?

@Zertz
Copy link
Contributor

Zertz commented Mar 23, 2022

It took me a while to get it just right but windowing works quite well with react-virtual!

Once I got everything set up, I was able to overcome most (all?) quirks by being somewhat generous with overscan. For a list of about 2000 items, 25 seems to do the trick.

@rovansteen
Copy link

@Zertz could you share how you made the listbox work with react-virtual?

@Miladiir
Copy link

It took me a while to get it just right but windowing works quite well with react-virtual!

Once I got everything set up, I was able to overcome most (all?) quirks by being somewhat generous with overscan. For a list of about 2000 items, 25 seems to do the trick.

Last time I tried something similar, keyboard navigation through items broke. I am also interested in your solution. Can you provide a minimal example?

@Zertz
Copy link
Contributor

Zertz commented Mar 24, 2022

Started putting together an example in the react playground, I'll try to create a PR this evening!

@briavicenti
Copy link

Just checking in @Zertz, were you still considering sharing a playground with a demo? I am really struggling to get react-virtual to work with the Listbox, would love to see what you did to combine them.

@Zertz
Copy link
Contributor

Zertz commented Apr 27, 2022

I'm sorry, I got caught up in a million things. I'll try to finish the demo shortly.

@leoyli-headsup
Copy link

Hitting a similar issue, would like to know if any one knows how to improve the perf issue 🙏

@aburtsev
Copy link

aburtsev commented Jun 8, 2022

@leoyli-headsup
Look at react-virtualized or react-virtualized for that. With one of these tools, you can easily solve the performance problem

@rovansteen
Copy link

@aburtsev you can't use these together with the listbox component because of the problem I outlined above.
#474 (comment)

@imrids
Copy link

imrids commented Jun 19, 2022

@Zertz Don't mean to be a bother, but any chance you'll get around to that example anytime soon? Would be highly appreciated.

@bzbetty
Copy link

bzbetty commented Jun 29, 2022

here's a quick demo of using it with tanstack react-virtual.

couple of notes

  • react-virtual currently doesn't support react 18 in strict mode
  • the virtual list has to be in a separate compoent not directly embedded in the <combobox.options> as it doesn't handle losing it's ref
import { Fragment, useRef, useState } from 'react'
import { Combobox, Transition } from '@headlessui/react'
import { ArrowDown, CommentIcon } from '@SvgList'
import classNames from 'classnames'
import { useVirtualizer } from '@tanstack/react-virtual'

type ListItem = {
    value: string | number
    text: string | undefined | null
}

type Props = {
    name?: string
    items?: ListItem[]
    label?: string
    onChange?: (event: { target: any; type?: any; }) => Promise<void | boolean>;
    children?: never
    className: string
};

export default function Select({ name, label, items, className, onChange }: Props) {
    const [selected, setSelected] = useState<ListItem>();
    const [query, setQuery] = useState('');
    const btnRef = useRef<HTMLButtonElement>(null);

    const filteredItems =
        query === ''
            ? items
            : items?.filter((item: ListItem) =>
                (item?.text ?? '')
                    .toLowerCase()
                    .replace(/\s+/g, '')
                    .includes(query.toLowerCase().replace(/\s+/g, ''))
            ) ?? [];


    var classes = classNames(
        "relative w-full cursor-default overflow-hidden text-left focus:outline-none focus-visible:ring-2 focus-visible:ring-white focus-visible:ring-opacity-75 focus-visible:ring-offset-2 focus-visible:ring-offset-teal-300",
        "mt-1 block w-full h-9 bg-background-grey border rounded text-base text-dark-grey shadow-sm placeholder-light-grey focus:bg-white focus:outline-none focus:border-accent-purple focus:ring-2 focus:ring-accent-light disabled:bg-light-grey disabled:text-light-grey disabled:border-light-grey disabled:shadow-none invalid:border-red-100 invalid:text-light-grey focus:invalid:border-red-100 focus:invalid:ring-red-100 leading-none border-light-grey",
        className,
    );

    return (
        <label className="block">
            <span className="block text-sm font-semibold text-dark-grey">{label}</span>
            <div className="relative mt-1">
                <Combobox value={selected} onChange={(v) => { setSelected(v); onChange?.({ target: { name, value: v?.value } }); }}>
                    <div className="relative mt-1">
                        <div className={classes}>
                            <Combobox.Input onFocus={(e: any) => { e.target.select(); if (!e.relatedTarget) { btnRef?.current?.click(); } }} className="w-full border-none py-2 pl-3 pr-10 text-sm leading-5 text-gray-900 focus:ring-0"
                                displayValue={(item: ListItem) => item?.text ?? ''}
                                onChange={(event) => setQuery(event.target.value)}
                            />
                            <Combobox.Button ref={btnRef} className="absolute inset-y-0 right-0 flex items-center pr-2">
                                <ArrowDown />
                            </Combobox.Button>
                        </div>

                        <Combobox.Options>
                            {filteredItems?.length === 0 && query !== '' ? (
                                <div className="relative cursor-default select-none py-2 px-4 text-gray-700">
                                    Nothing found.
                                </div>
                            ) : (
                                <VirtualizedList items={filteredItems ?? []} />
                            )}
                        </Combobox.Options>
                    </div>
                </Combobox>
            </div>
        </label>
    )
}

function VirtualizedList({ items }: { items: ListItem[] }) {
    const parentRef = useRef<HTMLDivElement>(null);

    const rowVirtualizer = useVirtualizer({
        count: items?.length,
        getScrollElement: () => parentRef.current,
        estimateSize: () => 35,
        overscan: 5,
    });

    return (
        <div ref={parentRef} className="absolute z-50 mt-1 max-h-60 w-full overflow-auto rounded-md bg-white py-1 text-base shadow-lg ring-1 ring-black ring-opacity-5 focus:outline-none sm:text-sm">
            <div
                style={{
                    height: `${rowVirtualizer.getTotalSize()}px`,
                    width: '100%',
                    position: 'relative',
                }}
            >
                {rowVirtualizer.getVirtualItems().map((virtualRow: any) => (
                    <Combobox.Option
                        key={virtualRow.index}
                        style={{
                            position: 'absolute',
                            top: 0,
                            left: 0,
                            width: '100%',
                            height: `${virtualRow.size}px`,
                            transform: `translateY(${virtualRow.start}px)`,
                        }}
                        className={({ active }) =>
                            `relative cursor-default select-none py-2 pl-2 pr-4 ${active ? 'bg-dark-grey text-white' : 'text-gray-900'
                            }`
                        }
                        value={items?.[virtualRow.index]}
                    >
                        {({ selected, active }) => (
                            <span className={`block truncate ${selected ? 'font-medium' : 'font-normal'}`}>
                                {items?.[virtualRow.index].text}
                            </span>
                        )}
                    </Combobox.Option>
                ))}
            </div>

        </div>
    );
}

@Dorianslavov
Copy link

Anyone managed to make a workaround for Vue for this performance issue ?

@hi-reeve
Copy link

got the some issue with vue, isn't this supposed to be top priority to do?

@hi-reeve
Copy link

got the some issue with vue, isn't this supposed to be top priority to do?

finally i fix this on vue, combining headlessui and UseVirtualListComponent from vueuse
works fine. i just hope this get built in since this is important issue.

@Tinoooo
Copy link

Tinoooo commented Aug 2, 2022

This issue should be reopened as it is not fixed for the Vue variant.

@RobinMalfait
Copy link
Collaborator

It's also not "fixed" for React. As mentioned in my comment here: #474 (comment) currently we don't support it and you can use other tools to work together with Headless UI to implement windowing.

It's not a bug, it's just a feature we didn't implement yet.

@tailwindlabs tailwindlabs locked as resolved and limited conversation to collaborators Aug 2, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests