OpenLayers OpenLayers

Ticket #1607 (closed feature: fixed)

Opened 2 months ago

Last modified 1 month ago

add methods for managing css class names

Reported by: tschaub Assigned to: tschaub
Priority: critical Milestone: 2.7 Release
Component: BaseTypes.Element Version: 2.6
Keywords: Cc:
State: Complete

Description

These can go in OpenLayers.Element and be named things like hasClassName, addClassName, removeClassName.

http://trac.openlayers.org/browser/sandbox/topp/almanac/lib/OpenLayers/BaseTypes/Element.js#L126

I'll add a patch.

Attachments

classy.patch (6.3 kB) - added by tschaub on 07/28/08 17:13:03.
functions for managing element class names

Change History

07/02/08 04:40:39 changed by tschaub

  • state set to Review.

Element tests pass in FF2. All I have available at the moment.

07/02/08 08:47:32 changed by pagameba

tests pass in FF3 and Safari 3.1.1

Before these become API methods, I propose that we change the names to hasClass, addClass, removeClass. The addition of Name in the method names doesn't add semantic value and its more to type - if these methods get widely used in the library itself then that just adds unnecessary size to the library (albeit minor).

As this is pretty common functionality of several existing libraries, I compared this implementation to what is being done in the mootools library and I have a couple of questions:

1) hasClass(Name) is used quite a bit in the add and remove functions, and it seems likely that these methods will start to find widespread use. Without wanting to get into premature optimization, the mootools approach is to use the indexOf String method with an optional separator seems faster:

(code) hasClass: function(element, name) {

return (' '+element.className+' ').indexOf(' '+name+' ');

} (end)

I just profiled the two approaches in FF3 using loops of 10000 iterations and the indexOf approach seems to be somewhat to quite a bit faster - I was getting variable results that didn't always make sense.

2) I don't quite understand how the regex in removeClass works, but the equivalent function in mootools is a bit different and I just want to make sure someone knowledgeable takes a look at the difference to make sure that our function covers all the bases. The mootools regex is:

(code) this.className.replace(new RegExp('(|\\s)' + className + '(?:\\s|$)'), '$1').clean() (end)

taking out the differences, the difference seems to be the addition of '?:' in the last part of the regex and $1 instead of ' '.

07/28/08 15:08:09 changed by euzuro

  • owner changed from euzuro to tschaub.

07/28/08 15:18:03 changed by euzuro

  • priority changed from minor to critical.

07/28/08 17:13:03 changed by tschaub

  • attachment classy.patch added.

functions for managing element class names

07/28/08 17:14:13 changed by tschaub

Function names updated as suggested by Paul and conferred by others here.

07/28/08 17:15:06 changed by tschaub

Oh, and I'd say patches for performance can come later.

07/28/08 17:16:40 changed by tschaub

  • status changed from new to closed.
  • state changed from Review to Complete.
  • resolution set to fixed.

(In [7579]) Adding functions to manage dom element class names. Use OpenLayers.Element.hasClass, addClass, removeClass, toggleClass for css class name management. r=pagameba,me (closes #1607)