An utterly pointless jQuery program

This is the first jQuery program I've written, and it doesn't do much, but I'd appreciate the feedback. Just click on the different colored buttons to do things.

Codepen



var $greenElement = $(
' \

Hello \
'
);

var $redElement = $(
' \

Goodbye \
'
);

/ Main JQuery loop /
$(document).ready(function() {

/ Add a red item and fade to the right value/
$('.add-item-red').mouseenter(function() {
$(this).fadeTo('fast', 0.4);
$(this).click(function() {
$('.output-start').after($redElement);
});
});

/ Check if the mouse is no longer on red button /
$('.add-item-red').mouseout(function() {
$(this).fadeTo('fast', 1);
});

/ Add a green item and fade to the right amount /
$('.add-item-green').mouseenter(function() {
$(this).fadeTo('fast', 0.4);
$(this).click(function() {
$('.output-start').after($greenElement);
});
});

/ Check if mouse is no longer on green button /
$('.add-item-green').mouseout(function() {
$(this).fadeTo('fast', 1);
});

/ Reset all the items /
$('.reset-items').mouseenter(function() {
$(this).fadeTo('fast', 0.4);
$('.reset-items').click(function() {
$('.green-bar').remove();
$('.red-bar').remove();
});
});

/ Check if mouse is no longer on white button /
$('.reset-items').mouseout(function() {
$(this).fadeTo('fast', 1);
});
});
body {
background-color: black;
}

.button-bar div{
display: table;
display: table-cell;
}

.add-item-red {
width: 50px;
height: 50px;
background-color: red;
border-color: black;
border-style: solid;
border-width: 3.5px;
}

.add-item-green {
width: 50px;
height: 50px;
background-color: green;
border-color: black;
border-style: solid;
border-width: 3.5px;
}

.reset-items {
width: 50px;
height: 50px;
background-color: white;
border-color: black;
border-style: solid;
border-width: 3.5px;
}

.output-start {
width: 50px;
height: 15px;
}
`

Ok, so there's two different things here:

• Hover effects (fade in/out)

• Actions

Now, #1 is the same for all three elements, and thus shouldn't need to be repeated in code. But #2 is expressly different for each button, which means it probably shouldn't mixed with the hover effects.

Aside: Speaking of duplication, your CSS is full of it too. You should look into that. You should also avoid naming things for how they look. E.g. add-item-red is fine until that item is no longer red; usually it'll have some purpose other than being red. Its color is a presentation detail. Name stuff for what it does, not how it looks.

Anyway, these days (read: modern browsers), I'd probably just use CSS transitions to handle the hover effect; no javascript necessary. For instance:



div {
background: green;
padding: 0.3em;
display: inline-block;
transition-duration: 0.5s;
}

div:hover {
opacity: 0.4;
}
Hover



But since this is about jQuery, here's how I'd handle it with a common class name for all the buttons (which, incidentally, I'd use ` tags for), and jQuery's appropriately named .hover() event handler.



$(".fade").hover(function () {
$(this).fadeTo('fast', 0.4);
}, function () {
$(this).fadeTo('fast', 1.0);
});
body { background: black; }

button {
border: none;
padding: 1em;
}

#helloButton { background: green; }
#goodbyeButton { background: red; }
#clearButton { background: white; }


One
Two
Three



Now, you're using mouseenter combined with mouseout. But that's not really "symmetrical". The opposite of mouseenter is mouseleave (and the opposite of mouseout is mouseover). The enter/leave events are originally Internet Explorer-only events which jQuery has replicated, and they behave differently than the standard over/out events with regard to nested elements. jQuery's docs have a discussion of this and a demo to illustrate. It happens to work in your case, but it's a bit like saying that the opposite of multiplying is subtracting; it just happens to work out for certain numbers, but that doesn't make it right.

Next, there's the click actions. You're adding those inside the mouseenter event. This is a big no-no. It means that every time you mouse over a button, a new click event handler gets added! You just don't notice it because you're inserting and re-inserting the same element. But if you mouse over the red button 3 times before clicking it, the effect is that you're clicking it 3 times because now you've got 3 separate but identical event handlers attached.

Yet another reason to keep the actions separate from the hover effects.

You can either attach the event handlers to an element by its ID (as below), or you can use some other means of linking an element to an action.



$(".fade").hover(function () {
$(this).fadeTo('fast', 0.4);
}, function () {
$(this).fadeTo('fast', 1.0);
});

var hello = $("
").text("Hello").css({background: "green"});
var goodbye = $("
").text("Goodbye").css({background: "red"});

$("#helloButton").on("click", function () {
$("#output").prepend(hello);
});

$("#goodbyeButton").on("click", function () {
$("#output").prepend(goodbye);
});

$("#clearButton").on("click", function () {
$("#output").empty();
});
body { background: black; }

button {
border: none;
padding: 1em;
}

#helloButton { background: green; }
#goodbyeButton { background: red; }
#clearButton { background: white; }


One
Two
Three





You'll notice a few things here:

-
I'm prepending the elements to the output div, rather than inserting them next to it. This makes more sense to me since now, all the output is, well, in the output. Not beside it. And it means I can just call empty to clean up.

-
I'm constructing the elements using jQuery rather than with strings of HTML. I generally don't like a ton of hardcoded HTML in my JS. I also don't need IDs for them, because, again, I just need to call empty.

I might go a step further and add the elements in the HTML itself, and simply hide them until needed, which would be even simpler in a lot of ways, and keep the JS free of hardcoded elements of any kind.

In the end, I'd probably replicate your code with something like this:



$("#helloButton").on("click", function () {
$("#hello").show();
});

$("#goodbyeButton").on("click", function () {
$("#goodbye").show();
});

$("#clearButton").on("click", function () {
$("#output").children().hide();
});
body { background: black; }

button {
border: none;
padding: 1em;
display: inline-block;
}

.fade { transition-duration: 0.5s; }
.fade:hover { opacity: 0.4; }

#helloButton { background: green; }
#goodbyeButton { background: red; }
#clearButton { background: white; }

#hello { background: green; }
#goodbye { background: red; }

#output p { display: none; }

One
Two
Three

Hello
Goodbye
`



It doesn't make the paragraphs switch places, but I don't think that's the most salient feature eith

StackExchange Code Review Q#84113, answer score: 7