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] Add option to reset highlighted index #19705

Closed
jdoklovic opened this issue Feb 14, 2020 · 9 comments · Fixed by #19923
Closed

[Autocomplete] Add option to reset highlighted index #19705

jdoklovic opened this issue Feb 14, 2020 · 9 comments · Fixed by #19923
Labels
bug 🐛 Something doesn't work component: autocomplete This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process.

Comments

@jdoklovic
Copy link

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

Summary 💡

Currently the highlighted index is saved between open and closes so that the next time the listbox is opened it highlights the last known highlighted option.

This works if your option list is always the same, however, if your options change between open/close, the component still highlights the index from the old options, which may or may not exist.

It would be better if there was a way to either have it auto-reset to 0 if it notices the options have changed, or simply provide a prop that can be used to force the highlighted index to 0 onClose

Examples 🌈

hilite-index

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work component: autocomplete This is the name of the generic UI component, not the React module! labels Feb 14, 2020
@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 14, 2020

@jdoklovic Interesting, I think that the current wrong behavior comes from an approximation I took, I have made any inputValue change -> reset highlighted index. In an ideal world, the index should also be kept in sync with the (value, option) index position that matches.

@jdoklovic
Copy link
Author

I've also noticed that if you hit "Enter" on a freesolo with the listbox open but no option selected, it clears the input value, which is not what I want in my case.

@jdoklovic
Copy link
Author

ac-clearing

@kurtwilbies
Copy link

@djoklovic Could you please share your code!

@jdoklovic
Copy link
Author

@kurtwilbies Here's what I was able to put together to get 99% there. With this code, the highlighted index problem is mostly fixed by always setting a loading option, however, in some cases it still doesn't highlight anything, which then causes the other bug where hitting ENTER clears the input if no option is selected.

I haven't found a workaround for those yet.

Note that in the code below, fetch() calls an async "suggestor" function that returns options as {displayName:string, value:string}

interface JQLInputProps {
    jqlAutocompleteRestData: JqlAutocompleteRestData;
    suggestionFetcher: FieldSuggestionFetcher;
    onJqlChange?: (newValue: string) => void;
    value?: string;
    disabled?: boolean;
}

const loadingOption: Suggestion = { displayName: 'Loading...', value: '__-loading-__' };

export const JQLInput: React.FunctionComponent<JQLInputProps & TextFieldProps> = ({ jqlAutocompleteRestData, suggestionFetcher, value, onJqlChange, disabled, ...tfProps }) => {
    const [inputValue, setInputValue] = useState((value) ? value : '');
    const [selStart, setSelStart] = useState<number>(0);
    const [fieldPositionDirty, setFieldPositionDirty] = useState<boolean>(false);
    const inputField = useRef<HTMLInputElement>();
    const [open, setOpen] = React.useState<boolean>(false);
    const [fetching, setFetching] = React.useState<boolean>(false);

    const suggestor = useMemo<JqlSuggestor>(() => new JqlSuggestor(jqlAutocompleteRestData, suggestionFetcher), [jqlAutocompleteRestData, suggestionFetcher]);

    // get initial data
    const suggestions: Suggestion[] = [{ displayName: 'NOT', value: 'NOT' }, { displayName: 'start group (', value: '(' }];
    suggestions.push(...jqlAutocompleteRestData.visibleFieldNames.map<Suggestion>(f => ({ displayName: f.displayName, value: f.value })));

    const [options, setOptions] = useState<Suggestions>(suggestions);
    const optionsLoading = open && fetching;

    const fetch = React.useMemo(
        () =>
            async (input: string, startIndex: number) => {
                setOptions([loadingOption]);
                if (!open) {
                    setOpen(true);
                }
                var suggestions: Suggestions = [];

                setFetching(true);
                suggestions = await suggestor.getSuggestions(input, startIndex);
                setFetching(false);
                setOptions(suggestions);
            },
        [open, suggestor],
    );

    const handleTextFieldChange = (event: React.ChangeEvent<HTMLInputElement>) => { // fires on typing, but not when option is selected
        // update our local state
        if (inputField.current) {
            var newStart = (inputField.current.selectionStart) ? inputField.current.selectionStart : 0;
            fetch(event.target.value, newStart);
            setSelStart(newStart);
        }
        setInputValue(event.target.value);

    };

    const handleAutocompleteChange = (event: React.ChangeEvent<HTMLInputElement>, value: any) => { //only fires when an option is selected with the option value
        var insertText: string = (value) ? value.value : "";
        var newCursorPos = 0;
        if (insertText === '' || insertText === loadingOption.value) {
            return;
        }

        if (inputField.current) {
            newCursorPos = selStart;
            [insertText, newCursorPos] = suggestor.calculateNewValueForSelected(inputValue, insertText, selStart);
        }
        setInputValue(insertText);
        setSelStart(newCursorPos);

        // if we're not at the end, we're editing and need to update the cursor position later when the text field catches up
        if (newCursorPos !== insertText.length) {
            setFieldPositionDirty(true);
        }
    };

    const getOptionLabel = (option: Suggestion): string => { //fires when option is selected and should return the new input box value in it's entirety. Also fires on focus events, and randomly.
        // if we recently edited a value, we need to set the new cursor position manually
        if (fieldPositionDirty && inputField.current) {
            inputField.current.value = inputValue; // have to reset the value to get the right sel index
            inputField.current.selectionStart = selStart;
            inputField.current.selectionEnd = selStart;
            setFieldPositionDirty(false);
        }

        return inputValue;
    };

    const handleTextFieldKeyDown = (event: React.KeyboardEvent<HTMLInputElement>) => {
        // update our local cursorPosition on arrow keys so we fetch the proper options
        switch (event.key) {
            case 'ArrowRight': {
                if (inputField.current && inputField.current.selectionStart !== null && inputField.current.selectionStart !== inputField.current.value.length) {
                    const newStart = (inputField.current.selectionStart < inputField.current.value.length) ? inputField.current.selectionStart + 1 : inputField.current.value.length;
                    fetch(inputField.current.value, newStart);
                    setSelStart(newStart);
                }
                break;
            }
            case 'ArrowLeft': {
                if (inputField.current && inputField.current.selectionStart && inputField.current.selectionStart !== 0) {
                    const newStart = (inputField.current.selectionStart > 0) ? inputField.current.selectionStart - 1 : 0;
                    fetch(inputField.current.value, newStart);
                    setSelStart(newStart);
                }
                break;
            }
            case 'Enter': {

                break;
            }
        };
    };

    useEffect(() => {

        if (onJqlChange) {
            onJqlChange(inputValue);
        }

    }, [inputValue, onJqlChange]);

    return (
        <Autocomplete
            id="jql-editor"
            disableOpenOnFocus={false}
            getOptionLabel={getOptionLabel}
            filterOptions={x => x}
            options={options}
            value={inputValue}
            includeInputInList
            freeSolo
            autoHighlight
            onChange={handleAutocompleteChange}
            loading={optionsLoading}
            open={open}
            disabled={disabled}
            onOpen={() => {
                setOpen(true);
            }}
            onClose={() => {
                setOpen(false);
            }}
            renderInput={params => (
                <TextField
                    {...params}
                    {...tfProps}
                    inputRef={inputField}
                    onChange={handleTextFieldChange}
                    onKeyDown={handleTextFieldKeyDown}
                    InputProps={{
                        ...params.InputProps,
                        endAdornment: (
                            <React.Fragment>
                                {optionsLoading ? <CircularProgress color="inherit" size={20} /> : null}
                                {params.InputProps.endAdornment}
                            </React.Fragment>
                        ),
                    }}
                />
            )}
            renderOption={option => {
                const matches = match(option.displayName, suggestor.getCurrentToken().value);
                const parts = parse(option.displayName, matches);

                return (
                    <div key={option.value}>
                        {parts.map((part, index) => (
                            <span key={index} style={{ fontWeight: part.highlight ? 700 : 400 }}>
                                {part.text}
                            </span>
                        ))}
                    </div>
                );
            }}
        />
    );
};

