# Revealing and hiding a playlist

Is there a better way of writing this? I feel like there's a lot of lines for such a small thing. Can it be optimized?

$j('#hideShowBtn').toggle(function() {$j('.playlist-bar-tray').slideUp();
$j(this).attr("title","Show Playlist");$j(this).children("div").html('Show Playlist');
$j(this).children("span").html('&#9660;'); }, function() {$j('.playlist-bar-tray').slideDown();
$j(this).attr("title","Hide Playlist");$j(this).children("div").html('Hide Playlist');
$j(this).children("span").html('&#9650;'); });  - ## 3 Answers Well, it can be written differently with less repetition, but it's readable as it is. There are 4 things that need to happen when the toggling occurs; your code does exactly that. It's the simplest way to write it, really. My only real suggestion would be to cache $j(this) in a variable to avoid recreating it.

Regardless, here's a funky rewrite, just for the sake of it

function toggle(hiding) {
var title   = (hiding ? "Show Playlist" : "Hide Playlist"),
symbol  = (hiding ? "&#9960"        : "&#9650"),
method  = (hiding ? "slideUp"       : "slideDown");

return function () {
var element = $j(this); element.attr("title", title).children("div").html(title); element.children("span").html(symbol);$j(".playlist-bar-tray")[method]();
};
}

$j('#hideShowBtn').toggle( toggle(true), toggle(false) );  - I actually really like that solution, thanks a lot! – Lelly Nov 23 '12 at 13:33 @Lelly Glad you found it useful. But do check out ZeroOne's answer too, and read its comments. There's a lot to gain by keeping the content in the markup, and only the behavior in the JS – Flambino Nov 23 '12 at 17:35 How about "cheating" a bit by creating two copies of those elements whose values should change and then just alternate between which element to show. Something like this: <div class="toggleable"> <span>&#9660;</span> Show playlist </div> <div class="toggleable" style="display: none;"> <span>&#9650;</span> Hide playlist </div>  And then somewhere in your JavaScript you call the .toggle() function from the jQuery UI: $(".toggleable").toggle();


Now it would look as though the texts changed, while actually you are hiding one element and displaying another.

-
I think this is the neater approach. In the original code there is a lot of html/content embedded in the code, so anyone reading the .html would be unable to see what is going on. This approach means the javascript is just the action. – Quango Nov 23 '12 at 9:03
I agree with @Quango - keep content and behavior separate. Other alternatives would be to use a class and some CSS so show/hide elements (and simply use .toggleClass()), or store the hide/show strings in data-* attributes so they're in the HTML and not the JS. – Flambino Nov 23 '12 at 9:13

Another option (though I have to say that I like @ZeroOne's solution in this case):

var getToggleHandler = function (opts) {
return function () {
var $this =$j(this);
$j('.playlist-bar-tray')[opts.method]();$this.attr("title", opts.title);
$this.children("div").html(opts.title);$this.children("span").html(opts.spanContent);
};
}

\$j('#hideShowBtn').toggle(getToggleHandler({
method: 'slideUp',
title: 'Show Playlist',
spanContent: '&#9660;'
}), getToggleHandler({
method: 'slideDown',
title: 'Hide Playlist',
spanContent: '&#9650;'
}));

-