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] Option highlighted event #20588

Closed
aslupik opened this issue Apr 16, 2020 · 7 comments · Fixed by #20691 or #20923
Closed

[Autocomplete] Option highlighted event #20588

aslupik opened this issue Apr 16, 2020 · 7 comments · Fixed by #20691 or #20923
Labels
component: autocomplete This is the name of the generic UI component, not the React module! ready to take Help wanted. Guidance available. There is a high chance the change will be accepted

Comments

@aslupik
Copy link

aslupik commented Apr 16, 2020

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

Summary 💡

<Autocomplete
   onOptionHighlighted={(option: T, reason: 'keyboardNavigation' | 'mouseOver' | 'automatic') => { ... }}
   ...
/>

This event would trigger when an option is highlighted (such that pressing Enter would then select that option). For instance, it would trigger upon using the arrow keys to cycle through the options and report which option is currently highlighted.

Motivation 🔦

Give context to the user on the highlighted option to allow him to make a better choice. Right now, we only know which option is selected after the user makes a choice; this would allow providing feedback proactively rather than reactively.

@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 16, 2020

Give context to the user on the highlighted option to allow him to make a better choice.

@aslupik How do you plan to implement this extra context? (assuming you have this callback available).

@oliviertassinari oliviertassinari added status: waiting for author Issue with insufficient information component: autocomplete This is the name of the generic UI component, not the React module! labels Apr 16, 2020
@aslupik
Copy link
Author

aslupik commented Apr 16, 2020

Give context to the user on the highlighted option to allow him to make a better choice.

@aslupik How do you plan to implement this extra context? (assuming you have this callback available).

I'm using Redux, so the callback would trigger an action allowing the rest of the application to know which option is currently highlighted. Then, a separate component on the page can subscribe to that and display information related to the highlighted option. This component updates dynamically as the user cycles through the options in the dropdown.

@oliviertassinari oliviertassinari removed the status: waiting for author Issue with insufficient information label Apr 16, 2020
@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 16, 2020

@aslupik Ok sounds fair. What do you think of this API to anticipate for a future case when somebody will need to control the highlighted option (at the cost of harming performance, I imagine)

diff --git a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.d.ts b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.d.ts
index 910be8d93..0dd380265 100644
--- a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.d.ts
+++ b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.d.ts
@@ -134,6 +134,18 @@ export interface UseAutocompleteCommonProps<T> {
    * @param {string} reason Can be: `"toggleInput"`, `"escape"`, `"select-option"`, `"blur"`.
    */
   onClose?: (event: React.ChangeEvent<{}>, reason: AutocompleteCloseReason) => void;
+  /**
+   * Callback  fired when the highlighted option changes.
+   *
+   * @param {object} event The event source of the callback.
+   * @param {T} option
+   * @param {string} reason Can be: `"keyboard"`, `"auto"`, `"mouseOver"`.
+   */
+  onHighlightChange?: (
+    event?: React.ChangeEvent<{}>,
+    option: T,
+    reason: AutocompleteHighlightChangeReason
+  );
   /**
    * Callback fired when the input value changes.
    *
@@ -178,6 +190,12 @@ export type AutocompleteChangeReason =
   | 'remove-option'
   | 'clear'
   | 'blur';
+
+export type AutocompleteHighlightChangeReason =
+  | 'keyboard'
+  | 'mouseOver'
+  | 'auto';
+
 export interface AutocompleteChangeDetails<T = string> {
   option: T;
 }

@oliviertassinari oliviertassinari added the ready to take Help wanted. Guidance available. There is a high chance the change will be accepted label Apr 16, 2020
@aslupik
Copy link
Author

aslupik commented Apr 16, 2020

@oliviertassinari That looks good to me!

@marcosvega91
Copy link
Contributor

I Will create a PR with this feature soon :)

@abbasyadollahi
Copy link

@oliviertassinari @marcosvega91
There seems to be an issue with this new feature when used alongside the filterSelectedOptions prop.

When navigating over the listbox options, the value argument passed to the onHighlightChange function doesn't take into consideration the options that have been filtered out by the input. As a result, value corresponds to the same index option as if there was no filter.

I made a small sandbox to show what I mean. The first option (Jaws) is correct, but starting from the second option (Avengers), the values are shifted by one position.

@marcosvega91
Copy link
Contributor

Hi @theGirrafish I will correct the issue :)

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! ready to take Help wanted. Guidance available. There is a high chance the change will be accepted
Projects
None yet
4 participants