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

[Autocomplete] Document listbox + scroll constraint #18766

Closed
1 task done
zatine opened this issue Dec 9, 2019 · 6 comments · Fixed by #20101
Closed
1 task done

[Autocomplete] Document listbox + scroll constraint #18766

zatine opened this issue Dec 9, 2019 · 6 comments · Fixed by #20101
Labels
component: autocomplete This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@zatine
Copy link
Contributor

zatine commented Dec 9, 2019

  • I have searched the issues of this repository and believe that this is not a duplicate.

Summary 💡

I would like some way to read the value of the currently highlighted item or react on the value changing in the Autocomplete component.

Examples 🌈

No strong opinions on how it should be implemented, but I would like the ability to do something along these lines:

<Autocomplete {...props} onHighlight={(index) => doSomething(index)} />

Motivation 🔦

I followed the guidelines in the docs for implementing a virtualized Autocomplete, and it worked like a charm. However, I'm in a situation where I'm not able to add react-window as a dependency. I do have react-virtualized to work with, and tried implementing it like this:
https://codesandbox.io/s/sleepy-hypatia-igog8?fontsize=14&hidenavigation=1&module=%2Fsrc%2FVirtualizedAutocomplete.js&theme=dark

It's close to working, the only thing off is the keyboard navigation. If I navigate to something outside of the visible items, the list doesn't scroll with the highlight, which would be the desired behavior. List in react-virtualized does have a scrollToIndex method, which I could have utilized if I could somehow get a hold of the currently highlighted index.

My other option would be to use the useAutocomplete hook instead and decorate the onKeyDown method, but since the only thing I'm adding is the virtualization I'd like to avoid that if possible.

@oliviertassinari
Copy link
Member

oliviertassinari commented Dec 9, 2019

@zatine If you want to scroll feature to work, you need to make sure the element with the listbox role holds the scroll container, as do with react-window. I would discourage any other approach.

@oliviertassinari
Copy link
Member

I'm not so sure about having a demo with react-window and react-virtualized, but why not: https://npm-stat.com/charts.html?package=react-window,react-virtualized.

@oliviertassinari oliviertassinari added the component: autocomplete This is the name of the generic UI component, not the React module! label Dec 9, 2019
@oliviertassinari oliviertassinari changed the title [Autocomplete] Ability to react on highlighted item [Autocomplete] Demo with react-virtualized Dec 9, 2019
@zatine
Copy link
Contributor Author

zatine commented Dec 10, 2019

Thank you, that was exactly what I needed to understand to get it working! If it feels redundant to add another example maybe it's enough to give a small heads up of the requirements for integration next to the current one? Something like "if you want to integrate this with another library make sure that the scrolling component gets the listbox role from the ListboxComponent props" (and anything else that feels relevant).

@oliviertassinari
Copy link
Member

Sure, so you want to add such mention in the documentation? Maybe it should be listed as a limitation?

@oliviertassinari oliviertassinari added docs Improvements or additions to the documentation good first issue Great for first contributions. Enable to learn the contribution process. labels Dec 10, 2019
@zatine
Copy link
Contributor Author

zatine commented Dec 11, 2019

Sure, I can do that. Any other limitations you can think of that should be mentioned in the same place?

@oliviertassinari
Copy link
Member

oliviertassinari commented Dec 11, 2019

None that I can think of. I think that we can do something very simple, like:

diff --git a/docs/src/pages/components/autocomplete/autocomplete.md b/docs/src/pages/components/autocomplete/autocomplete.md
index af3785c0e..416fb099d 100644
--- a/docs/src/pages/components/autocomplete/autocomplete.md
+++ b/docs/src/pages/components/autocomplete/autocomplete.md
@@ -172,6 +172,11 @@ Search within 10,000 randomly generated options. The list is virtualized thanks
 VoiceOver on iOS Safari doesn't support the `aria-owns` attribute very well.
 You can work around the issue with the `disablePortal` prop.

+### listbox + scroll
+
+In the event you provide a custom `ListboxComponent` prop,
+you need to make sure that the element with the listbox `role` is also the scroll container.
+This ensures a correct behavior of the scroll, e.g. when using the keyboard to navigate the list.
+
 ## Accessibility

 (WAI-ARIA: https://www.w3.org/TR/wai-aria-practices/#combobox)

@oliviertassinari oliviertassinari changed the title [Autocomplete] Demo with react-virtualized [Autocomplete] Document listbox + scroll constraint Dec 14, 2019
zatine added a commit to zatine/material-ui that referenced this issue Mar 13, 2020
Adding a ListboxComponent limitation entry for the Autocomplete component as discussed in this issue: mui#18766
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: autocomplete This is the name of the generic UI component, not the React module! docs Improvements or additions to the documentation good first issue Great for first contributions. Enable to learn the contribution process.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants