search

GavinJoyce / ember-headlessui

https://gavinjoyce.github.io/ember-headlessui/
Other
92 stars 34 forks source link

:bug: <Listbox> should scroll option into view only once #173

Closed far-fetched closed 1 year ago

far-fetched commented 1 year ago

Failing test reference: click

fixes: #137

Issue:

In this repo, we vastly used the pattern "register children elements" in modifier. Then when in modifier callback, we use this.args.* it will be reevaluated when that args changes <-- this is desired behavior. But because of this mechanism, we have to be very careful what functions are called inside modifier. This bug is caused by calling this.scrollIntoView more than once. It affects the situation when user selects a new value (test) or new options are lazy loaded.

Solution:

It turns out that we can scroll listbox option into viewport not from root component, but delegate it to children (option). There is an only one moment when option should be scrolled (when is selected – what is being checked in constructor of option).

Before:

Lazy loaded options with "jumpy" effect when promise resolved:

https://user-images.githubusercontent.com/11621383/208042239-f82d6d06-7fe1-4bbb-aeef-be64e91cb9cd.mp4

After:

https://user-images.githubusercontent.com/11621383/208042199-cebf7586-679d-4130-9fbe-0475deb62da6.mp4

dmcnamara-eng commented 1 year ago

Looks solid 👊 Tested manually locally by mutating list of options and working well ✅