Skip to content

fix(company-input-v2): commit typed/autofilled value on blur - #241

Merged
smarcet merged 1 commit into
mainfrom
fix/company-input-autofill-commit
May 18, 2026
Merged

fix(company-input-v2): commit typed/autofilled value on blur#241
smarcet merged 1 commit into
mainfrom
fix/company-input-autofill-commit

Conversation

@gcutrini

@gcutrini gcutrini commented May 15, 2026

Copy link
Copy Markdown
Contributor

ref: https://app.clickup.com/t/86b9rnr4r

Summary

CompanyInputV2 wraps MUI Autocomplete in freeSolo mode, but onChange only fires when the user explicitly selects an option. Values entered via typing-and-tabbing or browser autofill (notably iOS Chrome) never propagate to the parent, so required-field validation fails on submit.

Reproduce with autofill simulation in DevTools: set the input value via the native setter, dispatch input, then blur — without this PR the parent's company stays {id: null, name: ""}; with it the parent receives {id: 0, name: <typed>}.

Changes

  • Add autoSelect to the Autocomplete. MUI's intended mechanism for committing the current input value on blur in a free-text combo.
  • Normalize strings in onChange to the {id, name} shape consumers expect, since autoSelect calls onChange with the raw typed/autofilled string.

Summary by CodeRabbit

  • Bug Fixes
    • Improved company input field to properly normalize manually typed values, including automatic whitespace trimming and consistent formatting when input changes occur.

Review Change Stack

MUI Autocomplete in freeSolo mode only fires onChange when the user
explicitly selects an option, so values entered via typing-and-tabbing
or browser autofill (notably iOS Chrome) never propagate to the parent
and required-field validation fails on submit.

- Enable Autocomplete autoSelect so the current input value is committed
  on blur, matching MUI's intended pattern for free-text combos.
- Normalize string values inside onChange to the {id, name} shape that
  consumers expect, since autoSelect emits the raw typed string.
@coderabbitai

coderabbitai Bot commented May 15, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 498003a2-926b-4342-b4de-8655813e88b4

📥 Commits

Reviewing files that changed from the base of the PR and between e01d8de and c5935fb.

📒 Files selected for processing (1)
  • src/components/inputs/company-input-v2.js

📝 Walkthrough

Walkthrough

CompanyInputV2 now enables autoSelect on the Autocomplete component and normalizes raw typed strings by trimming and converting them into { id: 0, name: <trimmed> } objects for consistent downstream processing before the change event is emitted.

Changes

Company Input String Normalization

Layer / File(s) Summary
String-to-object normalization on company input change
src/components/inputs/company-input-v2.js
Autocomplete is configured with autoSelect. When a raw typed string is entered, the onChange handler trims it and converts it into a { id: 0, name } object for consistent downstream handling before emitting the change event.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes

Poem

🐇 A company name typed with care,
Now trimmed and shaped as a proper pair,
With id: 0 and name so clean,
The finest normalization I've seen!
Auto-select brings harmony true,
A simple fix, yet it makes things new. ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the primary change: adding blur event handling to commit typed/autofilled values in CompanyInputV2. It directly reflects the main fix introduced in this PR.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/company-input-autofill-commit

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@gcutrini
gcutrini requested a review from smarcet May 15, 2026 23:25

@smarcet smarcet left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@smarcet
smarcet merged commit d0e2c46 into main May 18, 2026
5 checks passed
smarcet pushed a commit that referenced this pull request May 18, 2026
MUI Autocomplete in freeSolo mode only fires onChange when the user
explicitly selects an option, so values entered via typing-and-tabbing
or browser autofill (notably iOS Chrome) never propagate to the parent
and required-field validation fails on submit.

- Enable Autocomplete autoSelect so the current input value is committed
  on blur, matching MUI's intended pattern for free-text combos.
- Normalize string values inside onChange to the {id, name} shape that
  consumers expect, since autoSelect emits the raw typed string.
JpMaxMan added a commit that referenced this pull request Jul 13, 2026
…241)

