Skip to content

Commit

Permalink
Ensure IME works on Android devices (#2580)
Browse files Browse the repository at this point in the history
* simplify `isComposing`

We had an issue #2409 where typing using an IME (Input Method Editor,
E.g.: Japanese — Romaji) already submitted characters while the user was
still composing the characters together.

1. Type `wa`
2. Expected result: `わ`
3. Actual result: `wあ` (where `あ` is the character `a`)

This was solved by not triggering change events at all until the
`compositionend` event was triggered. This worked fine for this use
case. However this also meant that only at the end of your typing
session (when you press `enter`/`space`) the actual value was submitted.

Fast forward to today, we received a new issue #2575 where this
behaviour completely broke on Android devices. Android _always_ use the
IME APIs for handling input... if we think about our solution form
above, it also means that while you are typing on an Android device no
options are being filtered at all. The moment you hit enter/space the
combobox will open and results will be filtered.

This is where this fix comes in. The goals are simple:

1. Make it work
2. Try to make the current code simpler

I started digging to see _why_ this `wあ` was even submitted. A normal
input field doesn't do that?! We have some code that does the following
things:

1. Sync the selected value with the `input` such that if you update the
   value from the outside, then the value in the `input` is up-to-date
   with the `displayValue` of that incoming value.
2. A fix for macOS VoiceOver to improve the VoiceOver experience when
   opening the `Combobox` component. This is done by manually resetting
   the value of the `input` field.

   1. Keep track of the current value
   2. Keep track of the current selection range (start/end) state
   3. Reset the input to an empty string `''`
   4. Restore the value to the captured value
   5. Restore the selection range

When you are typing, the input field doesn't have to update yet because
typing doesn't cause an option to become the `selected` option,
therefore it doesn't have to sync the value yet. So (1.) isn't the issue
here.

However, when you start typing, the Combobox should open and then we
trigger the macOS VoiceOver fix. This is touching the `input` field
because we change the value & selection.

Because we touched the `input` while the user was still in a composing
mode, it bailed and submitted whatever characters it had. This is the
part that we don't want. Not applying the macOS VoiceOver fix while
typing solves this issue. In addition, because _we_ are touching the
input field, VoiceOver is acting normally.

In hindsight, the solution is very simple: do not touch the input field
when the user is typing.

We still keep track whether the user `isComposing` so that we can bail
on the default `Enter` behaviour (marking the current option as the
selected option) because pressing `Enter` while composing should get out
of the IME.

Fixes: #2575

* update changelog
  • Loading branch information
RobinMalfait committed Jul 6, 2023
1 parent 17b2d34 commit 00fdb7e
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 26 deletions.
1 change: 1 addition & 0 deletions packages/@headlessui-react/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

