I'm working on building a website to help people practice English. In this practice, the answer in the blank needs to be validated on hitting enter.
I'd like to know if there is a simpler/better way to write it since I need to use it for a lot of pages.
HTML:
<p>There’s an energy here. At least, I think there is. Is it just that I know this city to be so? That there can be skiing a half hour away on the north shore mountains <input class="fillBlank" type="text" id="one" placeholder="your answer"> there is tennis in the city?</p>
<p>But then, <input class="fillBlank" type="text" id="two" placeholder="your answer"> my walk I took this photo below.</p>
<p>The city is also lush which means that it rains on <input class="fillBlank" type="text" id="three" placeholder="your answer">. Okay, maybe more than occasionally at this time of year.</p>
CSS:
.fillBlank {
width: 100px;
border-radius: 0;
border: none;
background-color: #eff9f7;
padding: 3px;
text-align: center;
}
JS:
$("#vancouver").ready(function() {
$(".fillBlank").val("");
});
// vancouver quiz answer one
var one = document.getElementById("one");
one.addEventListener("keydown", function (e) {
if (e.keyCode === 13) {
valone(e);
}
});
function valone(e) {
var one = document.getElementById("one").value;
if (one === "while") {
document.getElementById("one").style.color = "green";
}
else {
document.getElementById("one").style.color = "red";
}
}
// vancouver quiz answer two
var two = document.getElementById("two");
two.addEventListener("keydown", function (e) {
if (e.keyCode === 13) {
valtwo(e);
}
});
function valtwo(e) {
var two = document.getElementById("two").value;
if (two === "on") {
document.getElementById("two").style.color = "green";
}
else {
document.getElementById("two").style.color = "red";
}
}
// vancouver quiz answer three
var three = document.getElementById("three");
three.addEventListener("keydown", function (e) {
if (e.keyCode === 13) {
valthree(e);
}
});
function valthree(e) {
var three = document.getElementById("three").value;
if (three === "occasion") {
document.getElementById("three").style.color = "green";
}
else {
document.getElementById("three").style.color = "red";
}
}
1 Answer 1
I can be rather quick with the CSS part, use indentation:
.example{
oneTab: indented;
}
Round one
Then, you Javascript. Again, indentation. After that, I've removed several blank lines which had no purpose
Result:
$("#vancouver").ready(function() {
$(".fillBlank").val("");
});
// vancouver quiz answer one
var one = document.getElementById("one");
one.addEventListener("keydown", function (e) {
if (e.keyCode === 13) {
valone(e);
}
});
function valone(e) {
var one = document.getElementById("one").value;
if (one === "while") {
document.getElementById("one").style.color = "green";
}
else {
document.getElementById("one").style.color = "red";
}
}
// vancouver quiz answer two
var two = document.getElementById("two");
two.addEventListener("keydown", function (e) {
if (e.keyCode === 13) {
valtwo(e);
}
});
/* repeat code for every question */
Round Two
Make use of the short if/else. Example below:
function valone(e) {
var one = document.getElementById("one").value;
// Statement ? true : false
document.getElementById("one").style.color = one==="while" ? "green" : "red";
}
//---------------------------------------------------------------------
// And now save the 'one' element so we dont select it twice
function valone(e) {
var one = document.getElementById("one");
// Statement ? true : false
one.style.color = one.value==="while" ? "green" : "red";
}
Round Three
You repeat yourself for every question. This means that when you add one question, you have to update all of that. What if you add 15 questions? Imagine the huge amount of code that that would take!
So, time to fix that. All three of your functions do the same. They check an element for a specific value. They all get triggered on a keydown. Lets try to fix that to a single function:
// First select the elements we want to check the values of, you've allready classed those [fillBlank]:
$('.fillBlank').on('keydown', function(e){
// At this point we have all the inputs on all keydown event, we only want enter:
if (e.keyCode === 13) {
// Now we have all the elements but only execute this on enter.
}
});
Now comes the fun part. We want to be able to check ALL the inputs in one go, but give every input a UNIQUE value to check. For this we can use objects:
var correctAnswers = {
one : 'while',
two : 'on',
three : 'occasion'
}
$('.fillBlank').on('keydown', function(e){ /* ... */ });
Neat little list like that right? Easy to expand aswell, just add one line et voila. We're now gonna add this to our code:
if (e.keyCode === 13) {
// Now we have all the elements but only excecute this on enter.
// Start checking the value for the input where the user pressed enter:
// We can access the value via this.id because we named the input 'one', and there is a key 'one' in correctAnswers
this.style.color = this.value=== correctAnswers[this.id] ? "green" : "red";
}
Well, thats it really. Now you can add plenty more items without building a million functions. The code below does the same thing as yours, but without hardcoding.
Result:
$("#vancouver").ready(function() {
$(".fillBlank").val("");
});
// The correct answers:
var correctAnswers = {
one : 'while',
two : 'on',
three : 'occasion'
}
// First select the elements we want to check the values of
$('.fillBlank').on('keydown', function(e){
// At this point we have all the inputs on all keydown event, we only want enter:
if (e.keyCode === 13) {
this.style.color = this.value=== correctAnswers[this.id] ? "green" : "red";
}
});
There are some extra things you can do to improve your code.
- What if I type
While
. If you want to ignore uppercase, you can lowercase thethis.value
to always check the answers lowercased - You might want to add a check if
this.id
is an existing value of correctAnswers (hint: it's called.indexOf()
) - You might want to add/remove a class. CSS is for styling, not JS. This way, when you want to change the style of something correct/incorrect you do it in the stylesheet.
- You might want to add the color names (or if you've updated the classnames) to a variable, e.g.:
var outcomes = {right:'green', wrong: 'red'}; alert(outcomes.right);
-
\$\begingroup\$ One think to note here. Its better if you toggle a class such as
.text-error
or.text-success
instead of setting the color as its more flexible with the styling. Just don't forget to set some rules in css \$\endgroup\$Fanis Despoudis– Fanis Despoudis2017年02月21日 15:52:53 +00:00Commented Feb 21, 2017 at 15:52 -
\$\begingroup\$ Haha I was just added that as bonus excersise for the OP \$\endgroup\$Martijn– Martijn2017年02月21日 15:53:34 +00:00Commented Feb 21, 2017 at 15:53
-
\$\begingroup\$ Oh my I'm embarrassed lol \$\endgroup\$Fanis Despoudis– Fanis Despoudis2017年02月21日 15:56:07 +00:00Commented Feb 21, 2017 at 15:56
on
on all the elements. Demo. Will suggest to usekeyup
andblur
, check demo \$\endgroup\$