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(this).attr("title","Show Playlist");         
     $j(this).children("div").html('Show Playlist');
 }, function() {
     $j(this).attr("title","Hide Playlist");
     $j(this).children("div").html('Hide Playlist');

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);

$j('#hideShowBtn').toggle( toggle(true), toggle(false) );
  • \$\begingroup\$ I actually really like that solution, thanks a lot! \$\endgroup\$
    – Lelly
    Nov 23 '12 at 13:33
  • \$\begingroup\$ @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 \$\endgroup\$
    – 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 class="toggleable" style="display: none;">
    <span>&#9650;</span> Hide playlist

And then somewhere in your JavaScript you call the .toggle() function from the jQuery UI:


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

  • \$\begingroup\$ 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. \$\endgroup\$
    – Quango
    Nov 23 '12 at 9:03
  • 1
    \$\begingroup\$ 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. \$\endgroup\$
    – 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);
    $this.attr("title", opts.title);

  method: 'slideUp',
  title: 'Show Playlist',
  spanContent: '&#9660;'
}), getToggleHandler({
  method: 'slideDown',
  title: 'Hide Playlist',
  spanContent: '&#9650;'

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.