Comments and changes to this ticket

• 

  MadRabbit September 30th, 2010 @ 09:44 AM

  • Assigned user set to “MadRabbit”
  • Tag changed from “element#wrappers” to “element#wrappers, feature-request”

  Hi there,

  Yes, I thought about, and it is a net idea, but it will slow down the elements construction/initialization. You have a simple case but people will ask for the whole css3 support.

  I'm not against the functionality, but I think it should go into a plugin, so people had a choice have it or not. I'm going to bend this part a bit for RightJS 2.0.1 and if you would care to write such a plugin I could expose this part so it could be externally replaced.

• 

  MadRabbit September 30th, 2010 @ 09:49 AM

  • Tag changed from “element#wrappers, feature-request” to “dom, element, feature-request, wrappers”

• You flagged this item as spam.
  

  LemmingKing September 30th, 2010 @ 11:26 AM

  Hi,

  I thought about using className as a lightweight alternative to fullfeatured css-selectors.
  Let alone the fact that css here can be a great overkill, using selectors you will meet two huge problems:
  1. times calling code similar to Element#match() which is slow ( is the number of your rules and you have 6 of them from the very beginning: 'FORM', 'INPUT', etc. This number will grow with the number of your widgets declared);
  2. This is hard to deal with rule detalization:
  '#container p.my-p a.my-a' '#container p a' '#container p.my-p a' You have to select the most peculiar one that still matches your element.

  But classes have no privileges among them (so you can get first matched) and searching through them is reasonably quick (usually, you have 2-3 of them for an element, so you have to do 3-4 queries to Element.Wrappers and this number will NOT grow with the number of your widgets).

  So, I don't think this will slow down the $() function dramatically, but I don't mind this implemented as a plugin (ok, in this case I'll try to write it).

• You flagged this item as spam.
  

  LemmingKing September 30th, 2010 @ 11:32 AM

  fix:
  1. N times calling ... (N is the number ...);
  (I just wrapped N with angle brackets)

• 

  MadRabbit September 30th, 2010 @ 04:24 PM

  Yes, I get the idea why you use className and that's cool. Just I meant that if you allow one thing, then people will probably want the rest of them.

• You flagged this item as spam.
  

  LemmingKing September 30th, 2010 @ 06:33 PM

  But you have already allowed one thing!
  Registering custom wrappers for tags is available, and people do want the rest. At least some of them :)

  But currently it is hard to implement this functionality even as a plugin (I dislike the idea of patching or replacing 'Wrapper' function - this could lead to some difficulties during updates).

• 

  MadRabbit October 1st, 2010 @ 09:55 AM

  • Milestone set to “2.1.0 Release”
  • Milestone order changed from “196630” to “0”

  That's exactly my point. Anyhow, I'm going to work on the next core update soon and I'll expose this part of code so it could be manipulated externally.

  Will keep you posted

• 

  MadRabbit October 9th, 2010 @ 04:58 PM

  • State changed from “new” to “resolved”

  Okay, I've moved this thing into an externally fixable function

  http://github.com/rightjs/rightjs-core/blob/master/src/dom/wrapper....

  will cook some plugin after the release

• You flagged this item as spam.
  

  LemmingKing October 13th, 2010 @ 02:30 AM

  • Tag cleared.

  Hi again!

  Please, check this (my attempt to write this plugin):
  http://pastebin.org/159744
  and this (demo HTML):
  http://pastebin.org/159747

  I came up with the idea of maintaining three different wrapper dictionaries:
  - full-featured css - "tag.class" or just ".class" - "tag" (for compatibility with the default wrappers)

  So, you will not decrease the performance until you actually register some slow css-ruled wrappers.
  Also, I decided to use the last matched rule in the order of registering (so that you could override previously registered Wrappers).

  Btw, I've not tested this enough yet, and probably I will not be able to do it this week, sorry.

  • custom-wrapperstar.gz 39.8 KB
• 

  MadRabbit October 13th, 2010 @ 10:22 AM

  some clever thinking over there!

  I would bend couple of things here and there but generally, generally I like it.

  Could you push your things on github? I'll merge them into the plugins on the weekend.
  And for god sake specify you real name, the motherland should know its heros! 8)

• You flagged this item as spam.
  

  LemmingKing October 14th, 2010 @ 11:13 PM

  Hi,

  I cleaned up the code a bit, wrote some documentation and fixed this plugin so that it worked with IE8.
  But I'm not able to make it work with old browsers because you didn't provide any way to call "native" querySelectorAll-method (only "find", which in turn calls the $ function again and will probably end up infinitely recursing).

  So, to finish this work I need something like querySelectorAll-method returning /bare/ dom elements available for old browsers.

  Should I post a ticket for this? Or may be it's better to just remove support for full-featured css-ruled wrappers from this plugin?

  BTW: Why to do that parent searching related stuff in the "match" method? (I just want to realize if I can skip this in my "_match" function and use 'document' instead).

  • right-custom-wrappers.zip 39.8 KB
• 

  MadRabbit October 20th, 2010 @ 02:04 PM

  Looked into this one once again today, seems like you're right, I'll need to adjust the match method so it worked properly with raw dom elements. Gonna fix it in the 2.1.2 release.

• 

  MadRabbit February 25th, 2011 @ 11:08 AM

  • State changed from “resolved” to “open”

  Finally, had a bit of time for this thing.

  I cooked a sort of beta implementation in rightjs-plugins over here
  https://github.com/rightjs/rightjs-plugins/blob/master/src/casting/...

  Plus I've made you a build, see the attachment.
  Please check it out, and let me know what you think

  • right-casting-src.js 3.8 KB
  • right-casting.js 1.1 KB
