0
\$\begingroup\$

I'm pretty new to JQuery, maybe a week's worth of experience, but I'm having a lot of fun so far. One of the things I thought I would give a shot is attempting to implement a custom CMS for my current project. And it's going fairly well. So well in fact, that I thought I would start adding some nifty little features and finalizing it. I mean, why not, I'm learning anyways and what better way than to apply it to an actual project that might benefit from it.

One of the features I thought I'd add would be a dynamic edit toolbar. It uses a delegate to perform event functions on a specific div. Specifically, when the #editable section is in focus it will show the toolbar, and on blur it will hide it.

$( '#content' ).on( {
    focus : function() {
        if( $( this ).prop( 'contentEditable' ) ) {
            $( '#toolbar' ).show();
            //thinking there might be a way to reuse $( '#toolbar' ) below
            //within this pseudo scope
        }
    },

    blur : function() {
        $( '#toolbar' ).hide();
    }
}, '#edit' );

Pretty neat. I just found this syntax and have been playing with it for a few hours now. Beats having to manually type out a new delegate each time.

This second one does something pretty similar, but it focuses on a single event where the functionality will vary depending on which element is clicked. For instance, if addImage is clicked I want it to add an image, if bold I want it to bold, etc... It's a little more complex than that, but this gets the idea across. As you can see, the syntax is different (it's all one function and uses a switch), and no amount of tweaking or searching has helped me to discover if a better way actually exists, or if this is the best already.

$( '#toolbar' ).on( 'click', 'a', function() {
    switch( $( this ).parent().prop( 'id' ) ) {
        case 'addImage' :
            addImage();
        break;
        case 'bold' :
            bold();
        break;
        case 'italics' :
            italics();
        break;
        case 'underline' :
            underline();
        break;
    }

    return false;
} );

I do have an alternative, but I find the one above a little more attractive. Mostly just because there is less indentation, though I guess I could fix that...

$( '#toolbar' ).on( {
    click : function() {
        //import above switch statement here
    }
}, 'a' );

Here are my questions:

  • Is there a better way to do this?
  • Am I abusing the selectors/delegates?
  • Any other general observations or improvements would be appreciated as well.

UPDATE

Thank you Flambino. Your first example reminded me of something we call variable-functions in PHP. In case you are unfamiliar with the term, these are variables that serve as a textual representation of a function and can be used to call that function by adding parenthesis. Example:

function test1() { echo 'test1'; }
function test2() { echo 'test2'; }
$test1 = 'test2';
$test2 = 'test1';
$test1();//variable-function calling test2()
$test2();//variable-function calling test1()

They are not well liked in the PHP community due to their lack of legibility and potential security risk. I know JS isn't going to be as concerned with the security bit, seeing as its front-end and completely visible and manipulable client-side, but lack of legibility looks like it might still be a potential problem. Though, because of the object clarifying this purpose, this does not appear to be as big an issue either. Are these not equally disliked in JS?

Here is my implementation using custom events.

var $toolbar = $( '#toolbar' );
$toolbar.on( 'click', 'a', function() {
    var method = $( this ).parent().prop( 'id' );
    $toolbar.trigger( method );
    return false;
} );

$toolbar.on( {
    addImage : function() {
        addImage();
    },
    bold : function() {
        bold();
    },
    //etc...
} );

When implementing custom events I noticed that you are namespacing them with format:. In my implementation you may have noticed that I neglected to add this. This is because namespacing in this manner would prevent me from using an event object. I played around with this and determined that I could use format_ instead, but this just seemed awkward. Is this violating good practice or something? I can see the benefit, but if they are bound to a specific container, why would namespacing be necessary?

I tried combining the two .on() blocks, but because the first uses the "a" classifier it limits those events to the anchor tags, which means I would have to explicitly trigger the events like this:

$( '#toolbar > a' ).trigger( method );

Is there a way around this? I'm still toying with this, but haven't found any promising leads yet. I understand from your suggestion about semantics this won't be an issue here anymore, but I've also been applying this to other areas, and in those this looks like its still an issue.

Finally, I am still looking in to your data-* suggestion. It looks promising, not just here, but in other places as well.

BTW: why are you using .attr()? Wasn't that deprecated as of 1.6 in favor of .prop()? Or is there some reason it should be used here over prop()?

\$\endgroup\$
2
\$\begingroup\$

I'd probably make an object with the various formatting functions, like:

var formatting = {
  addImage: function () { ... },
  bold: function () { ... },
  ...
};

And then have an event handler like:

$( '#toolbar' ).on( 'click', 'a', function() {
  var method = $(this).parent().prop('id');
  if( method in formatting ) {
    formatting[method](); // call the formatting method
  }
  return false;
});

Then you can add/remove formatting methods without having to manually change a switch statement each time.

I'd probably also use a data-* attribute instead of id to store the name of the formatting method to call. It's a bit of extra markup, but it's more clear, I think:

<a href="#" id="boldButton" data-formatting="bold">Bold</a>

and

var method = $(this).parent().attr('data-formatting');

