Joel Birch schrieb:
> On 28/02/2007, at 8:17 AM, Joel Birch wrote:
>> On 28/02/2007, at 12:29 AM, Klaus Hartl wrote:
>>> Joel Birch schrieb:
>>>> Hi jQuerivites,
>>>>
>>>> I'd like to offer up my humble plugin (humble is the key word here)
>>>> to get a sense of if something this basic is of any worth to the
>>>> community.
>>> I think its useful! Here's some work for you ;-)
>>>
>>> * Instead of doing two appends you could do:
>>>
>>> $('<div class="'+o.divClass+'"></div>').append([o.head,
>>> $list]).insertBefore(this.eq(0));
>>>
>>> * Instead of using this.innerHtml you should use $(this).text() to
>>> avoid
>>> nested links for example. Imagine article headlines that are links as
>>> well but should also serve as content navigation hook.
>>>
>>> Another useful option would be to specify where to append the created
>>> navigation. I could imagine appending it to the body and apply a
>>> fixed
>>> positioning, so that it is always in the viewport...
>>>
>
> Hi Klaus, I've done my homework :D
>
> Firstly, I reduced those two appends to one. I found that .append
> (o.head,$list) works - you must have made a typo with the square
> brackets but that was easy to figure out anyway.
>
> Secondly, inspired by your prompt to reduce function calls, I rewrote
> how the list was compiled by creating a string of the whole list
> before appending that to the list. This made the whole plugin run
> more than twice as fast as before (~30ms for 19 list items instead of
> ~80ms).
>
> Finally, I made it so you can target where the list is inserted as
> you suggested. There is a new option called "insertMethod" that takes
> one of "insertBefore", "insertAfter", "append" or "prepend", and
> another new option called "insertTarget" which takes a jQuery object.
>
> So now if you want to append the list to the body you can do:
> $("h2").contentMenu({"insertMethod":"append","insertTarget":$("body")});
>
> The default is to .insertBefore(this.eq(0));
>
> Obviously the code has grown larger, mainly due to the switch used to
> parse the insertMethod option. Can you think of a better way of doing
> that whilst avoiding eval()?
>
> I have learnt a lot doing this so thanks for humouring me Klaus - I
> know you are very busy at the moment. Oh, here's the new code:
>
> (function($) {
> $.fn.contentMenu = function(o){
> o = $.extend({ "head" : "<h3>Some handy links to help you navigate
> this page:</h3>",
> "divClass" : "contentMenu",
> "aClass" : "inPage",
> "insertMethod" : "insertBefore",
> "insertTarget" : this.eq(0) }, o || {});
> $.cmCount = $.cmCount+1 || 0;
> var $list = $("<ul>");
> var lastInd = this.length-1;
> var lis = '';
> var menu = $('<div class="'+o.divClass+'"></div>').append(o.head,
> $list);
> switch (o.insertMethod){
> case "insertBefore":
> menu.insertBefore(o.insertTarget);
> break;
> case "insertAfter":
> menu.insertAfter(o.insertTarget);
> break;
> case "append":
> o.insertTarget.append(menu);
> break;
> case "prepend":
> o.insertTarget.prepend(menu);
> break;
> default :
> menu.insertBefore(o.insertTarget);
> }
> return this.each(function(i){
> this.id = this.id || "menu"+$.cmCount+"-el"+i;
> lis += '<li><a href="#'+this.id+'" class="'+o.aClass+'">Skip to
>
> <em>'+$(this).text()+'</em></a></li>';
> if (i==lastInd){ $list.append(lis); }
> });
> };
> )(jQuery);
>
> Cheers
> Joel.
I spotted a little optimization: in the default branch
"menu.insertBefore(o.insertTarget);" is used. Thus you can remove the
case for "insertBefore", that one will just use the default...
I like the switch statement a lot and think it is very readable, but
some people say switch smells (Hey Jörn ;-)) and that it is a sign that
something could be refactored. Just so you know! I'm not sure about
that, the switch statement is considered very fast (google "duffs
device") and for predictable cases its fine for me.
-- Klaus
_______________________________________________
jQuery mailing list
discuss@jquery.com
http://jquery.com/discuss/
