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

Japanese IME not working when controlling inputValue in useCombobox #1452

Open
blovato opened this issue Dec 9, 2022 · 5 comments
Open

Japanese IME not working when controlling inputValue in useCombobox #1452

blovato opened this issue Dec 9, 2022 · 5 comments
Labels

Comments

@blovato
Copy link

blovato commented Dec 9, 2022

  • downshift version: 6.1.12
  • node version: 14.19.0
  • yarn version: 1.22.17

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
Copy link
Collaborator

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

@tychenjiajun
Copy link

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

@tychenjiajun
Copy link

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

@tychenjiajun
Copy link

Also, related #1108
Just don't use the onInputValueChange #1108 (comment)

@shutallbridge
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants