react-pin-field icon indicating copy to clipboard operation
react-pin-field copied to clipboard

AutoFill doesn't work correctly on Safari

Open hraboviyvadim opened this issue 1 year ago • 6 comments

Hi! Thanks for this lib, its awesome! But there is an issue which seems to be critical. In Safari (both mobile and desktop) autoFill works incorrect - it inserts code from sms in first input ignoring others:

image

Also, same issue when you get one-time-code from email (new feature from MacOS Sonoma, Safari 17): image

I've tried to debug a little and it seems onChange event doesn't fire for autofill.

hraboviyvadim avatar Sep 16 '23 17:09 hraboviyvadim

In Safari (both mobile and desktop) autoFill works incorrect - it inserts code from sms in first input ignoring others

Also, same issue when you get one-time-code from email (new feature from MacOS Sonoma, Safari 17)

It may be related to https://github.com/soywod/react-pin-field/issues/63. Did you use the latest version v3.1.4?

I've tried to debug a little and it seems onChange event doesn't fire for autofill.

The issue was similar on Firefox mobile, onChange was not triggered because the event.key was not standard. I needed to add this line to make it work:

https://github.com/soywod/react-pin-field/blob/61abac38660192f402ef9572bc9e2baea9614d4d/lib/src/pin-field/pin-field.tsx#L109

Maybe you could try to log here and see what is the event.key when the autocomplete is triggered?

soywod avatar Sep 17 '23 05:09 soywod

Yes, its 3.1.4.

I'll try to debug more. Hope to get some free time this week. I'll let you know if will figure out something.

hraboviyvadim avatar Sep 18 '23 14:09 hraboviyvadim

@soywod I've been playing around with this issue locally and figured out that using onChange for inputs with same handler as for onPaste makes it work for autoFill in Safari 17. Are there any reasons why you didn't use onChange for tracking input value changes?

pin-field.tsx

 ...
  const handleChange = (idx: number) => {
    return function (evt: React.ChangeEvent<HTMLInputElement>) {
      evt.preventDefault();
      const val = evt.target.value;

      dispatch({ type: 'handle-paste', idx, val });
    };
  };

  return (
    <>
      {range(0, codeLength).map((idx) => (
        <input
          type="text"
          autoCapitalize="off"
          autoCorrect="off"
          autoComplete="off"
          inputMode="text"
          {...inputProps}
          key={idx}
          ref={setRefAtIndex(idx)}
          autoFocus={hasAutoFocus(idx)}
          onFocus={handleFocus(idx)}
          onKeyDown={handleKeyDown(idx)}
          onKeyUp={handleKeyUp(idx)}
          onPaste={handlePaste(idx)}
          onChange={handleChange(idx)}
        />
      ))}
    </>
  );
...

hraboviyvadim avatar Sep 20 '23 15:09 hraboviyvadim

Are there any reasons why you didn't use onChange for tracking input value changes?

onChange is not triggered for non-visible characters like backspace or arrows, and some old browsers do not support it properly (hence the KeyboardEvent.key polyfill). I found it simpler to track changes using onKeyDown and onKeyUp, but somehow it does not work as expected on Safari. I have no macOS nor iOS so it is hard for me to test :/

soywod avatar Sep 20 '23 19:09 soywod

That seems reasonable as to why you're using the other listeners, but @hraboviyvadim was not suggesting replacing the existing listeners, but rather including an additional onChange listener as well.

Are there issues with adding the onChange handler as well?

enobrev avatar Oct 11 '23 23:10 enobrev

Are there issues with adding the onChange handler as well?

I could add the listener, I'm just afraid it may become overcomplicated. onChange would be used only for a subpart (not compatible browsers for visible characters only). I will give a try and see how it behaves!

soywod avatar Nov 03 '23 06:11 soywod