Skip to content

[WC-3454] Fix issue with IME input in dropdown filter#2271

Open
r0b1n wants to merge 1 commit into
mainfrom
fix/fix-ime-dropdown-filter
Open

[WC-3454] Fix issue with IME input in dropdown filter#2271
r0b1n wants to merge 1 commit into
mainfrom
fix/fix-ime-dropdown-filter

Conversation

@r0b1n

@r0b1n r0b1n commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator

Pull request type

Bug fix (non-breaking change which fixes an issue)


Description

This uses recommended workaround from
https://github.com/downshift-js/downshift/tree/master/src/hooks/useCombobox#inputvalue

What should be covered while testing?

  • IME inputs work correctly after the fix.
  • Filtering works correctly.
  • Keyboard interaction on input focused is working correctly (esc to clean, arrow keys)

@r0b1n r0b1n requested a review from a team as a code owner June 17, 2026 11:50
@github-actions

This comment has been minimized.

@r0b1n r0b1n force-pushed the fix/fix-ime-dropdown-filter branch from 1fbf020 to 1026b03 Compare June 17, 2026 12:21
@github-actions

This comment has been minimized.

@r0b1n r0b1n force-pushed the fix/fix-ime-dropdown-filter branch from 1026b03 to 220894b Compare June 22, 2026 13:55
@r0b1n r0b1n force-pushed the fix/fix-ime-dropdown-filter branch from 220894b to f76320a Compare July 3, 2026 10:11
@github-actions

github-actions Bot commented Jul 3, 2026

Copy link
Copy Markdown
Contributor

AI Code Review

⚠️ Approved with suggestions — low-severity items only, safe to merge


What was reviewed

File Change
packages/pluggableWidgets/datagrid-dropdown-filter-web/CHANGELOG.md Adds unreleased entry for the IME fix
packages/shared/widget-plugin-dropdown-filter/src/containers/EnumFilterContainer.tsx Passes new onChange prop to Combobox and TagPicker
packages/shared/widget-plugin-dropdown-filter/src/containers/RefFilterContainer.tsx Passes new onChange prop to Combobox and TagPicker
packages/shared/widget-plugin-dropdown-filter/src/controllers/mixins/ComboboxControllerMixin.ts Adds handleChange, switches onInputValueChangeonStateChange, removes InputChange path
packages/shared/widget-plugin-dropdown-filter/src/controllers/mixins/TagPickerControllerMixin.ts Adds handleChange, same onStateChange migration, adds missing Escape handler
packages/shared/widget-plugin-dropdown-filter/src/controls/combobox/Combobox.tsx Adds onChange prop and wires it to the input
packages/shared/widget-plugin-dropdown-filter/src/controls/tag-picker/TagPicker.tsx Adds onChange prop and wires it to the input

Skipped (out of scope): dist/, pnpm-lock.yaml

CI checks could not be fetched (command required approval) — verify green before merging.


Findings

⚠️ Low — TagPickerControllerMixin was missing the Escape handler before this PR

File: packages/shared/widget-plugin-dropdown-filter/src/controllers/mixins/TagPickerControllerMixin.ts (diff context around new onStateChange)
Note: The old onInputValueChange in TagPickerControllerMixin never handled InputKeyDownEscape; the ComboboxControllerMixin did. The fix adds the Escape path to TagPickerControllerMixin as well. This is a silent behavioural change (good bug fix) but it is not mentioned in the CHANGELOG entry. Consider expanding the entry to cover it: "We also fixed the Escape key not clearing the tag-picker input."


⚠️ Low — No unit tests for the new handleChange / IME path

File: packages/shared/widget-plugin-dropdown-filter/src/controllers/mixins/ComboboxControllerMixin.ts line 138; TagPickerControllerMixin.ts line 111
Note: The handleChange method and the onStateChange contract change are the core of this fix but there are no new tests exercising them. Existing DatagridDropdownFilter.spec.tsx tests do not cover typing in the input. Adding a test that simulates change events on the combobox input (bypassing downshift's InputChange state type) would guard against regressions and document the IME workaround intent.


Positives

  • Correctly follows the downshift-recommended workaround: listening to native onChange instead of relying on onInputValueChange/InputChange state type, which is the exact pattern the library docs prescribe for IME compatibility.
  • handleChange is properly registered as a MobX action in makeObservable for both mixins — no accidental out-of-action mutation.
  • The TagPickerControllerMixin.stateReducer correctly resets inputValue to state.inputValue on ItemClick/InputKeyDownEnter, which prevents the IME-committed text from leaking into the filter after a selection.
  • Import reordering (external → internal) is consistent with the rest of the codebase.
  • CHANGELOG entry is present in the correct widget package and uses user-facing language.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant