fix: make browser autofill work - #266
Conversation
* Determine the autocomplete token and input type based on field.type.name * Replace the hidden input for the country and province fields with a "visually" hidden <select> element * Add handleNativeSelectChange to keep the visible dropdown in sync when the browser fills the select * setAllFieldsTouched now supports the *select* elements for country and province.
| <div> | ||
| <input type="hidden" name={name} defaultValue="" ref={hiddenFieldRef} /> | ||
| {/* a `<select>`, not `type="hidden"` browsers only autofill what they render */} | ||
| <select |
There was a problem hiding this comment.
Does <input autoComplete="country"/> work?
Rendering 200+ option elements isn't ideal
There was a problem hiding this comment.
i tried it but doesn't work. i searching why and i think it's because the input already gets a value from the card's country before input showed up, and browser won't override an input that already has a value. that's why i use select + option, and select + option doesn't have that problem though. Also yeah, rendering 200+ options felt wrong to me too at first, so I wrapped it in a memo. Should be fine to leave as is?
| position: "absolute", | ||
| width: "1px", | ||
| height: "1px", | ||
| overflow: "hidden", | ||
| clipPath: "inset(50%)", | ||
| whiteSpace: "nowrap", | ||
| pointerEvents: "none", |
There was a problem hiding this comment.
clipPath is hiding everything here, but should probably also remove the margin, padding, and border
| // the `<select>` in dropdown mode, or the visible input in free text mode | ||
| const hiddenFieldRef = useRef<HTMLInputElement | HTMLSelectElement | null>( | ||
| null, | ||
| ); | ||
| const setFieldRef = useCallback( | ||
| (element: HTMLInputElement | HTMLSelectElement | null) => { | ||
| hiddenFieldRef.current = element; | ||
| }, | ||
| [], | ||
| ); |
There was a problem hiding this comment.
Why change to a function ref here? Also hiddenFieldRef is no longer "hidden"
There was a problem hiding this comment.
so province can render either a select or an input depending on the country, and the ref needs to point to whichever one shows up. and i can't pass one ref object to both since typescript wants it typed to one specific element, so wrapping it in a function was the easiest way that i think?, i can add comment to that if you want. also gonna renaming hiddenFieldRef, thankyou
| [options], | ||
| ); | ||
|
|
||
| const handleNativeSelectChange = useCallback( |
There was a problem hiding this comment.
Can you add tests for all the handleNativeSelectChange functions?
* Add margin/padding/border to the visually-hidden select styling * Rename hiddenFieldRef to valueFieldRef and explain why a callback ref is needed * Add tests for handleNativeSelectChange covering both country and province * Fix a bug surfaced by those tests: province could silently lose its value when switching between dropdown and free-text mode
simon-paris
left a comment
simon-paris
left a comment
There was a problem hiding this comment.
Looks good, but can you also assign autocomplete values to the cards address form in the test-data? The server config has them but the mock doesn't
|
Oh and could you also make the phone areacode autofill properly? |
sure, gonna fix that |
2 participants
Determine the autocomplete token and input type based on field.type.name
Replace the hidden input for the country and province fields with a "visually" hidden element Add handleNativeSelectChange to keep the visible dropdown in sync when the browser fills the select setAllFieldsTouched now supports the select elements for country and province.