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.09k stars 929 forks source link

Japanese IME not working when controlling `inputValue` in `useCombobox` #1452

Open blovato opened 1 year ago

blovato commented 1 year ago

Relevant code or config

type Book = { author: string; title: string };

const books: Book[] = [
  { author: 'Harper Lee', title: 'To Kill a Mockingbird' },
  { author: 'Lev Tolstoy', title: 'War and Peace' },
  { author: 'Fyodor Dostoyevsy', title: 'The Idiot' },
  { author: 'Oscar Wilde', title: 'A Picture of Dorian Gray' },
  { author: 'George Orwell', title: '1984' },
  { author: 'Jane Austen', title: 'Pride and Prejudice' },
  { author: 'Marcus Aurelius', title: 'Meditations' },
  { author: 'Fyodor Dostoevsky', title: 'The Brothers Karamazov' },
  { author: 'Lev Tolstoy', title: 'Anna Karenina' },
  { author: 'Fyodor Dostoevsky', title: 'Crime and Punishment' },
];

function getBooksFilter(inputValue: string | undefined) {
  return function booksFilter(book: Book) {
    return (
      !inputValue ||
      book.title.toLowerCase().includes(inputValue) ||
      book.author.toLowerCase().includes(inputValue)
    );
  };
}
export const Example = () => {
  const [items, setItems] = React.useState(books);
  const [inputValue, setInputValue] = React.useState('');
  const {
    isOpen,
    getToggleButtonProps,
    getLabelProps,
    getMenuProps,
    getInputProps,
    getItemProps,
    getComboboxProps,
  } = useCombobox<Book>({
    onInputValueChange(chg) {
      setInputValue(chg.inputValue || '');
      setItems(books.filter(getBooksFilter(chg.inputValue)));
    },
    inputValue, // Controlling inputValue here introduces IME composition issue
    items,
    itemToString(item) {
      return item ? item.title : '';
    },
  });

  return (
    <div>
      <div>
        <label {...getLabelProps()}>Choose your favorite book:</label>
        <div {...getComboboxProps()}>
          <input placeholder="Best book ever" {...getInputProps()} />
          <button
            aria-label="toggle menu"
            type="button"
            {...getToggleButtonProps()}
          >
            {isOpen ? <>&#8593;</> : <>&#8595;</>}
          </button>
        </div>
      </div>
      <ul {...getMenuProps()}>
        {isOpen &&
          items.map((item, index) => (
            <li
              key={`${item.title}${index}`}
              {...getItemProps({ item, index })}
            >
              <span>{item.title}</span>
              <span>{item.author}</span>
            </li>
          ))}
      </ul>
    </div>
  );
};

What you did: When entering Japanese (or any IME language) into a combobox that controls inputValue, IME composition fails. This first screenshot renders a dropdown with inputValue controlled. I clicked the input to focus it and then typed "j" + "a".

Screen Shot 2022-12-09 at 11 45 05 AM

What happened: The IME editor failed to open to further compose the characters before committing them. What I should have saw would have been this:

Screen Shot 2022-12-09 at 11 45 50 AM

Additionally, it appears that the compositionend event is never fired when inputValue is controlled.

Problem description: When useCombobox is controlling inputValue and updating it with onInputValueChange, IME composition exits before showing the composition editor.

Suggested solution: Wrap getInputProps's onChange handler with the spirit of the following:

  const onComposition = useRef(false);
  const getOnChangeWithCompositionSupport = useCallback(
    ({
        onChangeProp,
      }: {
        onChangeProp: (e: ChangeEvent<HTMLInputElement>) => void;
      }) =>
      (event: ChangeEvent<HTMLInputElement>) => {
        setInputValue(event?.target?.value);

        // IME method start
        if (event.type === 'compositionstart') {
          onComposition.current = true;
          return;
        }

        // IME method end
        if (event.type === 'compositionend') {
          onComposition.current = false;
        }

        // handle parent onChange
        if (!onComposition.current) {
          onChangeProp?.(event);
        }
      },
    [setInputValue]
  );
silviuaavram commented 1 year ago

Hi! Is there something we can fix in useCombobox for this issue? And if so, can you create a PR? Thank you!

tychenjiajun commented 1 year ago

Still exists in v7. Here's the reproduce codesandbox https://codesandbox.io/s/quizzical-williamson-c8tsyg?file=/src/hooks/useCombobox/basic-usage.js

tychenjiajun commented 1 year ago

To help finding the cause, I wrote a similar controlled input https://codesandbox.io/s/focused-poincare-zpky86?file=/src/App.js

tychenjiajun commented 1 year ago

Also, related https://github.com/downshift-js/downshift/issues/1108 Just don't use the onInputValueChange https://github.com/downshift-js/downshift/issues/1108#issuecomment-842407759

shutallbridge commented 1 year ago

Having the same issue here. Not a clean solution at all, but for now overriding the onChange input prop and controlling this value with useState fixes it.