downshift-js / downshift

🏎 A set of primitives to build simple, flexible, WAI-ARIA compliant React autocomplete, combobox or select dropdown components.
http://downshift-js.com/
MIT License
12.05k stars 933 forks source link

Add onSelect prop to hooks #1139

Open MonaiThang opened 4 years ago

MonaiThang commented 4 years ago

TLDR:

Ok, I understand. You need onSelect. Let's develop it, we will use this issue to track it.

You probably need to work in hooks/utils.js on callOnChangeProps. Call onChange if there if newState has selectedItem and we have onChange prop.

Also add unit tests, Typescript and documentation. See Downshift README for the last part. Good luck!

P.S. maybe you can also take #1042 to finish it if you feel like adding features? It should only need documentation and tests.

-----Initial Issue ----

Relevant code or config

import React, { useState } from "react";
import "./styles.css";
import Downshift from "downshift";

export default function App() {
  const options = [
    { id: 1, value: "Steak" },
    { id: 2, value: "Pasta" },
    { id: 3, value: "Rice" },
    { id: 4, value: "Cookie" }
  ];

  const [selectedOption, setSelectedOption] = useState(null);

  const onChangeAutocompleteInputField = selection => {
    console.log(selection);
    setSelectedOption(selection);
  };

  return (
    <div className="App">
      <Downshift
        onSelect={selection => {
          onChangeAutocompleteInputField(selection);
        }}
        itemToString={item => (item ? item.value : "")}
      >
        {({
          getInputProps,
          getMenuProps,
          getItemProps,
          isOpen,
          inputValue,
          reset
        }) => (
          <div>
            <input
              {...getInputProps({
                placeholder: "Eat something",
                onChange: reset
              })}
            />
            <div {...getMenuProps()}>
              {isOpen && (
                <div className="downshift-wrap">
                  {options
                    .filter(
                      item =>
                        !inputValue ||
                        item.value
                          .toLowerCase()
                          .includes(inputValue.toLowerCase())
                    )
                    .map((item, index) => (
                      <div
                        className="downshift-item"
                        {...getItemProps({ key: item.id, index, item })}
                        key={item.id}
                      >
                        {item.value}
                      </div>
                    ))}
                </div>
              )}
            </div>
          </div>
        )}
      </Downshift>
      {<p>You ate: {selectedOption ? selectedOption.value : null}</p>}
    </div>
  );
}

What you did: I try to make the input field value turns blank after selecting an option from the list. I read the documentation and searched for previous issues in this repo and found that some examples in https://github.com/downshift-js/downshift/issues/177#issuecomment-327575402, so I call reset in input onChange prop with an intention to reset the input state back to the starting point (which is the inputValue is blank and the selectedItem is null).

What happened: The inputValue doesn't reset back to blank. Now I'm not sure what do I miss.

Reproduction repository:

CodeSandbox URL: https://codesandbox.io/s/downshift-how-to-reset-inputvalue-after-running-onselect-0ew1w

Problem description: How to reset inputValue state after running onSelect while letting Downshift handles its own state all of the time except for the custom action I defined to clear inputValue when an option is selected.

Suggested solution: More examples on how to use each prop would be helpful

silviuaavram commented 4 years ago

Hi! First of all, consider migrating to useCombobox.

Your use case can probably be fixed more elegantly by using a stateReducer, since you don't want any side effect, only to change the next state a little bit. If you want to change the resulting state as a consequence of item change, consider catching Downshift.stateChangeTypes.clickItem and Downshift.stateChangeTypes.keyDownEnter or you can just check the changes.selectedItem, there are many ways to do it.

Here is a similar example with useCombobox https://www.downshift-js.com/use-combobox#state-reducer. Good luck!

MonaiThang commented 4 years ago

https://www.downshift-js.com/use-combobox#state-reducer

Thank you so much for your reply and the source code example URL of the suitable use case, it makes a lot of sense to me now.

MonaiThang commented 4 years ago

