Merging two simple javascript functions into one

Question

I have the following code in my index.html for an input field:

<input id="inpMinutes" type="number" style="width: 50px; text-align: right" min="1" value="1" onkeypress="return isNumberKey(event)" />
<span id="minuteS">minute</span>

I want to keep the entered number between 1 and 600 without allowing entering any non digit characters. This is done by 2 different functions in my main.js file, but I would like to merge them, but I am very noob at javascript, I just stole and modified this code.

// keep out non-digits from the input field that calls this
function isNumberKey(evt) {
    var charCode = (evt.which) ? evt.which : event.keyCode
    if (charCode > 31 && (charCode < 48 || charCode > 57))
        return false;
    return true;
}

$("#inpMinutes").on("input", function() {
    // do not allow to enter a number less than 1 into the minutes field (minus sign is prevented by isNumberKey())
    if ( this.value == 0 ) {
        this.value = 1;
    // do not allow to enter a number larger than 600
    } else if ( this.value > 600 ) {
        this.value = 600;
    }
    // make the "minute" word grammatically correct
    if ( this.value > 1 ) {
        minuteS.innerHTML = "minutes";
    } else {
        minuteS.innerHTML = "minute";
    }
});

What is the most elegant (most efficient?) way to merge these into one single function? Thanks in advance.

I think you shouldn't, for a couple of reasons. a) They serve different purposes. Best practise is to keep a function single-purpose. b) They are called on different occasions. Key blocking is done as you type, while input validation is usually done onblur (very annoying if you do that while typing). c) Be aware that you can still paste invalid characters, so you should validate the input anyway. So: always validate the entire string on blur to check for invalid characters. — GolezTrol
– GolezTrol, Commented Jun 4, 2015 at 20:21
Suggestion solution: set type=number to make (some) browsers enforce the numbers itself, and always validate yourself. Still, this is not guaranteed, so you should validate again on 'input', and also validate again on the server. Life is a female dog... — GolezTrol
– GolezTrol, Commented Jun 4, 2015 at 20:22
The type is already set to number, but when I tested it, it didn't want to work (using the latest firefox and chrome). Yeah, maybe you are right that it is better to keep them separated, I was also thinking about the possibility that later I may need to call only one of them from another input field (for example). I haven't met this onblur thing yet, but I like better the instant correction than the other case, when correction is done only when the input field is left. (I tried to paste invalid string into the field but I was not able.) I like your answers, learned something from them, thanks! — Tom
– Tom, Commented Jun 4, 2015 at 21:27

BluePill · Accepted Answer · 2015-06-04 22:06:25Z

1

Working demo: https://jsfiddle.net/20m6eLxx/

HTML

<input id="inpMinutes" type="number" min="1" value="1" data-oldvalue="1" data-regex="^([0-9]{1,2}|[1-5][0-9]{2}|600)$" />
<span id="minuteS">minute</span>

JS

$('#inpMinutes').on('input', function (e) {
    var $target = $(e.target),
            val = $target.val(),
            rgx = new RegExp($target.attr('data-regex'));

    if (!rgx.test(val)) {
        $target.val($target.attr('data-oldvalue'));
        return;
    }
    $target.attr('data-oldvalue', $target.val());
    $('#minuteS').html(val > 1 ? 'minutes' : 'minute');
});

To preserve the behavior of showing the max value when a >max value is typed, see this update: https://jsfiddle.net/20m6eLxx/1/

You could also generify this a little bit if you wanted to, extending it to all inputs that need validation and restricted keying, by broadening your selector, e.g.:

//<input class="restricted others" data-regex="etc." />
$('.restricted').on('input', function (e) {...... 
or
//for any input that has the data-regex attribute
$('input[data-regex]').on('input', function (e) {......

A small piece of advice

Unless you really know what you're doing, please be very considerate towards cancelling event defaults and bubbling. I've broken a few keyboards as a result of rage when tabs, ctrl + modifiers etc. didn't work because of extremely unthoughtful key blocking. It's a usability disaster unless well thought out, and well thought out is usually rare when it comes to forms and validation (imho).

edited Jun 4, 2015 at 22:06

answered Jun 4, 2015 at 20:47

BluePill

5153 silver badges11 bronze badges

Sign up to request clarification or add additional context in comments.

7 Comments

Tom Over a year ago

This looks cool. (Only one mistake I could find: you should have used "val > 1" instead of "val > 60") Thanks for your advice, I havent thought about those things yet, but as I tested, for example Tab works in the field (I can leave it with Tab), also backspace and arrows work too, and I don't need more. I probably will use this solution, but now I am going to sleep and will check back tomorrow. :)

BluePill Over a year ago

@Tom Ah yes, well spotted - corrected. I was thinking "seconds" all the time, for some reason. And the reason tab and other keys work is because I didn't touch the event propagation at any point. Cheers.

Tom Over a year ago

I am interested in generifying the checks, because I will need to use these kinds of checks later, but I just don't know (yet) how to use these $('.restricted') and $('[data-regex]') things... How should the HTML code looke like in those cases? And one more question: I kinda like in my original code, that when I press the number "1" four times in the field, it doesn't stop at "111", but the number is lowered down from "1111" to "600". How can I achieve this with your code? Thanks very much!

BluePill Over a year ago

@Tom Link added for preserving that behavior. Simple clause inside the regex rejector. Check the new fidddle. Add a max attribute to the element, and that little clause inside the first regex checking block. jsfiddle.net/20m6eLxx/1

BluePill Over a year ago

@Tom Also, HTML snippets added in the code (see answer) for those selectors you asked about.

|

hamecoded · Accepted Answer · 2015-06-04 20:43:01Z

0

why not move the entire login to the isNumberKey function. Build the to be value by merging the character stroke to the input value to check whether it stays in range and make the "minute" word grammatically correct, then return false to prevent the propagation, counting on only what you've set explicitly.

answered Jun 4, 2015 at 20:43

hamecoded

1,1197 silver badges5 bronze badges

1 Comment

Tom Over a year ago

This was my first thought, but I didn't know (still don't know) how to do it, because for example I have no clue what this line is doing: "var charCode = (evt.which) ? evt.which : event.keyCode" Ok, I can guess, but I don't know the exact details. It is some kind of if/else I guess... And I also don't know hot to check the value of the field.

Collectives™ on Stack Overflow

Merging two simple javascript functions into one

2 Answers 2

7 Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

7 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related