[jquery-ui commit] r1542 - trunk/ui

[codesite...@google.com]

Author: paul.bakaus
Date: Wed Jan 7 09:43:15 2009
New Revision: 1542

Modified:
trunk/ui/ui.draggable.js
trunk/ui/ui.droppable.js
trunk/ui/ui.sortable.js

Log:
draggable,droppable,sortable: properly implemented cssNamespace option
across plugins (selectable & resizable to go, so those options should stay
undocumented until all plugins are converted. Fixes #3707)

Modified: trunk/ui/ui.draggable.js
==============================================================================
--- trunk/ui/ui.draggable.js (original)
+++ trunk/ui/ui.draggable.js Wed Jan 7 09:43:15 2009
@@ -20,7 +20,7 @@
this.element[0].style.position = 'relative';

(this.options.cssNamespace &&
this.element.addClass(this.options.cssNamespace+"-draggable"));
- (this.options.disabled &&
this.element.addClass('ui-draggable-disabled'));
+ (this.options.disabled &&
this.element.addClass(this.options.cssNamespace+'-draggable-disabled'));

this._mouseInit();

@@ -28,7 +28,7 @@

destroy: function() {
if(!this.element.data('draggable')) return;
-
this.element.removeData("draggable").unbind(".draggable").removeClass('ui-draggable
ui-draggable-dragging ui-draggable-disabled');
+
this.element.removeData("draggable").unbind(".draggable").removeClass(this.options.cssNamespace+'-draggable '+this.options.cssNamespace+'-draggable-dragging '+this.options.cssNamespace+'-draggable-disabled');
this._mouseDestroy();
},

@@ -36,7 +36,7 @@

var o = this.options;

- if (this.helper || o.disabled ||
$(event.target).is('.ui-resizable-handle'))
+ if (this.helper || o.disabled ||
$(event.target).is('.'+this.options.cssNamespace+'-resizable-handle'))
return false;

//Quit if we're not on a valid handle
@@ -111,7 +111,7 @@
if ($.ui.ddmanager && !o.dropBehaviour)
$.ui.ddmanager.prepareOffsets(this, event);

- this.helper.addClass("ui-draggable-dragging");
+ this.helper.addClass(o.cssNamespace+"-draggable-dragging");
this._mouseDrag(event, true); //Execute the drag once - this causes the
helper not to be visible before getting its correct position
return true;
},
@@ -328,7 +328,7 @@
},

_clear: function() {
- this.helper.removeClass("ui-draggable-dragging");
+ this.helper.removeClass(this.options.cssNamespace+"-draggable-dragging");
if(this.helper[0] != this.element[0] && !this.cancelHelperRemoval)
this.helper.remove();
//if($.ui.ddmanager) $.ui.ddmanager.current = null;
this.helper = null;

Modified: trunk/ui/ui.droppable.js
==============================================================================
--- trunk/ui/ui.droppable.js (original)
+++ trunk/ui/ui.droppable.js Wed Jan 7 09:43:15 2009
@@ -42,7 +42,7 @@
drop.splice(i, 1);

this.element
- .removeClass("ui-droppable-disabled")
+
.removeClass(this.options.cssNamespace+"-droppable "+this.options.cssNamespace+"-droppable-disabled")
.removeData("droppable")
.unbind(".droppable");
},
@@ -105,7 +105,7 @@
if (!draggable || (draggable.currentItem || draggable.element)[0] ==
this.element[0]) return false; // Bail if draggable and droppable are same
element