In case other people have a similar issue to what I had, below is the CodeSandbox of the fixed version for this issue. https://codesandbox.io/s/downshift-how-to-reset-inputvalue-after-running-onselect-v80en?file=/src/App.js

import React, { useState } from "react";
import "./styles.css";
import { useCombobox } from "downshift";

const options = [
  { id: 1, value: "Steak" },
  { id: 2, value: "Pasta" },
  { id: 3, value: "Rice" },
  { id: 4, value: "Cookie" }
];

const menuStyles = {
  maxHeight: "200px",
  overflowY: "auto",
  width: "150px",
  position: "absolute",
  margin: 0,
  borderTop: 0,
  background: "white"
};

const comboboxStyles = { display: "inline-block", marginLeft: "5px" };

export default function App() {
  const [inputItems, setInputItems] = useState(options);
  const [selectedOption, setSelectedOption] = useState(null);

  const stateReducer = (state, actionAndChanges) => {
    const { type, changes } = actionAndChanges;
    // returning a mutated version of the downshift component state
    switch (type) {
      case useCombobox.stateChangeTypes.InputChange:
        return {
          // return normal changes.
          ...changes
        };
      // also on selection.
      case useCombobox.stateChangeTypes.ItemClick:
      case useCombobox.stateChangeTypes.InputKeyDownEnter:
      case useCombobox.stateChangeTypes.InputBlur:
        onSelectExtraActions(changes.selectedItem);
        return {
          ...changes,
          // if we had an item highlighted in the previous state.
          ...(state.highlightedIndex > -1 && {
            // we will reset input field value to blank
            inputValue: ""
          })
        };
      default:
        return changes; // otherwise business as usual.
    }
  };

  const {
    isOpen,
    getToggleButtonProps,
    getLabelProps,
    getMenuProps,
    getInputProps,
    getComboboxProps,
    highlightedIndex,
    getItemProps
  } = useCombobox({
    items: inputItems,
    stateReducer,
    onInputValueChange: ({ inputValue }) => {
      setInputItems(
        options.filter((item) =>
          item.value.toLowerCase().startsWith(inputValue.toLowerCase())
        )
      );
    }
  });

  const onSelectExtraActions = (selection) => {
    console.log(selection);
    setSelectedOption(selection);
  };

  return (
    <div className="App">
      <div>
        <label {...getLabelProps()}>Choose an element:</label>
        <div style={comboboxStyles} {...getComboboxProps()}>
          <input {...getInputProps()} />
          <button
            type="button"
            {...getToggleButtonProps()}
            aria-label="toggle menu"
          >
            &#8595;
          </button>
        </div>
        <ul {...getMenuProps()} style={menuStyles}>
          {isOpen &&
            inputItems.map((item, index) => (
              <li
                style={
                  highlightedIndex === index
                    ? { backgroundColor: "#bde4ff" }
                    : {}
                }
                key={`${item}${index}`}
                {...getItemProps({ item, index })}
              >
                {item.value}
              </li>
            ))}
        </ul>
      </div>
      {<p>You ate: {selectedOption ? selectedOption.value : null}</p>}
    </div>
  );
}
silviuaavram commented 4 years ago

One last thing. Consider moving onSelectExtraActions in a on change handler, and remove it from the reducer.

The reducer should be a pure function (only change the next state). Side effects should be in on change handlers, such as onSelectedItemChange or onStateChange.

MonaiThang commented 4 years ago

One last thing. Consider moving onSelectExtraActions in an on change handler, and remove it from the reducer.

The reducer should be a pure function (only change the next state). Side effects should be in on change handlers, such as onSelectedItemChange or onStateChange.

Thanks for your suggestion. I have moved onSelectExtraActions from the reducer into the on change handler in the useCombobox.

Though I managed to make onSelectExtraActions triggers every time any item is selected (including the same item as the previous selected), it looks a bit hacky because:

  1. onSelect seems to be the closest behaviour but the function never called. Not sure if I did something wrong or the props cannot be used in this way.
  2. If I use onSelectedItemChange or onStateChange, I have to force the combo box to reset its state to make able to fire the function every time an item is selected by using useEffect.

