There are a number of techniques to apply that can help you with this issue.
The code that we’re starting with I’ve put up on to jsfiddle at https://jsfiddle.net/pmw57/dL0oy6pb/
Use addEventListener instead of Inline HTML event attributes
The inline event handler attributes such as onClick and onKeyUp have for a long time now been superseded by more superior JavaScript-based techniques.
Instead of using onKeyUp we can migrate the code to use addEventListener.
I’ll use comments here to show the old line of code, followed by its replacement.
<!--<textarea onkeyup="textCounter(this,'counter',10);" id="message">-->
<textarea id="message">
var message = document.querySelector("#message");
message.addEventListener("keyup", function (evt) {
textCounter(this, 'counter', 10);
});
No breaks please
Before going further there is another problem. Break elements should not be used unless you are marking up addresses, or poetry. Paragraph elements are best used to achieve the formatting that you require.
<p>
<textarea id="message"></textarea>
<!--<input disabled maxlength="3" size="3" value="10" id="counter"><br><br>-->
<input disabled maxlength="3" size="3" value="10" id="counter">
</p>
Remove looping barriers
Currently the code uses HTML identifiers of message
and counter
. That’s fine with one target, but we want to be able to handle 10 of them. or potentially thousands.
Let’s put in a second textarea and see what needs to be done to get them both working.
<p>
<textarea id="message"></textarea>
<input disabled maxlength="3" size="3" value="10" id="counter">
</p>
<p>
<textarea id="message"></textarea>
<input disabled maxlength="3" size="3" value="10" id="counter">
</p>
Duplicate id attributes are not allowed.
We could individually number the id’s instead, but where there’s two there can be thousands, and you don’t want to individually number thousands.
Class names are the solution.
<!--<textarea id="message"></textarea>-->
<textarea class="message"></textarea>
<!--<input disabled maxlength="3" size="3" value="10" id="counter"><br><br>-->
<input disabled maxlength="3" size="3" value="10" class="counter"><br><br>
<!--<textarea id="message"></textarea>-->
<textarea class="message"></textarea>
<!--<input disabled maxlength="3" size="3" value="10" id="counter"><br><br>-->
<input disabled maxlength="3" size="3" value="10" class="counter"><br><br>
With the textCounter function, we can remove its reliance on id’s and pass it an element for the countfield instead. That way, the function consistently has elements to deal with.
// function textCounter(field, field2, maxlimit) {
function textCounter(field, countfield, maxlimit) {
// var countfield = document.getElementById(field2);
if (field.value.length > maxlimit) {
field.value = field.value.substring(0, maxlimit);
return false;
} else {
countfield.value = maxlimit - field.value.length;
}
}
var message = document.querySelector(".message");
message.addEventListener("keyup", function(evt) {
var countfield = document.querySelector(".counter");
// textCounter(this, 'counter', 10);
textCounter(this, countfield, 10);
});
We have transitioned over to using classes, and the first counter works. Next up is to get the other counter working.
Use querySelectorAll for multiple working counters
We can use querySelectorAll to get all matching elements, letting us retrieve either the first or the second from that list of elements.
// var message = document.querySelector(".message");
var message = document.querySelectorAll(".message")[0];
message.addEventListener("keyup", function(evt) {
// var countfield = document.querySelector(".counter");
var countfield = document.querySelectorAll(".counter")[0];
textCounter(this, countfield, 10);
});
var message = document.querySelectorAll(".message")[1];
message.addEventListener("keyup", function(evt) {
var countfield = document.querySelectorAll(".counter")[1];
textCounter(this, countfield, 10);
});
https://jsfiddle.net/pmw57/dL0oy6pb/3/
Use a loop
Now that we have multiple indexes being used, we can use a loop for them instead.
// var message = document.querySelectorAll(".message")[0];
var messages = document.querySelectorAll(".message");
messages.forEach(function (message, index) {
message.addEventListener("keyup", function(evt) {
// var countfield = document.querySelectorAll(".counter")[0];
var countfield = document.querySelectorAll(".counter")[index];
textCounter(this, countfield, 10);
});
});
That works, but there are some final issues to deal with.
https://jsfiddle.net/pmw57/dL0oy6pb/4/
Remove extraneous index variable
The index variable is an unnecessary complication. Instead of that, we can use the nextElementSibling
method instead.
// messages.forEach(function (message, index) {
messages.forEach(function (message) {
message.addEventListener("keyup", function(evt) {
// var countfield = document.querySelectorAll(".counter")[index];
var countfield = message.nextElementSibling;
...
The counter class can still be kept on the HTML code for when you want to style it differently.
https://jsfiddle.net/pmw57/dL0oy6pb/5/
If you want to keep sibling
in the code, there is a getNextSibling() function that lets you then use:
var countfield = getNextSibling(message, ".counter");
https://jsfiddle.net/pmw57/dL0oy6pb/6/
But it’s not yet worth the inclusion of that getNextSibling function until your needs require it.
Remove duplicate element reference
The this keyword is duplicating a reference to the message variable, and the code is clearer without the this keyword being there.
// textCounter(this, countfield, 10);
textCounter(message, countfield, 10);
The code is clearer now that we don’t have to mentally juggle what the this keyword is referring to.
https://jsfiddle.net/pmw57/dL0oy6pb/7/
Decouple functions to help remove deep nesting
Another issue to deal with is that there is significant nesting of the code. We can name the functions to help separate what it does, with how it does it.
function addCounterUpdate(message) {
...
}
var messages = document.querySelectorAll(".message");
// messages.forEach(function(message) {
// ...
// });
messages.forEach(addCounterUpdate);
And with the event handler, we can call that function counterKeyupHandler
function counterKeyupHandler(evt) {
var countfield = message.nextElementSibling;
textCounter(message, countfield, 10);
}
// message.addEventListener("keyup", function (evt) {
// ...
// });
message.addEventListener("keyup", counterKeyupHandler);
That helps to reveal another issue. The counterKeyupHandler function receives evt, but uses message in the function. It’s best if the function tries to get information from the function parameters that it’s given.
We can use evt.target
to get the message element instead.
function counterKeyupHandler(evt) {
var message = evt.target;
var countfield = message.nextElementSibling;
textCounter(message, countfield, 10);
}
And now the functions can be easily extracted out, where each function does one simple thing. That helps to make them a lot more reliable, and easily updated.
function counterKeyupHandler(evt) {
var message = evt.target;
var countfield = message.nextElementSibling;
textCounter(message, countfield, 10);
}
function addCounterUpdate(message) {
message.addEventListener("keyup", counterKeyupHandler);
}
var messages = document.querySelectorAll(".message");
messages.forEach(addCounterUpdate);
https://jsfiddle.net/pmw57/dL0oy6pb/9/
And we’re all finished with this update to the code.