var childrenIntersection = false;
-
this.element.find(":data(droppable)").not(".ui-draggable-dragging").each(function()
{
+
this.element.find(":data(droppable)").not("."+draggable.options.cssNamespace+"-draggable-dragging").each(function()
{
var inst = $.data(this, 'droppable');
if(inst.options.greedy && $.ui.intersect(draggable, $.extend(inst, {
offset: inst.element.offset() }), inst.options.tolerance)) {
childrenIntersection = true; return false;

Modified: trunk/ui/ui.sortable.js
==============================================================================
--- trunk/ui/ui.sortable.js (original)
+++ trunk/ui/ui.sortable.js Wed Jan 7 09:43:15 2009
@@ -17,7 +17,7 @@

var o = this.options;
this.containerCache = {};
- this.element.addClass("ui-sortable");
+ (this.options.cssNamespace &&
this.element.addClass(this.options.cssNamespace+"-sortable"));

//Get the items
this.refresh();
@@ -35,7 +35,7 @@

destroy: function() {
this.element
- .removeClass("ui-sortable ui-sortable-disabled")
+
.removeClass(this.options.cssNamespace+"-sortable "+this.options.cssNamespace+"-sortable-disabled")
.removeData("sortable")
.unbind(".sortable");
this._mouseDestroy();
@@ -169,7 +169,7 @@

this.dragging = true;

- this.helper.addClass('ui-sortable-helper');
+ this.helper.addClass(o.cssNamespace+'-sortable-helper');
this._mouseDrag(event); //Execute the drag once - this causes the helper
not to be visible before getting its correct position
return true;

@@ -270,7 +270,7 @@
this._mouseUp();

if(this.options.helper == "original")
-
this.currentItem.css(this._storedCSS).removeClass("ui-sortable-helper");
+
this.currentItem.css(this._storedCSS).removeClass(this.options.cssNamespace+"-sortable-helper");
else
this.currentItem.show();

@@ -422,13 +422,13 @@
for (var j = cur.length - 1; j >= 0; j--){
var inst = $.data(cur[j], 'sortable');
if(inst && inst != this && !inst.options.disabled) {
- queries.push([$.isFunction(inst.options.items) ?
inst.options.items.call(inst.element) : $(inst.options.items,
inst.element).not(".ui-sortable-helper"), inst]);
+ queries.push([$.isFunction(inst.options.items) ?
inst.options.items.call(inst.element) : $(inst.options.items,
inst.element).not("."+inst.options.cssNamespace+"-sortable-helper"), inst]);
}
};
};
}

- queries.push([$.isFunction(this.options.items) ?
this.options.items.call(this.element, null, { options: this.options, item:
this.currentItem }) : $(this.options.items,
this.element).not(".ui-sortable-helper"), this]);
+ queries.push([$.isFunction(this.options.items) ?
this.options.items.call(this.element, null, { options: this.options, item:
this.currentItem }) : $(this.options.items,
this.element).not("."+this.options.cssNamespace+"-sortable-helper"), this]);

for (var i = queries.length - 1; i >= 0; i--){
queries[i][0].each(function() {
@@ -552,8 +552,8 @@
element: function() {

var el = $(document.createElement(self.currentItem[0].nodeName))
- .addClass(className || self.currentItem[0].className+"
ui-sortable-placeholder")
- .removeClass('ui-sortable-helper')[0];
+ .addClass(className ||
self.currentItem[0].className+" "+self.options.cssNamespace+"-sortable-placeholder")
+ .removeClass(self.options.cssNamespace+'-sortable-helper')[0];

if(!className) {
el.style.visibility = "hidden";
@@ -823,12 +823,12 @@
for(var i in this._storedCSS) {
if(this._storedCSS[i] == 'auto' || this._storedCSS[i] == 'static')
this._storedCSS[i] = '';
}
- this.currentItem.css(this._storedCSS).removeClass("ui-sortable-helper");
+
this.currentItem.css(this._storedCSS).removeClass(this.options.cssNamespace+"-sortable-helper");
} else {
this.currentItem.show();
}

- if(this.domPosition.prev !=
this.currentItem.prev().not(".ui-sortable-helper")[0] ||
this.domPosition.parent != this.currentItem.parent()[0])
this._propagate("update", event, null, noPropagation); //Trigger update
callback if the DOM position has changed
+ if(this.domPosition.prev !=
this.currentItem.prev().not("."+this.options.cssNamespace+"-sortable-helper")[0]
|| this.domPosition.parent != this.currentItem.parent()[0])
this._propagate("update", event, null, noPropagation); //Trigger update
callback if the DOM position has changed
if(!$.ui.contains(this.element[0], this.currentItem[0])) { //Node was
moved out of the current element
this._propagate("remove", event, null, noPropagation);
for (var i = this.containers.length - 1; i >= 0; i--){
@@ -896,6 +896,7 @@
accurateIntersection: true,
appendTo: "parent",
cancel: ":input,option",
+ cssNamespace: 'ui',
delay: 0,
distance: 1,
dropOnEmpty: true,

[Scott González]

Why are we supporting this option?

[Paul Bakaus]

a) because it's pretty useful for individual projects to support other css namespaces
b) because adding a class, especially in draggables/droppables is a huge performance factor on initialization, as strange as it sounds.

--
Paul Bakaus
UI Architect
--
http://paulbakaus.com
http://www.linkedin.com/in/paulbakaus

[Richard D. Worth]

Seems to me there's a difference between being able to change the 'ui-' namespace and being able to tell draggables not to add a class (for performance reasons). In order to support an option to customize the 'ui-' namespace, it seems we would have to do so for all plugins (I don't think we should do it half-way). That would require a lot of changes and a lot of testing. And it would mean that the user would have to create their own css framework+theme from scratch, no?

- Richard

[Paul Bakaus]

On Thu, Jan 8, 2009 at 10:56 AM, Richard D. Worth <rdw...@gmail.com> wrote:

Seems to me there's a difference between being able to change the 'ui-' namespace and being able to tell draggables not to add a class (for performance reasons). In order to support an option to customize the 'ui-' namespace, it seems we would have to do so for all plugins (I don't think we should do it half-way). That would require a lot of changes and a lot of testing. And it would mean that the user would have to create their own css framework+theme from scratch, no?

You're correct on that. This is why I want it to stay undocumented at this point, because we're not ready. I do think however that for true customization purposes, it would be a nice feature.
 

[Scott González]

-1 for full customization.

$(el).draggable().addClass('my-class');

[Paul Bakaus]

mh, true. For performance reasons, we need a similar option though, could then be something like 'noClasses'. Do you have a better idea for a name?

[Scott González]

addClasses, applyClasses, useClasses?

[Eduardo Lundgren]

We have to support change the name of the css namespace or via noClasses to not add classes combined with a addClasses or via cssNamespace (that makes more sense for me).

All components should support this, and themeroller too.

This is useless: $(el).draggable().addClass('my-class')

--
Eduardo Lundgren
Software Engineer
Liferay, Inc.
Enterprise. Open Source. For Life.

[Richard D. Worth]

On Sat, Jan 17, 2009 at 1:44 AM, Eduardo Lundgren <eduardo...@gmail.com> wrote:

We have to support change the name of the css namespace or via noClasses to not add classes combined with a addClasses or via cssNamespace (that makes more sense for me).

Paul and Scott already agreed above that we're not going to support changing the css namespace.

So the only question left (as I understand it) is how to allow the user in this specific case, to specify they don't want classes added to each individual element (of which there may be many) for performance reasons. So Scott suggested three option names

- addClasses
- applyClasses
- useClasses

I assume we would chose one of these, and it would default to true?

All components should support this

All components don't have the performance need that it seems draggable has, to not add classes to each individual draggable element, where they may be hundreds or thousands.

and themeroller too.

ThemeRoller's role is pretty limited in this regard. Its reason for existing is to modify the visual style of jQuery UI CSS Framework classes (ui-widget, ui-state-default, ui-icon, etc.). This is a totally separate set of classes from ui-draggable, ui-draggable-disabled, etc., which we generally call plugin specific classes. Draggable adds no framework classes, and ThemeRoller does not modify styles of plugin specific classes.

This is useless: $(el).draggable().addClass('my-class')

This looks quite useful to me. The plugin itself adds the plugin classes (in the 'ui-' css namespace) unless the user has requested they not be added for performance reasons. If a user wants diffferent additional classes to select or style these elements, that's up to them. They know what these elements are, since they have selected them in order to call .draggable(). No need for the plugin to do what jQuery can do quite simply.

- Richard

[Richard D. Worth]

+1 addClasses.

- Richard

[Paul Bakaus]

On Sat, Jan 17, 2009 at 11:45 AM, Richard D. Worth <rdw...@gmail.com> wrote:

On Sat, Jan 17, 2009 at 1:44 AM, Eduardo Lundgren <eduardo...@gmail.com> wrote:

We have to support change the name of the css namespace or via noClasses to not add classes combined with a addClasses or via cssNamespace (that makes more sense for me).

Paul and Scott already agreed above that we're not going to support changing the css namespace.

So the only question left (as I understand it) is how to allow the user in this specific case, to specify they don't want classes added to each individual element (of which there may be many) for performance reasons. So Scott suggested three option names

- addClasses
- applyClasses
- useClasses

I assume we would chose one of these, and it would default to true?

All components should support this

All components don't have the performance need that it seems draggable has, to not add classes to each individual draggable element, where they may be hundreds or thousands.

Be careful about that Richard - you have to develop a feeling for real world enterprise cases (same goes for Scott). I could agree that probably draggable & droppable are the most often individually initialized components. However, you never know what users build. We should always optimize the initialization first. We will want to make sure that all components scale, this should be top priority.
 

and themeroller too.

ThemeRoller's role is pretty limited in this regard. Its reason for existing is to modify the visual style of jQuery UI CSS Framework classes (ui-widget, ui-state-default, ui-icon, etc.). This is a totally separate set of classes from ui-draggable, ui-draggable-disabled, etc., which we generally call plugin specific classes. Draggable adds no framework classes, and ThemeRoller does not modify styles of plugin specific classes.

This is useless: $(el).draggable().addClass('my-class')

This looks quite useful to me. The plugin itself adds the plugin classes (in the 'ui-' css namespace) unless the user has requested they not be added for performance reasons. If a user wants diffferent additional classes to select or style these elements, that's up to them. They know what these elements are, since they have selected them in order to call .draggable(). No need for the plugin to do what jQuery can do quite simply.

I agree.
 

[Richard D. Worth]

On Sat, Jan 17, 2009 at 8:04 AM, Paul Bakaus <paul....@googlemail.com> wrote:

On Sat, Jan 17, 2009 at 11:45 AM, Richard D. Worth <rdw...@gmail.com> wrote:

On Sat, Jan 17, 2009 at 1:44 AM, Eduardo Lundgren <eduardo...@gmail.com> wrote:

We have to support change the name of the css namespace or via noClasses to not add classes combined with a addClasses or via cssNamespace (that makes more sense for me).

Paul and Scott already agreed above that we're not going to support changing the css namespace.

So the only question left (as I understand it) is how to allow the user in this specific case, to specify they don't want classes added to each individual element (of which there may be many) for performance reasons. So Scott suggested three option names

- addClasses
- applyClasses
- useClasses

I assume we would chose one of these, and it would default to true?

All components should support this

All components don't have the performance need that it seems draggable has, to not add classes to each individual draggable element, where they may be hundreds or thousands.

Be careful about that Richard - you have to develop a feeling for real world enterprise cases (same goes for Scott). I could agree that probably draggable & droppable are the most often individually initialized components. However, you never know what users build. We should always optimize the initialization first. We will want to make sure that all components scale, this should be top priority.

When we're talking about all components, we're talking about every jQuery UI plugin (interactions and widgets). My point was even if it applies to draggables and droppables, it's not going to apply to dialog or tabs. So I don't agree this needs consideration as an option for all components.

- Richard

[Eduardo Lundgren]

Richard,

What happen if a client using jQuery UI need to change the css namespace for some reason? If we don't have this option they will have to find and replace the "ui-" for "somethingelse-".

Using an option as cssNamespace, we are able to:

cssNamespace: false - increase the performance
cssNamespace: 'somethingelse-' - change the namespace
cssNamespace: undefined - use the default ui- namespace

In order than a method to addClasses, this method is not simple, imagine if the user want to add the className to another internal element instead of the this.element how come we could do this with this option?

One option could be the user define a css class like this example below, that is resonable for me too:
.ui-element.customized-user-class .title {}

In my opinion they are two different options, and they are both important.

[Richard D. Worth]

That's something we've discussed before. We may discuss it again. But for now we have agreed that it's not something that we support. If we ever do support it, it would have to be complete, not partial. So it's not even an option for discussion as part of 1.6. It's big, complex, and far-reaching.

- Richard

[Scott González]

On Sat, Jan 17, 2009 at 1:42 PM, Eduardo Lundgren <eduardo...@gmail.com> wrote:

Richard,

What happen if a client using jQuery UI need to change the css namespace for some reason? If we don't have this option they will have to find and replace the "ui-" for "somethingelse-".

We haven't heard of any reasons and we can't think of any.
 

Using an option as cssNamespace, we are able to:

cssNamespace: false - increase the performance
cssNamespace: 'somethingelse-' - change the namespace
cssNamespace: undefined - use the default ui- namespace

Yes, we all fully understand what it would provide and we don't think it's necessary.

In order than a method to addClasses, this method is not simple, imagine if the user want to add the className to another internal element instead of the this.element how come we could do this with this option?

We don't want to support random customization of classes.  Either everything is classed or nothing, and you will only get this option of extremely simple (markup-wise) elements that require very high performance.
 

One option could be the user define a css class like this example below, that is resonable for me too:
.ui-element.customized-user-class .title {}

You should use CSS the way it is meant to be used by applying a custom class to the outermost element and writing styles appropriately.

$(el).plugin().addClass('myClass');

[Eduardo Lundgren]

It's not big nor complex Richard. It's a very simple and non-complex change.
What's the reason in not support this? Can you give me one good argument?

[Richard D. Worth]

If it's simple and non-complex create a ticket and attach a patch. If people download the patch and find it works for them, great - case closed. If people need the feature, we point them at the ticket and the patch, and they're in business. But the idea has already been tabled for 1.6. Post 1.6, with a ticket and a patch, you might have an easier time convincing people that the idea has merit and is worth consideration. Code talks.

- Richard

[Paul Bakaus]

On Sat, Jan 17, 2009 at 7:51 PM, Scott González <scott.g...@gmail.com> wrote:

On Sat, Jan 17, 2009 at 1:42 PM, Eduardo Lundgren <eduardo...@gmail.com> wrote:

Richard,

What happen if a client using jQuery UI need to change the css namespace for some reason? If we don't have this option they will have to find and replace the "ui-" for "somethingelse-".

We haven't heard of any reasons and we can't think of any.

Seriously Scott, are you kidding me? Every single application or framework with its own conventions may want to do this.
 

 

Using an option as cssNamespace, we are able to:

cssNamespace: false - increase the performance
cssNamespace: 'somethingelse-' - change the namespace
cssNamespace: undefined - use the default ui- namespace

Yes, we all fully understand what it would provide and we don't think it's necessary.

who's "we"? We, the borg? "We" is either you, or at maximum you and Richard. I'm sorry to be harsh, but this annoys me - there's no way you can speak for all of us at once. I still think the option is worth being added - yes, we decided it's too late to make the switch for 1.6, but there's no way you can kill it that easily.
 

In order than a method to addClasses, this method is not simple, imagine if the user want to add the className to another internal element instead of the this.element how come we could do this with this option?

We don't want to support random customization of classes.  Either everything is classed or nothing, and you will only get this option of extremely simple (markup-wise) elements that require very high performance.

Please look at cssNamespace and how it's implemented right now (i.e. in draggables). It switches the "ui-" namespace for all classes that are added (i.e. also ui-draggable-disabled). Easy, and useful.
 

 

One option could be the user define a css class like this example below, that is resonable for me too:
.ui-element.customized-user-class .title {}

You should use CSS the way it is meant to be used by applying a custom class to the outermost element and writing styles appropriately.

$(el).plugin().addClass('myClass');

As stated above, this won't solve the issue. There will still be "ui-draggable-disabled" on it, instead of "myClass-draggable-disabled".
 

[Wichert Akkerman]

On 1/17/09 8:54 PM, Paul Bakaus wrote:

On Sat, Jan 17, 2009 at 7:51 PM, Scott González <scott.g...@gmail.com> wrote:

On Sat, Jan 17, 2009 at 1:42 PM, Eduardo Lundgren <eduardo...@gmail.com> wrote:

Richard,

What happen if a client using jQuery UI need to change the css namespace for some reason? If we don't have this option they will have to find and replace the "ui-" for "somethingelse-".

We haven't heard of any reasons and we can't think of any.

Seriously Scott, are you kidding me? Every single application or framework with its own conventions may want to do this.

I'm curious: can you give a use case where that is needed? I can not think of one off hand.

Wichert.

[Paul Bakaus]

Sure thing. Say I'm building a java application framework, and I want to include jQuery UI as part of it. My framework my have it's own styling rules and conventions that are documented as API to the outside world and third parties (this is how it is for Liferay, for example). Rather than introducing another convention, they might want to stick with "myframework-myclass-selected", or something similar. This has been requested a couple of times, and is a real world use case.

Scott, now thinking of it, I still think your reply delivered the wrong message, but so did mine. I apologize for raging too much about it. Let's resume this particular discussion when 1.6 is out.
 

Wichert.

[Richard D. Worth]

On Sat, Jan 17, 2009 at 2:54 PM, Paul Bakaus <paul....@googlemail.com> wrote:

In order than a method to addClasses, this method is not simple, imagine if the user want to add the className to another internal element instead of the this.element how come we could do this with this option?

We don't want to support random customization of classes.  Either everything is classed or nothing, and you will only get this option of extremely simple (markup-wise) elements that require very high performance.

Please look at cssNamespace and how it's implemented right now (i.e. in draggables). It switches the "ui-" namespace for all classes that are added (i.e. also ui-draggable-disabled). Easy, and useful.

And totally incompatible with theming and ThemeRoller. This is just one consideration of many when I call a change like this big, complex, and far-reaching.

- Richard

[Paul Bakaus]

Absolutely, yeah. I think ThemeRoller compatibility is something that goes too far. People who will want to do this are likely big enterprises, and they figure how they style things themselves. Also, mostly the interaction plugins benefit from this.
 

- Richard

[Eduardo Lundgren]

To change it in the themeroller is not complex.

[Richard D. Worth]

Changing it in a theme is not complex. It's a search and replace of "ui-" with "whateveryouwant-". Same for code. Not tested, but I have to wonder if it isn't that simple. This is jQuery UI. We have declared a namespace for everything we do. In jQuery that namespace is $.ui. In css that namespace is 'ui-'.

- Richard

[Scott Jehl]

We've already done this when using several  UI themes on one page of the UI website. No need to change anything about the widget class names. Just add scope to your css.

Find: '.ui-'

Replace: '#mySection .ui-'

Quite simple. You can have a thousand non-conflicting themes on one page like this and our plugins need not know a thing about it. 

[Richard D. Worth]

Even better. Thanks Scott.

- Richard

[ScottJehl]

My answer to this was slightly simpler than what you'll really need to
do, but not much.
The example above was a little too greedy and would append #mySection
before every .ui- selector, even those that aren't at the beginning of
a selector string.

A quick regex could handle this though, as I noted here:
http://groups.google.com/group/jquery-ui/browse_thread/thread/f0109ae8701100f8

Something like:
find: ^\.ui
replace with: #mySection \.ui

...which would cover all the selectors at the beginning of a line.
Then you could get the ones that follow a comma like this:

find: , \.ui
replace with: , #mySection \.ui


I'm sure that could be combined into one regex with some wildcards,
but you get the idea.