That way, the ID can be whatever without tripping up the actual function of the link. (You can also use .data('formatting'), but I prefer being explicit about it being an attribute, whereas .data doesn't necessarily refer to markup).

Alternatively, you could make some custom events:

var toolbar = $('#toolbar');
toolbar.on( 'click', 'a', function() {
  var method = $(this).parent().attr('data-formatting');
  toolbar.trigger('format:' + method);
  return false;
});

Then you can listen for those events elsewhere

$('#toolbar').on('format:bold', function () {
  // make stuff bold
});

It's a bit of a roundabout way of doing it, but you get the flexibility of being able to add/remove event handlers as you please. (By the way, it might make more sense semantically to trigger the event on the editable element instead of the toolbar).


Update

In response to the updated question:

Your first example reminded me of something we call variable-functions in PHP [...] Are these not equally disliked in JS?

Nope - as you say, the security considerations aren't the same. Besides, where variable-functions in PHP do seem kinda hack'y and going against the grain, JS has always had a different concept of what a "function" is - namely that a function is itself an object and thus a property/variable. In some ways it's like if you were using createFunction() and callUserFuncArray() for everything in PHP and passing functions around (not a great comparison, but those features are PHP's attempt to mimic some of what you find in a language like JS).
The second part of the equation is JS's way of providing access to object properties, namely that you can use either the dot-notation, or the bracket notation. These are all the same:

window.alert("Hello, world");
window["alert"]("Hello, world");
var name = "alert";
window[name]("Hello, world");

// window is the global obj, and thus implicit, so you can also do:
alert("Hello, world");
// but, due to the vagaries of the syntax, you can't do
["alert"]("hello, world");
// because ["alert"] is interpreted as an array literal

When implementing custom events I noticed that you are namespacing them with format:. In my implementation you may have noticed that I neglected to add this. This is because namespacing in this manner would prevent me from using an event object

Actually, no. Because of the bracket notation, properties can in fact have otherwise "illegal" names - and a function is a property too. I.e. you can declare things like:

var obj = {
  "say hello!": function () {...}
};
obj["say bye!"] = function () {...}

And call it with obj["say hello"](). You may notice that this looks like JSON, with its quoted keys, and yup, it does. While JSON can't/won't encode functions, the "key": value syntax is designed to be compatible with JavaScript's object literal notation - in fact, it's the same as JavaScript's object literal notation.

I your case, that means you could do:

var handlers = {
  "format:bold": function () { ... }
};

Which makes it's possible to namespace your events, regardless of how you attach your handlers. Namespacing them is A Good Idea™, if only because it makes the event's meaning more explicit, e.g. format:bold, insert:image, insert:table etc.. Of course, it also makes it clear that it's not a native event.

I tried combining the two .on() blocks, but [that made it more complex]

I'd say don't combine them. Sure, it might make the code more terse, but I see it more as the a#onclick handler is in charge of "translating" the click event to a custom event. Handling those custom events is a separate concern that can be taken care of (or just ignored) elsewhere. Decoupling, basically. Likewise, I'd probably declare the object with the custom event handlers separately, and then attach it with .on afterwards. It adds a bit of flexibility (and readability, imho) to have the object "by itself" instead of inlining it. For instance, you can add and remove methods (handlers) in the object at any point in time, because you'll have a reference to the object (which you wouldn't if it's inlined).

Finally, I am still looking in to your data-* suggestion. It looks promising, not just here, but in other places as well.

It is. It came about because people (myself included) were overloading the class attribute with all kinds of stuff that weren't actually class names, but random keywords used by JS instead. data-* is very welcome addition to HTML.

BTW: why are you using .attr()?

Because old habits die hard :)
But also because .prop() is more general; "retrieve a property" whether it's in the markup as an attribute or not. However, in this case, we're explicitly looking for an attribute, and personally I like using attr in those cases, but either one will work (I see I've been inconsistent about it though, since I use prop("id") elsewhere - apologies for the confusion). You could also use .data("formatting") to get the value actually - there's a good case for using that instead. You're spoiled for choice really :)

\$\endgroup\$
  • \$\begingroup\$ Thank you for this detailed answer. I've learned quite a bit. I decided to go with your second suggestion. I see a few benefits of choosing this over the other. First, it is a bit more coherent (imho); Second, I can apply an event object like I did in my first code sample, making both blocks similar in syntax; And third, I can reuse events with alternate triggers, such as key sequences (Ctrl+B to bold). I've updated my question with the new code and added a few more questions pertaining to this new implementation if you have time. \$\endgroup\$ – mseancole Oct 15 '12 at 16:00
  • \$\begingroup\$ @mseancole Cool. And yes, I had some time (read: the stuff I was supposed to be doing was boring), so I've updated my answer \$\endgroup\$ – Flambino Oct 15 '12 at 21:38
  • \$\begingroup\$ Awesome, wish I could upvote again, but I guess you'll have to settle for accepted instead. Interestingly enough, I tried the 'format:bold' : function() { method first, but it didn't work. As you've probably guessed, it was because I was using single quotes. When it didn't work, I just assumed that it wouldn't and didn't think to try different quotes. Thank you for clearing that up :) \$\endgroup\$ – mseancole Oct 16 '12 at 14:02
  • \$\begingroup\$ @mseancole Happy to help. But, oddly, single quotes shouldn't cause any trouble - JS doesn't treat single quotes different from double quotes (unlike PHP and other languages, JS doesn't have string interpolation, so quoting style makes no difference). Weird. \$\endgroup\$ – Flambino Oct 16 '12 at 14:31
  • \$\begingroup\$ I know, I didn't think it would matter either, which is why I didn't try it again until you said it should work. I just implemented this and tested it again. With double quotes it works fine, but the single quotes seem to be tripping it up. Looking in Firebug I don't see any warnings or errors, so I don't know why its failing. \$\endgroup\$ – mseancole Oct 16 '12 at 14:43

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.