• 

  MadRabbit February 25th, 2011 @ 11:08 AM

  • Milestone cleared.

• You flagged this item as spam.
  

  LemmingKing February 26th, 2011 @ 02:33 AM

  Hi!
  Shame on me, I missed the new version of RightJS came out!

  Looked into this build today. Looks cool, I see, you've made a Right-thing from my draft :)

  There is a little bug in the 'add' method (it seems like broken during refactoring).
  Here I put the corrected version: http://pastebin.com/1MNtufCY

  Also, I almost didn't test this build, will do later.

• 

  MadRabbit February 26th, 2011 @ 09:56 AM

  No worries, let me know if you'll end up with something working.

  btw: I think the tag + key checks should happen before the key ones, coz it's kinda how CSS works, the more specific rules should go first.

• You flagged this item as spam.
  

  LemmingKing February 27th, 2011 @ 01:24 AM

  I totally agree!

  Made this, and also done the following:

  • altered RegExps in the "add" method (found a few oddities there);

  • moved #id matching before .class ones;

  • altered the rules adding/removing procedures so that tag names were always UPPERCASE (except in generic css rules)

  e.g.:
  Element.Wrappers.add('div.class-name', MyWrapper); // is stored as 'DIV.class-name'
  Element.Wrappers.has('div.class-name'); // --> true;
  Element.Wrappers.has('DIV.class-name'); // --> true;

  • altered the 'has' and 'remove' methods so that they could accept both selectors and wrappers;

  e.g.:
  Element.Wrappers.add('div.class-name', MyWrapper);
  Element.Wrappers.remove('DIV.class-name'); // remove this selector
  Element.Wrappers.remove(MyWrapper); // remove all selectors bound to the MyWrapper

  • added a NOTE about complex css selectors to the disclamer

  But, I still haven't tested it in the IEs...
  Will do later :)

  Here it is:
  http://pastebin.com/BYc0sXLz

  • right-castingtar.gz 42.1 KB

• You flagged this item as spam.
  

  LemmingKing February 27th, 2011 @ 12:09 PM

  Well, after breaf testing this seems to work under IE 6,7,8 and Opera 11!
  Don't have any possibility to test under IE9 though.

• 

  MadRabbit February 27th, 2011 @ 12:27 PM

  Nice work! Could you also hook me up with a simple test page? So we could check the things later on

• You flagged this item as spam.
  

  LemmingKing February 27th, 2011 @ 02:01 PM

  Ok, the simpliest one is included into the archive above.
  I'll make a more detailed one in a few days (don't have time today and tomorrow).

  BTW: I wonder if there is any way to remove elements from cache
  e.g.
  MyWrapper = new Class(Element, { getData: function() { return this.get('data-widata'); } });
  RightJS.Element.Wrappers.add("div.my-widget", MyWrapper);
  alert($('my-widget-1').getData()); // at this point, $('my-widget-1') is cached
  RightJS.Element.Wrappers.remove(MyWrapper);
  alert($('my-widget-1').getData()); // still works

  Removing wrappers will hardly be a frequent operation, but I think, user should be able to decide if he wants to keep all cached Widgets available or not.

• 

  MadRabbit February 27th, 2011 @ 02:25 PM

  okay, thanks, I'll check the things a bit later.

  regarding the cache. it get cleaned up automatically when you instance a new wrapper with the same dom-element, kinda like that

  var el = document.createElement('div');
  el.id = 'boo-hoo';
  
  var wrap1 = new Element(el);
  $('boo-hoo') // -> wrap1
  
  var wrap2 = new Element(el);
  $('boo-hoo') // -> wrap2
  

  If you feel particularly evil and want to clean it up manually you can do that via the RightJS.Wrapper.Cache list

  var el = document.createElement('div');
  var wrap = new Element(el);
  
  var uid = $uid(el);
  Wrapper.Cache[uid] = undefined;
  

  But be cautious, there are a bit of trickery down there, like for example documents and windows use negative UIDs, Events don't get cached at all and so on. Better use the first approach, it will take care of things automatically.

• 

  MadRabbit March 2nd, 2011 @ 03:48 PM

  • Tag set to “casting, plugin”

  Awright, I had another swoop over this thing, and there are several changes

  a) I got rid of the css matchers. The main reason is that they have no sense when you create new elements on your own without having them attached to the dom-tree. Besides, it would be awfully slow.

  b) Added Wrappers.has() method to check if it kinda has a wrapper for some css-rule.

  c) Added proper unit-tests for the things.

  Attaching a fresh build, lmk how it went for you

  • right-casting-src.js 3.6 KB

• You flagged this item as spam.
  

  LemmingKing March 3rd, 2011 @ 01:15 PM

  Hi!
  Sorry for being slow, I was busy recently...

  Well, removing generic css selectors is ok, but I would keep the other removed features:
  * the possibility to call "has" and "remove" with the Wrapper class as an argument; * css-preparation (pre-uppercasing of the tagName) when adding/removing rules. Don't see any reason to throw them away.

  Also, just noticed:
  You use the following example in the USAGE block: 'div#boo.hoo'. but this kind of selector is not supported by the current version of plugin. I think, USAGE should contain a list of the actually available selectors.

• 

  MadRabbit March 3rd, 2011 @ 01:24 PM

  remove actually works with a class, as for the #has one, yes, why not. comments are also fixable :)

• You flagged this item as spam.
  

  LemmingKing March 3rd, 2011 @ 02:44 PM

  Oops, sorry, didn't see it :)
  But, well, pre-uppercasing tag names when registering wrapper is not there anyway...
  Well, let it be :)

• 

  MadRabbit March 9th, 2011 @ 10:29 AM

  • State changed from “open” to “resolved”

  http://rightjs.org/plugins/casting