https://codesandbox.io/s/downshift-how-to-reset-inputvalue-after-running-onselect-v80en?file=/src/App.js

import React, { useState, useEffect } from "react";
import "./styles.css";
import { useCombobox } from "downshift";

const options = [
  { id: 1, value: "Steak" },
  { id: 2, value: "Pasta" },
  { id: 3, value: "Rice" },
  { id: 4, value: "Cookie" }
];

const menuStyles = {
  maxHeight: "200px",
  overflowY: "auto",
  width: "150px",
  position: "absolute",
  margin: 0,
  borderTop: 0,
  background: "white"
};

const comboboxStyles = { display: "inline-block", marginLeft: "5px" };

const stateReducer = (state, actionAndChanges) => {
  const { type, changes } = actionAndChanges;
  // returning a mutated version of the downshift component state
  switch (type) {
    case useCombobox.stateChangeTypes.InputChange:
      return {
        // return normal changes.
        ...changes
      };
    // also on selection.
    case useCombobox.stateChangeTypes.ItemClick:
    case useCombobox.stateChangeTypes.InputKeyDownEnter:
    case useCombobox.stateChangeTypes.InputBlur:
      // onSelectExtraActions(changes.selectedItem);
      return {
        ...changes,
        // if we had an item highlighted in the previous state.
        ...(state.highlightedIndex > -1 && {
          // we will reset input field value to blank
          inputValue: ""
        })
      };
    default:
      return changes; // otherwise business as usual.
  }
};

export default function App() {
  const [inputItems, setInputItems] = useState(options);
  const [selectedOption, setSelectedOption] = useState(null);

  const {
    isOpen,
    getToggleButtonProps,
    getLabelProps,
    getMenuProps,
    getInputProps,
    getComboboxProps,
    highlightedIndex,
    getItemProps,
    reset
  } = useCombobox({
    items: inputItems,
    stateReducer,
    onInputValueChange: ({ inputValue }) => {
      setInputItems(
        options.filter((item) =>
          item.value.toLowerCase().startsWith(inputValue.toLowerCase())
        )
      );
    },
    onSelect: (selectedItem) => {
      console.log("selectedItem:", selectedItem);
      setSelectedOption(selectedItem);
    },
    onSelectedItemChange: (state) => {
      if (
        state.type === "__item_click__" ||
        state.type === "__input_keydown_enter__"
      ) {
        console.log("selectedItem:", state);
        setSelectedOption(state.selectedItem);
      }
    }
    // onStateChange: (changes) => {
    //   // console.log("changes:", changes);
    //   if (
    //     changes.type === "__item_click__" ||
    //     changes.type === "__input_keydown_enter__"
    //   ) {
    //     console.log("changes:", changes);
    //     setSelectedOption(changes.selectedItem);
    //   }
    // }
  });

  // const onSelectExtraActions = (selection) => {
  //   console.log(selection);
  //   setSelectedOption(selection);
  // };

  useEffect(() => {
    reset();
  }, [selectedOption, reset]);

  return (
    <div className="App">
      <div>
        <label {...getLabelProps()}>Choose an element:</label>
        <div style={comboboxStyles} {...getComboboxProps()}>
          <input {...getInputProps()} />
          <button
            type="button"
            {...getToggleButtonProps()}
            aria-label="toggle menu"
          >
            &#8595;
          </button>
        </div>
        <ul {...getMenuProps()} style={menuStyles}>
          {isOpen &&
            inputItems.map((item, index) => (
              <li
                style={
                  highlightedIndex === index
                    ? { backgroundColor: "#bde4ff" }
                    : {}
                }
                key={`${item}${index}`}
                {...getItemProps({ item, index })}
              >
                {item.value}
              </li>
            ))}
        </ul>
      </div>
      {<p>You ate: {selectedOption ? selectedOption.value : null}</p>}
    </div>
  );
}
silviuaavram commented 4 years ago

