Skip to content

Code refactor for maintainability and some bug fixes - #12

Closed
FeIronMonkey wants to merge 7 commits into
kabachello:masterfrom
FeIronMonkey:revert_unintended_changes
Closed

FeIronMonkey wants to merge 7 commits into
kabachello:masterfrom
FeIronMonkey:revert_unintended_changes

Conversation

@FeIronMonkey

@FeIronMonkey FeIronMonkey commented Aug 6, 2021 •

Copy link
Copy Markdown

Fixed bug where programmatically closing the numpad called the open function instead.
Fixed bug (maybe) where conflicts with the locale based number formatting on input fields with type="number" causes error.
Fixed bug where limit of x characters counted the decimal separator (example: limit of 4 digits only allowed 3333 or 33.3).
Fixed bug where comma as decimal separator did not allow the value to turn negative.

Note: Although almost every line was changed, most of the changes were variable renaming, function renaming, movement of blocks of code into separate functions, or the addition of braces and/or empty space. With the exception of a few bug fixes, the logic is mostly the same.

Refactored code into a class which uses JQuery's $.extend() method to extend itself with the JQuery object of the newly created numpad element. All functions in this class are arrow functions, so the meaning of the keyword this will always be class-scoped, regardless of where the function is called. Most functions were prefixed with numpad_ to ensure no conflict with other functions on the JQuery object extended by the class.

The intent of this refactoring is to make the code easier to read for all and more accessible to those who are less accustomed to JavaScript's quirks (like this meaning something different depending on where the function is being called from).

Frederick Resnick added 4 commits August 5, 2021 17:54
…e the project maintainable and not too daunting for those just checking it out to use or modify. This is a great project, full of excellent ideas. Mostly eliminated the lawlessness that JavaScript invites by organizing almost everything into functions and a class. All class functions are arrow functions, so there are no surprises lurking in a call to "this" which does not explicitly affect the class it was called in. Demos are updated, too.
@FeIronMonkey

Copy link
Copy Markdown
Author

@kabachello The last set of approved changes to this project was 6 years ago, yet your project still appeared on a "top 12" list of best JavaScript pop-up keypads/keyboards (https://www.learningjquery.com/2017/04/12-awesome-jquery-virtual-keyboard-plugins). It would be great to see this project stay up-to-date with bug fixes and improvements. The changes I made in this PR were for myself, really, so I could manage to edit this project as needed for use in my own work, but figured if they let me read and understand the code better, they might lend well to helping others do the same.

@FeIronMonkey
FeIronMonkey force-pushed the revert_unintended_changes branch from 0af1e01 to db2f8ff Compare August 6, 2021 15:00
@FeIronMonkey FeIronMonkey mentioned this pull request Aug 6, 2021
@FeIronMonkey

Copy link
Copy Markdown
Author

I have since fixed a great number of bugs and added features. This PR is out of date, but I can open another if you want. As I know of a few bugs in this PR, I will close it.

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.

1 participant