Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issues around pasting into ReactCodeInput #154

Open
wants to merge 21 commits into
base: master
Choose a base branch
from

Conversation

acusti
Copy link

@acusti acusti commented Oct 14, 2021

note that this branch was branched off of #153 and will be easier to review in isolation from those changes.


fixed some issues around pasting, including:

  • Update focus to input after last pasted value or last input, fix Pasting in a PIN should move focus to the last digit #70 631bae4
  • Use type="text" when props.type is 'number' to avoid browser bugs b65e90e
    • fixes Bug in FF for type="number" component #62 (reproducible in chrome and safari also)
    • removes need for special keydown handling for ↑/↓/E keys
    • removes need for extra styles to remove default input type="number" browser styles
    • leaves filtering of non-numeric values to existing logic in the component’s handleChange method. this works for both pasted text and keyboard entry thanks to the input type being text; when input type is number, paste doesn’t trigger a react onChange event and it is impossible to get the behavior 100% correct (inputElement.value is empty even when it actually has characters in it, e.g. the letter e)
  • Remove maxLength from the inputs so filtered pasted values aren’t truncated 3403072
    • when using maxLength, if user has copied the text "123 456" and pastes it into an input rendered by ReactCodeInput with fields={6}, the last digit (“6”) won’t be pasted because the max length on the focused input is 6 digits

changes also include upgrading the package-lock.json file for the latest version of npm (7.24.0)
there is a direct import from prop-types in ReactCodeInput.js
prevents this error after upgrade to storybook v5: webpack/webpack#8082
no more need to add sass loader manually by extending the webpack config
Array.prototype.map() won’t early return if you return true, but Array.prototype.some() will
also, fix naming typo from chart → char
cleans up the code where that variable is used and ensures the value being used is always a number (e.g. in the else condition of the ternary for determining the index within this.textInput when assigning newTarget)
fixes 40818419#62 (reproducible in chrome and safari also)
• removes need for special keydown handling for ↑/↓/E keys
• removes need for styling to remove default input type="number" browser styles
• leaves filtering of non-numeric values to existing logic in handleChange (works for pasted text and keyboard entry)
when using maxLength, if user has "623 293" and pastes it into an input with fields={6}, the last digit (“3”) won’t be pasted because the max length on the focused input is 6 digits
@codecov
Copy link

codecov bot commented Oct 14, 2021

Codecov Report

Merging #154 (1236d1f) into master (2599c5d) will increase coverage by 1.15%.
The diff coverage is n/a.

❗ Current head 1236d1f differs from pull request most recent head 3403072. Consider uploading reports for the commit 3403072 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #154      +/-   ##
==========================================
+ Coverage   96.55%   97.70%   +1.15%     
==========================================
  Files           2        2              
  Lines         145      131      -14     
  Branches       41       35       -6     
==========================================
- Hits          140      128      -12     
+ Misses          5        3       -2     
Impacted Files Coverage Δ
src/ReactCodeInput.js
src/utils.js
utils.js 100.00% <0.00%> (ø)
ReactCodeInput.js 97.63% <0.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2599c5d...3403072. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pasting in a PIN should move focus to the last digit Bug in FF for type="number" component
1 participant