@MonaiThang onSelect is not part of useCombobox. You are in the wrong README. https://github.com/downshift-js/downshift/tree/master/src/hooks/useCombobox#onselecteditemchange

silviuaavram commented 4 years ago

What's the reset for? I don't understand. If you want to avoid onSelectedItemChange from firing when you select the same object, just handle this case in your stateReducer. If the selectedItem is the same, don't return it as next state.

Also you are forgetting itemToString.

MonaiThang commented 4 years ago

@MonaiThang onSelect is not part of useCombobox. You are in the wrong README. https://github.com/downshift-js/downshift/tree/master/src/hooks/useCombobox#onselecteditemchange

@silviuaavram Thank you for confirming that onSelect is not the part of useCombobox, I was confused whether it doesn't have this props or I did something wrong. Now I'm much relieved!

What's the reset for? I don't understand. If you want to avoid onSelectedItemChange from firing when you select the same object, just handle this case in your stateReducer. If the selectedItem is the same, don't return it as next state.

I tried to move onSelectExtraActions process/behaviour outside from the reducer (stateReducer) to the onChange handler (onSelectedItemChange or onStateChange) as you suggested, but I couldn't achieve to do the onSelectExtraActions behaviour when selecting the same item as the current selectedItem without mutating selectedItem state to null every time I select any item.

So I added the action prop reset into my custom hook (the useEffect block in my codesandbox) to reset the selectedItem state to null every time I select any item. But I feel the way I did it was a bit hacky. Should I put all those onSelectExtraActions processes back into the stateReducer or keep them this way?

Also you are forgetting itemToString.

Thank you for pointing out! I'm completely forgot the itemToString props.

Updated CodeSandbox https://codesandbox.io/s/downshift-how-to-reset-inputvalue-after-running-onselect-v80en?file=/src/App.js

import React, { useState, useEffect } from "react";
import "./styles.css";
import { useCombobox } from "downshift";

const options = [
  { id: 1, value: "Steak" },
  { id: 2, value: "Pasta" },
  { id: 3, value: "Rice" },
  { id: 4, value: "Cookie" }
];

const menuStyles = {
  maxHeight: "200px",
  overflowY: "auto",
  width: "150px",
  position: "absolute",
  margin: 0,
  borderTop: 0,
  background: "white"
};

const comboboxStyles = { display: "inline-block", marginLeft: "5px" };

const stateReducer = (state, actionAndChanges) => {
  const { type, changes } = actionAndChanges;
  // console.log(type);
  // console.log(changes);
  // returning a mutated version of the downshift component state
  switch (type) {
    case useCombobox.stateChangeTypes.InputChange:
      return {
        // return normal changes.
        ...changes
      };
    // also on selection.
    case useCombobox.stateChangeTypes.ItemClick:
    case useCombobox.stateChangeTypes.InputKeyDownEnter:
    case useCombobox.stateChangeTypes.InputBlur:
      // onSelectExtraActions(changes.selectedItem);
      return {
        ...changes,
        // if we had an item highlighted in the previous state.
        ...(state.highlightedIndex > -1 && {
          // we will reset input field value to blank
          inputValue: ""
        })
      };
    default:
      return changes; // otherwise business as usual.
  }
};

