square / maker

Maker Design System by Square
https://square.github.io/maker/styleguide/latest-stable/
Other
63 stars 14 forks source link

Pin Input does not autofocus first cell on shake/clear (if previously disabled) #258

Closed tatems closed 2 years ago

tatems commented 2 years ago

Bug description

When clearing/shaking a pin input that was previously disabled, the pin focus is not updated correctly.

eg ->

Reproduction

This is currently reproducible on the docs site with the async input

https://user-images.githubusercontent.com/3402466/158885078-1b37eec3-b94c-4744-980c-56955da69a8b.mov

Environment

System:
    OS: macOS 12.2.1
    CPU: (16) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 2.12 GB / 64.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 12.18.2 - ~/.nvm/versions/node/v12.18.2/bin/node
    npm: 6.14.5 - ~/.nvm/versions/node/v12.18.2/bin/npm
  npmPackages:
    @square/maker: ^6.7.3 => 6.7.1 
    vue: ^2.6.14 => 2.6.14 
    webpack: ^5.1.3 => 5.1.

Addressed by

No response

Can you contribute a fix?

pretzelhammer commented 2 years ago

cc @Ryguy11

pretzelhammer commented 2 years ago

hey @tatems

this issue is caused by: it's not possible to focus a disabled input. this is true for plain HTML/DOM, so it's not a pininput-specific issue.

the reason the problem exists for pininputs is that the example given in the styleguide for the mocked api call is because it attempts to focus the internal input before it's been un-disabled, this invalid sequencing can be fixed in a couple ways:

  1. do not disable MPinInput while you're asynchronously validating the code or
  2. un-disable MPinInput FIRST, then clearAndShake it SECOND

I update the PinInput docs in this PR to show how 2 can be done. You can verify that the fix fixes the issue in this styleguide deploy. If that PR fixes your issue please approve it. I'll merge it in and close this issue. Otherwise, please let me know how/why the fix doesn't apply to your situation and I'll work on an alternative fix.

pretzelhammer commented 2 years ago

I'm gonna assume this merged PR with updated PinInput docs has fixed the issue, but if you have any additional questions or concerns feel free to re-open this issue.