Minor inefficiencies in Droppable

[Danny]

These aren't a big deal, but they'll save a few bytes.
In Droppable _init, there's the line
var o = this.options, accept = o.accept;
but o is never used and accept is only used once; everywhere else it's
this.options.accept.

Change line 23 from
this.options.accept = this.options.accept && $.isFunction
(this.options.accept) ? this.options.accept : function(d) {
to
o.accept = $.isFunction(accept) ? accept : function(d) {



($.isFunction() correctly returns false for a false-y argument, so the
"accept &&" guard is unnecessary)

Also, further down, this.options.scope can be replaced by o.scope

And
(this.options.addClasses && this.element.addClass("ui-droppable"));

is a funny idiom. Why not the clearer
if (o.addClasses) this.element.addClass("ui-droppable"));

Danny

[Cloudream]

On Fri, Mar 6, 2009 at 1:45 AM, Danny <d.wa...@prodigy.net> wrote:

These aren't a big deal, but they'll save a few bytes.
In Droppable _init, there's the line
 var o = this.options, accept = o.accept;
but o is never used and accept is only used once; everywhere else it's
this.options.accept.

Change line 23 from
 this.options.accept = this.options.accept && $.isFunction
(this.options.accept) ? this.options.accept : function(d) {
to
 o.accept = $.isFunction(accept) ? accept : function(d) {

Thanks, all this.options change to o in _init 

($.isFunction() correctly returns false for a false-y argument, so the
"accept  &&" guard is unnecessary)

Also, further down, this.options.scope can be replaced by o.scope

And
(this.options.addClasses && this.element.addClass("ui-droppable"));

is a funny idiom. Why not the clearer
if (o.addClasses) this.element.addClass("ui-droppable"));

I don't know, but there're lots of in UI. :P

 

Danny

[Scott González]

Hi Danny,

On Thu, Mar 5, 2009 at 12:45 PM, Danny <d.wa...@prodigy.net> wrote:

Change line 23 from
 this.options.accept = this.options.accept && $.isFunction
(this.options.accept) ? this.options.accept : function(d) {
to
 o.accept = $.isFunction(accept) ? accept : function(d) {

This code actually needs to be rewritten anyway because it violates one of our rules (plugins may never change an option value to be something other than what was explicitly set by the user).  Feel free to file a ticket for this at http://dev.jqueryui.com/newticket (requires registration).  We'll be fixing this for 1.8 in all plugins.

And
(this.options.addClasses && this.element.addClass("ui-droppable"));

is a funny idiom. Why not the clearer
if (o.addClasses) this.element.addClass("ui-droppable"));

We use this to save two bytes because we require all if blocks to have curly braces.  (a && b) is shorter than if (a) { b }

[Danny]

Scott:
Sounds like a good rule, but I assume you still allow the plugin to
change its own
data (which is stored in this.options), as long as it's not a
documented, user-set option.
So
o.acceptFn = $.isFunction(accept) ? accept : function(d) { return
d.is(accept) };
would work, then change all the
this.options.accept.call(...
to
this.options.acceptFn.call(...

I'll file that ticket.
Danny

On Mar 5, 12:19 pm, Scott González <scott.gonza...@gmail.com> wrote:

> This code actually needs to be rewritten anyway because it violates one of
> our rules (plugins may never change an option value to be something other
> than what was explicitly set by the user). Feel free to file a ticket for

> this athttp://dev.jqueryui.com/newticket(requires registration). We'll be

> fixing this for 1.8 in all plugins.
>
> And
>
> > (this.options.addClasses && this.element.addClass("ui-droppable"));
>
> > is a funny idiom. Why not the clearer
> > if (o.addClasses) this.element.addClass("ui-droppable"));
>
> We use this to save two bytes because we require all if blocks to have curly
> braces. (a && b) is shorter than if (a) { b }

I think I remember that discussion from jquery-dev . I'd still vote
for the "if" form.

[Scott González]

On Thu, Mar 5, 2009 at 2:29 PM, Danny <d.wa...@prodigy.net> wrote:

I'll file that ticket.
Danny

Thanks.
 

I think I remember that discussion from jquery-dev . I'd still vote
for the "if" form.

Yeah, it's personal preference.  We don't have a rule about which to use, just that you must include the braces with if statements.

[Jörn Zaefferer]

Nope, any mutable data is stored in "this", not in this.options.
options is public and mutable only by the user, documented or not.
Right Scott?

Jörn

[Scott González]

On Thu, Mar 5, 2009 at 3:10 PM, Jörn Zaefferer <joern.z...@googlemail.com> wrote:

Nope, any mutable data is stored in "this", not in this.options.
options is public and mutable only by the user, documented or not.
Right Scott?

Right, so this.accept could be set to anything by the plugin, but this.options.accept may only be set by the user.

[Danny]

ticket created: http://dev.jqueryui.com/ticket/4282

On Mar 5, 2:53 pm, Scott González <scott.gonza...@gmail.com> wrote:
> On Thu, Mar 5, 2009 at 3:10 PM, Jörn Zaefferer <
>