2
\$\begingroup\$

I have got this working ok, I just think that it can be improved and reduce the amount of code. If anybody could help that would be brilliant.

// Max Length Input
$('input[maxlength]').each(function() {         
    var maxCharLength = $(this).attr("maxlength");  

    if(maxCharLength < 0 || maxCharLength > 5000){
    } else {
        var charLength = $(this).val().length;
        $(this).after("<span class='charCount clearBoth' id='charCount_"+$(this).attr("name").replace(".","_")+"'> " + charLength + " of " + maxCharLength + " characters used</span>");        
        $(this).keyup(function() {              
            var charLength = $(this).val().length;              
            $(this).next("span").html(charLength + ' of ' + maxCharLength + ' characters used');

            if(charLength == maxCharLength ) {
                $(this).next("span").html('<strong> Maximum of ' + maxCharLength + ' characters used.</strong>');
                $(this).next("span").addClass("red");
            } else { $(this).next("span").removeClass("red"); }
        });
    }
});

// Max Length textarea
$('textarea[maxlength]').each(function() {          
    var maxCharLength = $(this).attr("maxlength");  

    if(maxCharLength < 0 || maxCharLength > 5000){
    } else {
        var charLength = $(this).val().length;
        $(this).after("<span class='charCount clearBoth'> " + charLength + " of " + maxCharLength + " characters used</span>");

            $('textarea[maxlength]').keyup(function(){

                var limit = parseInt($(this).attr('maxlength'));
                var text = $(this).val();
                var chars = text.length;

                $(this).next("span").html(chars + ' of ' + limit + ' characters used');

                if(chars > limit){
                    var new_text = text.substr(0, limit);
                    $(this).val(new_text);

                    $(this).next("span").html('<strong> Maximum of ' + limit + ' characters used.</strong>');
                    $(this).next("span").addClass("red");
                } else { $(this).find("span").removeClass("red"); }
            });
    }
});
\$\endgroup\$
2
\$\begingroup\$

There are a few things I would suggest:
1. Replace you numbers with constants and revert the condition to get rid of 'else':

var minTextLength = 0;
var maxTextLength = 5000;
if (maxCharLength >= minTextLength || maxCharLength <= maxTextLength){
      //Your code...
    }

2. Move the block of code:

$(this).next("span").html('<strong> Maximum of ' + maxCharLength + ' characters used.</strong>');
            $(this).next("span").addClass("red");
        } else { $(this).next("span").removeClass("red"); }

To a separate function.

And there are a couple of things that confuse me:
1. In 'Max Length textarea' you have:

$('textarea[maxlength]').keyup()

Is it correct? Becasue it means that you'll add a lot of handlers to all your textarea[maxlength].

2 . In your 'Max Length Input' you have:

if(charLength == maxCharLength )

Maybe it could be replaced with:

if(charLength > maxCharLength )

Depending on the answers to these questions, maybe it'll be possible to move the keyup handler to a separate function. And maybe even the whole code from both 'Max Length textarea' and 'Max Length Input'.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.