- Ensure the caret is in a consistent position when syncing the `Combobox.Input` value ([#2568](https://github.com/tailwindlabs/headlessui/pull/2568))
- Improve "outside click" behaviour in combination with 3rd party libraries ([#2572](https://github.com/tailwindlabs/headlessui/pull/2572))
- Ensure IME works on Android devices ([#2580](https://github.com/tailwindlabs/headlessui/pull/2580))

## [1.7.15] - 2023-06-01

Expand Down
33 changes: 20 additions & 13 deletions packages/@headlessui-react/src/components/combobox/combobox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -778,9 +778,11 @@ function InputFn<
// - By clicking `outside` of the Combobox
useWatch(
([currentDisplayValue, state], [oldCurrentDisplayValue, oldState]) => {
// When the user is typing, we want to not touch the `input` at all. Especially when they are
// using an IME, we don't want to mess with the input at all.
if (isTyping.current) return
let input = data.inputRef.current

let input = data.inputRef.current
if (!input) return

if (oldState === ComboboxState.Open && state === ComboboxState.Closed) {
Expand Down Expand Up @@ -821,6 +823,10 @@ function InputFn<
useWatch(
([newState], [oldState]) => {
if (newState === ComboboxState.Open && oldState === ComboboxState.Closed) {
// When the user is typing, we want to not touch the `input` at all. Especially when they are
// using an IME, we don't want to mess with the input at all.
if (isTyping.current) return

let input = data.inputRef.current
if (!input) return

Expand All @@ -844,19 +850,12 @@ function InputFn<
)

let isComposing = useRef(false)
let composedChangeEvent = useRef<React.ChangeEvent<HTMLInputElement> | null>(null)
let handleCompositionStart = useEvent(() => {
isComposing.current = true
})
let handleCompositionEnd = useEvent(() => {
d.nextFrame(() => {
isComposing.current = false

if (composedChangeEvent.current) {
actions.openCombobox()
onChange?.(composedChangeEvent.current)
composedChangeEvent.current = null
}
})
})

Expand Down Expand Up @@ -885,6 +884,10 @@ function InputFn<
case Keys.Enter:
isTyping.current = false
if (data.comboboxState !== ComboboxState.Open) return

// When the user is still in the middle of composing by using an IME, then we don't want to
// submit this value and close the Combobox yet. Instead, we will fallback to the default
// behaviour which is to "end" the composition.
if (isComposing.current) return

event.preventDefault()
Expand Down Expand Up @@ -983,12 +986,16 @@ function InputFn<
})

let handleChange = useEvent((event: React.ChangeEvent<HTMLInputElement>) => {
if (isComposing.current) {
composedChangeEvent.current = event
return
}
actions.openCombobox()
// Always call the onChange listener even if the user is still typing using an IME (Input Method
// Editor).
//
// The main issue is Android, where typing always uses the IME APIs. Just waiting until the
// compositionend event is fired to trigger an onChange is not enough, because then filtering
// options while typing won't work at all because we are still in "composing" mode.
onChange?.(event)

// Open the combobox to show the results based on what the user has typed
actions.openCombobox()
})

let handleBlur = useEvent(() => {
Expand Down
1 change: 1 addition & 0 deletions packages/@headlessui-vue/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Ensure the caret is in a consistent position when syncing the `Combobox.Input` value ([#2568](https://github.com/tailwindlabs/headlessui/pull/2568))
- Improve "outside click" behaviour in combination with 3rd party libraries ([#2572](https://github.com/tailwindlabs/headlessui/pull/2572))
- Improve performance of `Combobox` component ([#2574](https://github.com/tailwindlabs/headlessui/pull/2574))
- Ensure IME works on Android devices ([#2580](https://github.com/tailwindlabs/headlessui/pull/2580))

## [1.7.14] - 2023-06-01

Expand Down
33 changes: 20 additions & 13 deletions packages/@headlessui-vue/src/components/combobox/combobox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -745,9 +745,11 @@ export let ComboboxInput = defineComponent({
watch(
[currentDisplayValue, api.comboboxState],
([currentDisplayValue, state], [oldCurrentDisplayValue, oldState]) => {
// When the user is typing, we want to not touch the `input` at all. Especially when they
// are using an IME, we don't want to mess with the input at all.
if (isTyping.value) return
let input = dom(api.inputRef)

let input = dom(api.inputRef)
if (!input) return

if (oldState === ComboboxStates.Open && state === ComboboxStates.Closed) {
Expand Down Expand Up @@ -788,6 +790,10 @@ export let ComboboxInput = defineComponent({
// already in an open state.
watch([api.comboboxState], ([newState], [oldState]) => {
if (newState === ComboboxStates.Open && oldState === ComboboxStates.Closed) {
// When the user is typing, we want to not touch the `input` at all. Especially when they
// are using an IME, we don't want to mess with the input at all.
if (isTyping.value) return

let input = dom(api.inputRef)
if (!input) return

Expand All @@ -810,19 +816,12 @@ export let ComboboxInput = defineComponent({
})

let isComposing = ref(false)
let composedChangeEvent = ref<(Event & { target: HTMLInputElement }) | null>(null)
function handleCompositionstart() {
isComposing.value = true
}
function handleCompositionend() {
disposables().nextFrame(() => {
isComposing.value = false

if (composedChangeEvent.value) {
api.openCombobox()
emit('change', composedChangeEvent.value)
composedChangeEvent.value = null
}
})
}

Expand Down Expand Up @@ -852,6 +851,10 @@ export let ComboboxInput = defineComponent({
case Keys.Enter:
isTyping.value = false
if (api.comboboxState.value !== ComboboxStates.Open) return

// When the user is still in the middle of composing by using an IME, then we don't want
// to submit this value and close the Combobox yet. Instead, we will fallback to the
// default behaviour which is to "end" the composition.
if (isComposing.value) return

event.preventDefault()
Expand Down Expand Up @@ -945,12 +948,16 @@ export let ComboboxInput = defineComponent({
}

function handleInput(event: Event & { target: HTMLInputElement }) {
if (isComposing.value) {
composedChangeEvent.value = event
return
}
api.openCombobox()
// Always call the onChange listener even if the user is still typing using an IME (Input Method
// Editor).
//
// The main issue is Android, where typing always uses the IME APIs. Just waiting until the
// compositionend event is fired to trigger an onChange is not enough, because then filtering
// options while typing won't work at all because we are still in "composing" mode.
emit('change', event)

// Open the combobox to show the results based on what the user has typed
api.openCombobox()
}

function handleBlur() {
Expand Down

2 comments on commit 00fdb7e

@vercel
Copy link

@vercel vercel bot commented on 00fdb7e Jul 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

headlessui-vue – ./packages/playground-vue

headlessui-vue-tailwindlabs.vercel.app
headlessui-vue-git-main-tailwindlabs.vercel.app
headlessui-vue.vercel.app

@vercel
Copy link

@vercel vercel bot commented on 00fdb7e Jul 6, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Successfully deployed to the following URLs:

headlessui-react – ./packages/playground-react

headlessui-react-tailwindlabs.vercel.app
headlessui-react.vercel.app
headlessui-react-git-main-tailwindlabs.vercel.app

Please sign in to comment.