export default function App() {
  const [inputItems, setInputItems] = useState(options);
  const [selectedOption, setSelectedOption] = useState(null);

  const {
    isOpen,
    getToggleButtonProps,
    getLabelProps,
    getMenuProps,
    getInputProps,
    getComboboxProps,
    highlightedIndex,
    getItemProps,
    reset
  } = useCombobox({
    items: inputItems,
    stateReducer,
    onInputValueChange: ({ inputValue }) => {
      setInputItems(
        options.filter((item) =>
          item.value.toLowerCase().startsWith(inputValue.toLowerCase())
        )
      );
    },
    onSelectedItemChange: (state) => {
      if (
        state.type === "__item_click__" ||
        state.type === "__input_keydown_enter__"
      ) {
        console.log("selectedItem:", state);
        setSelectedOption(state.selectedItem);
      }
    },
    itemToString: (item) => {
      return item ? item.value : "";
    }
    // onStateChange: (changes) => {
    //   // console.log("changes:", changes);
    //   if (
    //     changes.type === "__item_click__" ||
    //     changes.type === "__input_keydown_enter__"
    //   ) {
    //     console.log("changes:", changes);
    //     setSelectedOption(changes.selectedItem);
    //   }
    // }
  });

  // const onSelectExtraActions = (selection) => {
  //   console.log(selection);
  //   setSelectedOption(selection);
  // };

  useEffect(() => {
    reset();
  }, [selectedOption, reset]);

  return (
    <div className="App">
      <div>
        <label {...getLabelProps()}>Choose an element:</label>
        <div style={comboboxStyles} {...getComboboxProps()}>
          <input {...getInputProps()} />
          <button
            type="button"
            {...getToggleButtonProps()}
            aria-label="toggle menu"
          >
            &#8595;
          </button>
        </div>
        <ul {...getMenuProps()} style={menuStyles}>
          {isOpen &&
            inputItems.map((item, index) => (
              <li
                style={
                  highlightedIndex === index
                    ? { backgroundColor: "#bde4ff" }
                    : {}
                }
                key={`${item}${index}`}
                {...getItemProps({ item, index })}
              >
                {item.value}
              </li>
            ))}
        </ul>
      </div>
      {<p>You ate: {selectedOption ? selectedOption.value : null}</p>}
    </div>
  );
}
silviuaavram commented 4 years ago

Ok, it seems I lost track, too much info and code.

So, you want, when selecting an item, to clear the input. You did that in stateReducer.

You want to display the selected item somewhere so you grab the value from the onSelectedItemChange and call setSelectedOption with it.

Now you want onSelectExtraActions to fire only when the selectedItem is different? Or am I confused?

MonaiThang commented 4 years ago

@silviuaavram The context of my question itself was quite a lot of information so it is quite easy to lost track.

The onSelectExtraActions itself can be more complicated processing that occurs every time an option item is selected even if the selectedItem is the same as the previous select. I simplified this function to only display the selected item somewhere for the example of the question.

Let's break down each case at a time.

So, you want, when selecting an item, to clear the input. You did that in stateReducer.

Yes.

You want to display the selected item somewhere so you grab the value from the onSelectedItemChange and call setSelectedOption with it.

Yes.

Now you want onSelectExtraActions to fire only when the selectedItem is different? Or am I confused?

Now I understand which part confuses you. Sorry for the unclear source code, I should have called onSelectExtraActions instead of setSelectedOption. Let's treat onSelectExtraActions as setSelectedOption for now - there are more complex logic in the onSelectExtraActions but I simplified the function for sake of example for the question.

The problem when I used onSelectedItemChange is, it will not fire onSelectExtraActions if I select the same item as I select earlier (e.g. select "Steak" first, then select "Steak" again). Though using onStateChange with the specified state type condition fires the onSelectExtraActions, it cannot parse the selectedItem values into the onSelectExtraActions because there is no change to the selectedItem state.

That is why I add a custom hook to reset the selectedItem state every time to trick the useCombobox, so that I can fire onSelectExtraActions to every selection. However, I feel this solution looks a bit hacky so I'm not sure if it will cause the unwanted side-effect to the component.

To reduce that confusion part, I have updated the code. https://codesandbox.io/s/downshift-how-to-reset-inputvalue-after-running-onselect-v80en?file=/src/App.js

import React, { useState, useEffect } from "react";
import "./styles.css";
import { useCombobox } from "downshift";

const options = [
  { id: 1, value: "Steak" },
  { id: 2, value: "Pasta" },
  { id: 3, value: "Rice" },
  { id: 4, value: "Cookie" }
];

