GavinJoyce / ember-headlessui

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

✨ <Listbox> Add an argument to prevent automatically scrolling options into view #137

Closed dmcnamara-eng closed 1 year ago

dmcnamara-eng commented 2 years ago

✨ What Changed & Why

Current behaviour:

<Listbox> always scrolls the selected option into view when the listbox opens or when the selected option changes.

New behaviour:

In some situations (e.g. showing extra content such as a search box inside the dropdown), we do not want to scroll the selected option into view (e.g. the search box should always be visible when the listbox opens).

This change adds a preventScrollToOption argument to listbox that prevents this behaviour if configured to be true.

GavinJoyce commented 2 years ago

Hi @dmcnamara-eng, thanks for this.

Do you know if there an equivalent API to this in the React/Vue implementations?

dmcnamara-eng commented 2 years ago

Do you know if there an equivalent API to this in the React/Vue implementations?

Hey @GavinJoyce as far as I can see there is no equivalent in the react/vue implementations.

I haven't tried to reproduce this bug there, but it's possible the bug exists there too if the listbox is extended in this way.

far-fetched commented 2 years ago

Hey everyone. I encountered a similar problem when I tried to add lazy loading for items in the list.

https://user-images.githubusercontent.com/11621383/162381833-699667ae-e674-4dd9-967b-444c7128ba7b.mp4

The way how I render list is simple:

{{#each Loader.data as |client|}}
  <listbox.Option @value={{client.id}}>
      {{client.firstName}}
      {{client.lastName}}
  </listbox.Option>
{{/each}}

So as you can notice any time when a new portion is loaded – list is scrolled into a selected item. I checked code, and we use scrollIntoView in only 2 places: what causes my issue is when new item is added as a new option (is register). Each time when I load more data, I created new array with previously loaded items + new. So each does not track correctly by default and does not reuse existing nodes. My solution was simple, add key:

{{#each Loader.data key="id" as |client|}}
  <listbox.Option @value={{client.id}}>
      {{client.firstName}}
      {{client.lastName}}
  </listbox.Option>
{{/each}}

with result:

https://user-images.githubusercontent.com/11621383/162382931-46155110-1ed8-47f4-aaad-6ed0f5472f12.mp4

So it scrolls only once (when the selected item arrives to the array of options) – but it works the same in react – I checked it.

Regarding proposing change: IMO, we should keep API almost the same. Rare inconsistency should only be addressed when framework limits us. Hopley, my solution is also your case, or anyone how lands here with similar problem ;)