@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 22, 2020

Here is a proposed solution for #19637, #19779, and #19637. Let us know if it works for your use case!

diff --git a/packages/material-ui-lab/src/Autocomplete/Autocomplete.test.js b/packages/material-ui-lab/src/Autocomplete/Autocomplete.test.js
index bd9dc472f..c71b1b88f 100644
--- a/packages/material-ui-lab/src/Autocomplete/Autocomplete.test.js
+++ b/packages/material-ui-lab/src/Autocomplete/Autocomplete.test.js
@@ -757,7 +757,6 @@ describe('<Autocomplete />', () => {
         />,
       );

-      fireEvent.keyDown(document.activeElement, { key: 'ArrowDown' });
       fireEvent.keyDown(document.activeElement, { key: 'ArrowDown' });
       fireEvent.keyDown(document.activeElement, { key: 'Enter' });

diff --git a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
index 9957ed76d..2200b20c1 100644
--- a/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
+++ b/packages/material-ui-lab/src/useAutocomplete/useAutocomplete.js
@@ -396,6 +396,32 @@ export default function useAutocomplete(props) {
     changeHighlightedIndex('reset', 'next');
   }, [inputValue]); // eslint-disable-line react-hooks/exhaustive-deps

+  React.useEffect(() => {
+    if (!open) {
+      return;
+    }
+
+    const valueItem = multiple ? value[0] : value;
+
+    if (filteredOptions.length === 0 || valueItem == null) {
+      changeHighlightedIndex('reset', 'next');
+      return;
+    }
+
+    if (!filterSelectedOptions && valueItem != null) {
+      const itemIndex = findIndex(filteredOptions, optionItem =>
+        getOptionSelected(optionItem, valueItem),
+      );
+
+      setHighlightedIndex(itemIndex);
+      return;
+    }
+
+    if (highlightedIndexRef.current >= filteredOptions.length - 1) {
+      setHighlightedIndex(filteredOptions.length - 1);
+    }
+  // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [value, open, filterSelectedOptions, multiple]);
+
   const handleOpen = event => {
     if (open) {
       return;

We will probably need to add one test case per issue for this logic (corresponding to each issue).

@oliviertassinari oliviertassinari added the good first issue Great for first contributions. Enable to learn the contribution process. label Feb 22, 2020
@captain-yossarian
Copy link
Contributor

Hi @jdoklovic, could you please checkout on my branch and check if this fix will work for your case.
If no, could you please share minimum code example?

@jdoklovic
Copy link
Author

@oliviertassinari @captain-yossarian Sorry I'm late to this.
I'm not going to have time to test anything until after next week.

Also, I'm not really sure how I would go about local testing. There don't seem to be any docs on how to consume component changes in a different project locally. yalc? npm links? any suggestions would be helpful.

@oliviertassinari
Copy link
Member

@jdoklovic The change is going live this weekend. You will be able to try it directly.

Regarding your question on yarn link, etc. You are right, it's not documented. The simplest would be to used the version of the packages we publish for each pull request. You can find it following the codesandbox ci breadcrumbs. Better documentation on the topic could potentially help.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: autocomplete This is the name of the generic UI component, not the React module! 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.

4 participants