soywod / react-pin-field

📟 React component for entering PIN codes.
https://soywod.github.io/react-pin-field/
MIT License
412 stars 24 forks source link

AutoFill doesn't work correctly on Safari #80

Open hraboviyvadim opened 11 months ago

hraboviyvadim commented 11 months ago

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.

soywod commented 11 months ago

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?

hraboviyvadim commented 11 months ago

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 commented 11 months ago

@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)}
        />
      ))}
    </>
  );
...
soywod commented 11 months ago

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 :/

enobrev commented 10 months ago

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?

soywod commented 9 months ago

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!