const menuStyles = {
  maxHeight: "200px",
  overflowY: "auto",
  width: "150px",
  position: "absolute",
  margin: 0,
  borderTop: 0,
  background: "white"
};

const comboboxStyles = { display: "inline-block", marginLeft: "5px" };

const stateReducer = (state, actionAndChanges) => {
  const { type, changes } = actionAndChanges;
  // console.log(type);
  // console.log(changes);
  // returning a mutated version of the downshift component state
  switch (type) {
    case useCombobox.stateChangeTypes.InputChange:
      return {
        // return normal changes.
        ...changes
      };
    // also on selection.
    case useCombobox.stateChangeTypes.ItemClick:
    case useCombobox.stateChangeTypes.InputKeyDownEnter:
    case useCombobox.stateChangeTypes.InputBlur:
      // onSelectExtraActions(changes.selectedItem);
      return {
        ...changes,
        // if we had an item highlighted in the previous state.
        ...(state.highlightedIndex > -1 && {
          // we will reset input field value to blank
          inputValue: ""
        })
      };
    default:
      return changes; // otherwise business as usual.
  }
};

export default function App() {
  const [inputItems, setInputItems] = useState(options);
  const [selectedOption, setSelectedOption] = useState(null);

  const {
    isOpen,
    getToggleButtonProps,
    getLabelProps,
    getMenuProps,
    getInputProps,
    getComboboxProps,
    highlightedIndex,
    getItemProps,
    reset
  } = useCombobox({
    items: inputItems,
    stateReducer,
    onInputValueChange: ({ inputValue }) => {
      setInputItems(
        options.filter((item) =>
          item.value.toLowerCase().startsWith(inputValue.toLowerCase())
        )
      );
    },
    onSelectedItemChange: (state) => {
      if (
        state.type === "__item_click__" ||
        state.type === "__input_keydown_enter__"
      ) {
        console.log("selectedItem:", state);
        onSelectExtraActions(state.selectedItem);
      }
    },
    itemToString: (item) => {
      return item ? item.value : "";
    }
    // onStateChange: (changes) => {
    //   // console.log("changes:", changes);
    //   if (
    //     changes.type === "__item_click__" ||
    //     changes.type === "__input_keydown_enter__"
    //   ) {
    //     console.log("changes:", changes);
    //     setSelectedOption(changes.selectedItem);
    //   }
    // }
  });

  const onSelectExtraActions = (selection) => {
    console.log(selection);
    setSelectedOption(selection);
  };

  useEffect(() => {
    reset();
  }, [selectedOption, reset]);

  return (
    <div className="App">
      <div>
        <label {...getLabelProps()}>Choose an element:</label>
        <div style={comboboxStyles} {...getComboboxProps()}>
          <input {...getInputProps()} />
          <button
            type="button"
            {...getToggleButtonProps()}
            aria-label="toggle menu"
          >
            &#8595;
          </button>
        </div>
        <ul {...getMenuProps()} style={menuStyles}>
          {isOpen &&
            inputItems.map((item, index) => (
              <li
                style={
                  highlightedIndex === index
                    ? { backgroundColor: "#bde4ff" }
                    : {}
                }
                key={`${item}${index}`}
                {...getItemProps({ item, index })}
              >
                {item.value}
              </li>
            ))}
        </ul>
      </div>
      {<p>You ate: {selectedOption ? selectedOption.value : null}</p>}
    </div>
  );
}
silviuaavram commented 4 years ago

Ok, I understand. You need onSelect. Let's develop it, we will use this issue to track it.

You probably need to work in hooks/utils.js on callOnChangeProps. Call onSelect if there if newState has selectedItem and we have onSelect prop.

Also add unit tests, Typescript and documentation. See Downshift README for the last part. Good luck!

P.S. maybe you can also take https://github.com/downshift-js/downshift/pull/1042 to finish it if you feel like adding features? It should only need documentation and tests.