Increasing visible text with on-click "read more"

Question

What can I do better?

$(function () {
 "use strict";
 /*
 * Header text. Vergroot de tekst door op lees meer te klikken
 */
 var heading = $(".header .text h2").height(),
 firstP = $(".header p:first").height(),
 areaSmall = heading + firstP,
 areaBig = $(".header .text").height(),
 box = $(".header .text"),
 readmore = $('.header .text a[title*="meer"]'),
 closeBox = $('.header .text a[title*="Sluit"]');
 $(".header .text").css({ height: areaSmall });
 readmore.click(function () {
 $(".header .text").animate({ height: areaBig }, 1000, "easeInQuart", function () {
 $(".header .text p").css({ visibility: "visible" });
 });
 });
 closeBox.click(function () {
 $(".header .text").animate({ height: areaSmall }, 1000, "easeOutQuart", function () {
 $(".header .text p").css({ visibility: "hidden" });
 $(".header .text p:first").css({ visibility: "visible" });
 });
 });
});

Answer 1

You have lots of calls to $(".header .text"), which you can replace with calls to box:

var areaBig = box.height();

Similarly, you can simplify calls to selectors within box, such as:

// Better than: readmore = $('.header .text a[title*="meer"]')
var readmore = box.find('a[title*="meer"]');
// Better than: $(".header .text").animate(...
box.animate({ height: areaBig }, 1000, "easeInQuart", function () {
 box.find("p").css({ visibility: "visible" });
});

These changes will not only make the code easier to read but also increase execution speed.

• cHao: Just make sure that the definition of box comes first, or at least before any code that intends to use it.

Answer 2

I would:

• call heights heights (or "ht"), as in areaSmall
• name jQuery object to start with $, and use them for elements I reference multiple times
• don't change names of things (header !== heading)
• don't create variables that are only used once
• remove unused variables (box)

Here's one take...

var $header = $('.header'),
 $text = $('.text', $header),
 $p = $('p', $text),
 areaSmallHt = $("h2", $text).height() + $("p:first", $header).height(),
 textHt = $text.height();
$text.css({ height: areaSmallHt });
$('a[title*="meer"]', $text).click(function () {
 $text.animate({ height: textHt }, 1000, "easeInQuart", function () {
 $p.css({ visibility: "visible" });
 });
});
$('a[title*="Sluit"]', $text).click(function () {
 $text.animate({ height: areaSmallHt }, 1000, "easeOutQuart", function () {
 $p.css({ visibility: "hidden" });
 $("p:first", $text).css({ visibility: "visible" });
 });
});

Answer 3

I am not sure of what you want to be better but, for better code legibility and maintainability I would say that you should replace all your string constants with variables:

var headerClassId = ".header";
var textClassId = ".text";
var aTextValueThatMeansSomething = "easeInQuart"; 

Do the same with all other constants:

var aVariableNameThatMeansSomething = 1000;
...
$(headerClassId +" "+textClassId).animate({ height: areaBig },
 aVariableNameThatMeansSomething , aTextValueThatMeansSomething, function ...

• cHao: This really doesn't do much for legibility. headerClassId +" "+textClassId is less readable to me than ".header .text" -- for one thing, because it requires that i know the value and intent of headerClassId and textClassId. (Not to mention, *ClassId is a rather crappy name -- is it a class, or an id, or both? "Either" is a bad answer.)