From 928701b5d16810c095977fee0e4c3e4bf44487bd Mon Sep 17 00:00:00 2001 From: Gabriel Horacio Cutrini Date: Tue, 14 Jul 2026 13:21:47 -0300 Subject: [PATCH] feat(company-input-v2): port free-text row + hover-then-tab fix from main (v4.x) 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 """ 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. --- .../inputs/__tests__/company-input-v2.test.js | 157 ++++++++++++++++-- src/components/inputs/company-input-v2.js | 94 +++++++++-- 2 files changed, 221 insertions(+), 30 deletions(-) diff --git a/src/components/inputs/__tests__/company-input-v2.test.js b/src/components/inputs/__tests__/company-input-v2.test.js index 6cd0479a..809785de 100644 --- a/src/components/inputs/__tests__/company-input-v2.test.js +++ b/src/components/inputs/__tests__/company-input-v2.test.js @@ -178,6 +178,79 @@ describe("CompanyInputV2 integration", () => { return { ...utils, getValue: () => setValue }; }; + it("on blur takes the typed text and, when it matches no option, commits it as free text (never a highlighted option)", () => { + // The bug this guards: with autoSelect, mousing over a suggestion and + // tabbing away committed that highlighted company. Now blur must commit + // exactly what was typed. + let resolveQuery; + queryRegistrationCompanies.mockImplementation((_summitId, _input, cb) => { + resolveQuery = cb; + }); + + const onChange = jest.fn(); + renderControlled({ onChange }); + const input = screen.getByRole("combobox"); + + // Type "Tip"; two suggestions come back (either could get highlighted on hover). + fireEvent.change(input, { target: { value: "Tip" } }); + act(() => { resolveQuery([{ id: 1, name: "Tipit" }, { id: 2, name: "Tipco" }]); }); + + // Tab away without picking anything. + fireEvent.blur(input); + + const committed = onChange.mock.calls.map((c) => c[0].target.value).pop(); + // Exactly what was typed, as a free-text entry — not id 1 or id 2. + expect(committed).toEqual({ id: 0, name: "Tip" }); + }); + + it("commits a browser-autofilled value on blur even when it never fired onInputChange (#241 iOS Chrome)", () => { + // Autofill writes straight to the DOM without React's onInputChange, so + // the component's input state stays empty. The blur handler must read + // the DOM value (event.target.value), not React state, or required-field + // validation regresses — the exact bug #241's autoSelect fixed. + queryRegistrationCompanies.mockImplementation(() => {}); + + const onChange = jest.fn(); + renderControlled({ onChange }); + const input = screen.getByRole("combobox"); + + // Simulate autofill: set the DOM value directly (no fireEvent.change -> + // no onInputChange), then blur. + input.value = "Autofilled Co"; + fireEvent.blur(input); + + const committed = onChange.mock.calls.map((c) => c[0].target.value).pop(); + expect(committed).toEqual({ id: 0, name: "Autofilled Co" }); + }); + + it("clears the committed company when the field is emptied (delete-all-text) and blurred", () => { + // With disableClearable there's no (x), so deleting the text and blurring + // is the only way to clear — the value must propagate as null. + queryRegistrationCompanies.mockImplementation(() => {}); + + const onChange = jest.fn(); + renderControlled({ initialValue: { id: 1, name: "Tipit" }, onChange }); + const input = screen.getByRole("combobox"); + + fireEvent.change(input, { target: { value: "" } }); + fireEvent.blur(input); + + const committed = onChange.mock.calls.map((c) => c[0].target.value).pop(); + expect(committed).toBeNull(); + }); + + it("does not fire a redundant change when an already-empty field is blurred", () => { + queryRegistrationCompanies.mockImplementation(() => {}); + + const onChange = jest.fn(); + renderControlled({ initialValue: null, onChange }); + const input = screen.getByRole("combobox"); + + fireEvent.blur(input); + + expect(onChange).not.toHaveBeenCalled(); + }); + it("on blur commits the canonical existing match when the typed text matches case-insensitively", () => { // Capture the API callback so we can resolve it manually. let resolveQuery; @@ -206,25 +279,31 @@ describe("CompanyInputV2 integration", () => { }); it("auto-replaces a free-text commit with the canonical match when the API response arrives after blur", () => { - // Withhold the API callback to simulate the network being slower than blur. - let resolveQuery; - queryRegistrationCompanies.mockImplementation((_summitId, _input, cb) => { - resolveQuery = cb; - }); + // Withhold the API callback so we control when the response "arrives". + queryRegistrationCompanies.mockImplementation(() => {}); const onChange = jest.fn(); - // Start with the free-text commit already in place (what happens when - // blur fires before the response). - renderControlled({ initialValue: { id: 0, name: "tipit" }, onChange }); + renderControlled({ onChange }); const input = screen.getByRole("combobox"); - // Type to populate inputValue so the effect kicks in. + // Type "tipit": fires onInputChange, populates inputValue, triggers the effect. fireEvent.change(input, { target: { value: "tipit" } }); - // Now the response arrives with the canonical option. - act(() => { resolveQuery([{ id: 1, name: "Tipit" }]); }); + // Blur: autoSelect commits the typed string as free-text { id: 0, name: "tipit" } + // via the wrapper's onChange handler, which writes it back to value. The + // value change re-runs the effect with the new normalizedValue closure. + fireEvent.blur(input); + onChange.mockClear(); + + // Pull the API callback from the most recent effect run (the one with + // the post-blur normalizedValue captured in its closure). + expect(queryRegistrationCompanies).toHaveBeenCalled(); + const [, , cb] = queryRegistrationCompanies.mock.calls[queryRegistrationCompanies.mock.calls.length - 1]; + + // Now the API response arrives with the canonical option. The effect + // sees the active value is a free-text match and auto-replaces it. + act(() => { cb([{ id: 1, name: "Tipit" }]); }); - // The component should have called onChange with the canonical option. const promoted = onChange.mock.calls .map((c) => c[0].target.value) .find((v) => v && typeof v === "object" && v.id === 1); @@ -244,4 +323,58 @@ describe("CompanyInputV2 integration", () => { // MUI's clear button uses aria-label="Clear". expect(screen.queryByLabelText("Clear")).not.toBeInTheDocument(); }); + + it("never renders the clear icon, even with a real selected company (disableClearable)", () => { + queryRegistrationCompanies.mockImplementation(() => {}); + render( + {}} + /> + ); + expect(screen.queryByLabelText("Clear")).not.toBeInTheDocument(); + }); + + it("offers a 'Use \"\"' option that commits the typed text as a free-text entry", () => { + let resolveQuery; + queryRegistrationCompanies.mockImplementation((_summitId, _input, cb) => { + resolveQuery = cb; + }); + + const onChange = jest.fn(); + renderControlled({ onChange }); + const input = screen.getByRole("combobox"); + + // Type a name with no existing match; the listbox opens with just the + // synthetic "Use ..." row. + fireEvent.change(input, { target: { value: "Acme" } }); + act(() => { resolveQuery([]); }); + + const useOption = screen.getByText('Use "Acme"'); + fireEvent.click(useOption); + + const committed = onChange.mock.calls + .map((c) => c[0].target.value) + .find((v) => v && typeof v === "object" && v.id === 0); + // Committed clean, without the display-only marker. + expect(committed).toEqual({ id: 0, name: "Acme" }); + }); + + it("does not offer the 'Use' row when the typed text already matches an option", () => { + let resolveQuery; + queryRegistrationCompanies.mockImplementation((_summitId, _input, cb) => { + resolveQuery = cb; + }); + + renderControlled({}); + const input = screen.getByRole("combobox"); + + fireEvent.change(input, { target: { value: "Tipit" } }); + act(() => { resolveQuery([{ id: 1, name: "Tipit" }]); }); + + // The real company shows; no redundant "Use "Tipit"" row. + expect(screen.queryByText('Use "Tipit"')).not.toBeInTheDocument(); + }); }); diff --git a/src/components/inputs/company-input-v2.js b/src/components/inputs/company-input-v2.js index dd3848bf..6fec35eb 100644 --- a/src/components/inputs/company-input-v2.js +++ b/src/components/inputs/company-input-v2.js @@ -25,7 +25,7 @@ export const isCompanyObject = (o) => export const isExistingCompany = (o) => isCompanyObject(o) && o.id > 0; // A company name the user typed that isn't in the database yet -// (id === 0 is the sentinel autoSelect uses for free-text values). +// (id === 0 is the sentinel used for free-text values). export const isNewCompany = (o) => isCompanyObject(o) && o.id === 0 && !!o.name.trim(); // Find an existing company in `candidates` whose name matches `name` @@ -49,6 +49,16 @@ export const normalizeCompanyValue = (v) => { return null; }; +// Extract the display name from an option. String options come through when +// the consumer passes value as a plain string; object options carry `name`. +// Returns "" for null/undefined/malformed shapes so callers can chain string +// ops without null guards. +export const getOptionName = (option) => { + if (typeof option === "string") return option; + if (isCompanyObject(option)) return option.name; + return ""; +}; + const CompanyInputV2 = ({ summitId, isRequired, sx, onChange, id, name, label, value, error, helperText, onBlur, placeholder, options2Show, disableShrink, ...rest }) => { const [inputValue, setInputValue] = React.useState(""); const [options, setOptions] = React.useState([]); @@ -110,25 +120,60 @@ const CompanyInputV2 = ({ summitId, isRequired, sx, onChange, id, name, label, v name={name} options={options} autoComplete - autoSelect freeSolo + // No clear (x) icon: the field commits free text, so an explicit clear + // affordance isn't wanted; users empty it by deleting the text. + disableClearable includeInputInList filterSelectedOptions value={normalizedValue} - onBlur={() => { if (onBlur) onBlur(name) }} - getOptionLabel={(option) => { - if (typeof option === "string") return option; - return option.name; + // NOTE: `autoSelect` is intentionally NOT set. With it, blurring the field + // committed the currently *highlighted* option — so merely mousing over a + // suggestion and then tabbing away silently populated the wrong company. + // Selection is now explicit (click / Enter) only; see onBlur for the + // "keep what was typed" fallback. + onBlur={(event) => { + // On blur with no explicit selection, commit the field's value as-is. + // Read the *DOM* value (event.target.value), NOT React input state: + // browser autofill (notably iOS Chrome) can populate the field without + // firing onInputChange, so the React state would be stale. Reading the + // DOM value is what MUI's `autoSelect` did internally — this preserves + // the typed/autofilled-value-on-blur fix (#241) — but we commit the + // field *text*, never a highlighted option, so hovering a suggestion and + // tabbing away no longer selects the wrong company (why autoSelect was + // removed). Resolve to an existing company only on an exact + // (case-insensitive) name match, else free-text { id: 0, name }. Skip + // when the text already matches the committed value (e.g. right after an + // explicit selection). + const typed = (event?.target?.value ?? inputValue).trim(); + const currentName = isCompanyObject(normalizedValue) + ? normalizedValue.name + : (typeof normalizedValue === "string" ? normalizedValue : ""); + if (!typed) { + // Field emptied (delete-all-text). With disableClearable there's no + // (x), so this is the only way to clear — propagate null. Skip if + // already cleared to avoid a redundant change. + if (normalizedValue) fireChange(null); + } else if (typed.toLowerCase() !== currentName.trim().toLowerCase()) { + fireChange(findExistingByName(options, typed) || { id: 0, name: typed }); + } + if (onBlur) onBlur(name); }} + getOptionLabel={getOptionName} onChange={(_, newValue) => { let tmpValue = newValue; - // autoSelect commits the raw typed/autofilled string on blur. If the - // string matches an existing company case-insensitively, pick that - // option (so typing "tipit" tabs out to "Tipit"). Otherwise commit - // the typed value as a free-text {id: 0, name} entry. + // freeSolo commits the raw typed string when the user presses Enter + // without picking an option (reason "createOption"). If the string + // matches an existing company case-insensitively, pick that option (so + // "tipit" + Enter resolves to "Tipit"); otherwise commit it as a + // free-text {id: 0, name} entry. if (typeof tmpValue === "string" && tmpValue.trim()) { const trimmed = tmpValue.trim(); tmpValue = findExistingByName(options, trimmed) || { id: 0, name: trimmed }; + } else if (tmpValue && typeof tmpValue === "object" && tmpValue.isFreeTextOption) { + // The synthetic "Use "…"" row: commit a clean free-text entry, + // dropping the display-only marker. + tmpValue = { id: 0, name: tmpValue.name }; } // Prepend the committed value but drop any existing entry with // the same id; otherwise resolving to an existing company would @@ -147,9 +192,22 @@ const CompanyInputV2 = ({ summitId, isRequired, sx, onChange, id, name, label, v onInputChange={(_, newInputValue) => { setInputValue(newInputValue); }} - // The API already filters server-side; disable MUI's client-side filtering - // so all returned matches stay visible regardless of substring match. - filterOptions={(opts) => opts} + // The API already filters server-side, so all returned matches stay + // visible (no client-side substring filtering). We *prepend* a synthetic + // "Use """ row so the user can explicitly commit their free text + // when it isn't already listed. First position makes the typed text the + // primary action (arrow-down + Enter commits without scrolling past + // suggestions) and matches the user's intent that they took the trouble + // to type. + filterOptions={(opts, params) => { + const trimmed = params.inputValue.trim(); + const alreadyListed = trimmed && opts.some( + (o) => getOptionName(o).trim().toLowerCase() === trimmed.toLowerCase() + ); + return trimmed && !alreadyListed + ? [{ id: 0, name: trimmed, isFreeTextOption: true }, ...opts] + : opts; + }} renderInput={(params) => ( { const { key, ...optionProps } = props; - // Mirror getOptionLabel: string options come through when the - // consumer passes value as a plain string. Without this guard - // those rows render empty. - const label = typeof option === "string" ? option : option?.name; + const optionName = getOptionName(option); + // The synthetic free-text row reads Use "" so it's clearly a + // commit-what-I-typed action, not a matched company. + const displayLabel = option?.isFreeTextOption ? `Use "${optionName}"` : optionName; return ( // eslint-disable-next-line react/jsx-props-no-spreading
  • @@ -177,7 +235,7 @@ const CompanyInputV2 = ({ summitId, isRequired, sx, onChange, id, name, label, v variant="body2" sx={{ fontSize: "1em", color: "text.secondary", padding: "5px 0" }} > - {label} + {displayLabel}
    • );