Skip to content
This repository was archived by the owner on Mar 18, 2024. It is now read-only.

New feature - Auto-clear fields when typing. #224

Merged

Conversation

CollinLorenzo
Copy link
Contributor

Summary of Changes

  • Updated the onKeydown event handler in Word.js.

Now, if a user types a valid letter into a character field, it will automatically be cleared out, instead of the user having to click backspace before typing. This is especially beneficial for correcting typos.

Note that I separated 'key' into two constants: key & keyCode.
This is because I am validating the keyCode as opposed to the key to determine if a letter is being typed. Reason being that values like 'Tab', 'Shift' or 'F11' would be included in an if (/[a-zA-Z]/.test(input)) clause, and we don't want fields being cleared out unless a user is typing a valid character into one.

@coveralls
Copy link

coveralls commented Mar 8, 2019

Pull Request Test Coverage Report for Build 477

  • 2 of 3 (66.67%) changed or added relevant lines in 1 file are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage increased (+0.5%) to 87.143%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/js/components/Word.js 2 3 66.67%
Files with Coverage Reduction New Missed Lines %
src/js/components/Word.js 1 87.5%
Totals Coverage Status
Change from base Build 455: 0.5%
Covered Lines: 158
Relevant Lines: 180

💛 - Coveralls

Copy link
Owner

@ollelauribostrom ollelauribostrom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Amazing work 👍 Just a minor nit that needs to be fixed before merging

const key = e.key || e.keyCode;
if (key === 'Enter' || key === 13) {
const { key } = e;
const { keyCode } = e;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could destructure these on the same line (as well as the target),

const { key, keyCode, target } = e;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great suggestion; I've implemented those changes and it looks like it's running smoothly.

@ollelauribostrom ollelauribostrom merged commit 817f00b into ollelauribostrom:master Mar 13, 2019
@ollelauribostrom
Copy link
Owner

Thanks! 👍

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

Successfully merging this pull request may close these issues.

3 participants