Follow-up to removing autoSelect. autoSelect (added in #241) committed the
input's DOM value on blur, which is how it captured iOS-Chrome browser autofill
(autofill writes to the DOM without firing onInputChange). The initial onBlur
here read React input state, which would be stale for autofill and reintroduce
the #241 required-field-validation bug.

- onBlur now reads event.target.value (the DOM value), not React state, so
  typed AND autofilled values propagate on blur — while still committing the
  field text, never a highlighted option (the autoSelect-on-hover bug fix
  stands).
- Test: commits a browser-autofilled value on blur even when onInputChange
  never fired. 26/26 green.

Needs a Chrome-iOS device re-test of the #241 autofill path before merge.
smarcet pushed a commit that referenced this pull request Jul 14, 2026
…main (v4.x) (#290)

Port of PR #289 + layered commits from main:
- Remove `autoSelect` (root cause of hover-then-tab wrong-company commits)
- Explicit onBlur that reads event.target.value (DOM value; falls back to
  React input state) so browser autofill still propagates on blur --
  preserves #241/#246's iOS Chrome autofill fix
- `disableClearable`: with free-text supported, an explicit clear (x)
  isn't wanted; users empty by deleting the text
- Synthetic "Use "<typed>"" row prepended to dropdown when the typed text
  isn't already listed; committed cleanly (marker stripped) via onChange
- Extract `getOptionName` helper used by getOptionLabel, filterOptions,
  renderOption; rename local `label` in renderOption to `displayLabel`
  to stop shadowing the outer prop

Tests: +7 (28/28 green), including regression coverage for the hover-then-tab
guard, the DOM-value autofill path, and the Use/free-text commits.
smarcet pushed a commit that referenced this pull request Jul 14, 2026
…"<typed>"" row (#289)

* fix: company-input-v2 — explicit selection only, no clear icon, "Use "<typed>"" row

Root cause of the wrong-company bug: `autoSelect` committed the currently
*highlighted* option on blur, so merely mousing over a suggestion and then
tabbing away silently populated the wrong company.

- Remove `autoSelect`. Selection is now explicit (click / Enter) only.
- New onBlur: tab/click-away commits exactly what was typed — resolved to an
  existing company only on an exact (case-insensitive) name match, else a
  free-text { id: 0, name }. Never commits a merely-highlighted option.
- `disableClearable`: remove the MUI clear (x) icon; the field commits free
  text, so an explicit clear affordance isn't wanted (delete text to empty).
- Append a synthetic 'Use "<typed>"' row so users can explicitly commit their
  text; skipped when it already matches a listed company. Commits a clean
  { id: 0, name }, dropping the display-only marker.
- Predictive typeahead unchanged (freeSolo + server-side queryRegistrationCompanies).

Tests: +5 (blur keeps typed text / never a highlighted option; clear icon never
renders even with a real value; "Use" row commits clean free text; no redundant
"Use" row on exact match). 25/25 green.

* fix: read DOM value in onBlur so autofill still propagates (preserve #241)

Follow-up to removing autoSelect. autoSelect (added in #241) committed the
input's DOM value on blur, which is how it captured iOS-Chrome browser autofill
(autofill writes to the DOM without firing onInputChange). The initial onBlur
here read React input state, which would be stale for autofill and reintroduce
the #241 required-field-validation bug.

- onBlur now reads event.target.value (the DOM value), not React state, so
  typed AND autofilled values propagate on blur — while still committing the
  field text, never a highlighted option (the autoSelect-on-hover bug fix
  stands).
- Test: commits a browser-autofilled value on blur even when onInputChange
  never fired. 26/26 green.

Needs a Chrome-iOS device re-test of the #241 autofill path before merge.

* fix: clear the committed company when the field is emptied on blur (CodeRabbit)

With disableClearable there's no (x), so delete-all-text + blur is the only way
to clear the field — but onBlur previously fired nothing for empty input,
leaving the prior company committed (a required field still read as filled).

- onBlur now propagates null when the field is emptied, guarded so an
  already-cleared field doesn't fire a redundant change. Non-empty
  exact-match / free-text behavior unchanged.
- Tests: clear-on-empty-and-blur; no redundant change on already-empty blur.
  28/28 green.

* fix(company-input-v2): move "Use "<typed>"" row to the top of the dropdown

Prepend the synthetic free-text row instead of appending. Puts the
user's typed text as the primary action (arrow-down + Enter commits it
without scrolling past API suggestions) and matches the intent that
they took the trouble to type.

* refactor(company-input-v2): extract getOptionName helper

getOptionLabel, filterOptions, and renderOption each computed the
option-shape-to-name mapping inline. Extracts a single exported helper
so the string / company-object / malformed cases live in one place.

* refactor(company-input-v2): rename renderOption's local label to displayLabel

The local variable shadowed the outer `label` prop, which is confusing
to skim. Same-block only, no behavior change.

---------

Co-authored-by: Gabriel Horacio Cutrini <gabriel.cutrini@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants