creationix / jquery-haml

jQuery-haml is a haml like language written in JSON. This allows for easy dom building so that web apps can do more work independent of the server for a better user experience.
http://plugins.jquery.com/project/jquery-haml
MIT License
111 stars 6 forks source link

$().haml() to return reference to generated object? #3

Open jsmestad opened 14 years ago

jsmestad commented 14 years ago

I feel its more useful to have the .haml() command return a reference to the haml generated element once its inserted into the DOM. This would allow for something like:

var item = $("#item").haml([".item", "im an item"])
item.click(function() {...})

There may be a way to do this already built-in?

creationix commented 14 years ago

It should do this already, if it doesn't, then it's a bug.

jsmestad commented 14 years ago

Alright well the following code:

jQuery.fn.loadTemplates = function() {
  var self = $(this);
  $.getJSON(Spiff.templates_path(), function(data) {
    $.each(data, function(i,template){
      var template = self.haml([".template", {"data-template-id": template._id},
                  [".hero", 
                   ["%img", {src: template.stock_image}],
                   ["%h4", "Add"]
                  ],
                 [".description.audi_font", template.model + " Template"]
                ]);
      console.debug(template); // this returns self instead of the haml root
      });
  });
  return this;
};

See comment for whats happening. I may be doing something clearly wrong.

creationix commented 14 years ago

Oh, I see what you mean. I had forgot that I was chaining on the original jquery query result.

It's a simple change, but I don't think we should change the default behavior for two reasons:

  1. The jQuery plugin guidelines say "Your method must return the jQuery object, unless explicity noted otherwise."
  2. Many people are already using the plugin and depending on the old behavior.

I could provide an alternate api that returns the new set of created elements instead of the original set of matches. What would be a good name for such a method?

jsmestad commented 14 years ago

$("").insertHaml() ?

creationix commented 14 years ago

Ok, it's added. I'm not sure how it would work if your search result contains more than one node though.

jsmestad commented 14 years ago

It should return the top node, so in general it would return the .template div.

creationix commented 14 years ago

In jquery plugins can apply to a set of elements, not just one.

For example, you can do $("div").haml(.....) and the constructed dom tree will be appended to each and every div on the page. So I'm not sure which of them will be the one returned to you.

For you case you're probably fine.

jsmestad commented 14 years ago

So in that case it should return a collection of all the inserted haml elements?

creationix commented 14 years ago

perhaps, but I'm not sure jquery's append gives me that option. Also the tree you're appending may have more than one root node.

jsmestad commented 14 years ago

Also just to mention your comment about jquery returning the result set by default.

think of something like this...

$('div').find('h4').click(..).end().append('...')

I was hoping that the .haml() function would do something similar since I feel the expected behavior is more of an insert AND find then just an append() command. I do realize the confusion, but maybe we could do an API increment and have:

$("div").appendHaml() // this would be the current .haml() behavior

$("div").insertHaml() // this would be the additional haml

All this would change is the $("div").appendHaml(...).end() from the current $("div").haml() code

This also allows for other behaviors to be added later without confusion with the original .haml() function. I think that function would be better served doing a simple render than doing any DOM manipulation.

var html = $.haml(...)

if you increment the major version people should expect the behavior to change. Its not my lib but just offering my insight for what its worth.

creationix commented 14 years ago

I'm fine with changing the API as long as the new behavior is well defined. What would be the result of insertHaml under this case?

$("div").insertHaml([".cool"], [".crazy"])

Assuming your page has more than one div on it, this will append two divs to the bottom of each div. jQuery is nice about making collections look like single elements, but it doesn't work for two dimensional structures like this.

If you knew that you were inserting into only one node, then the result could the collection of the inserted haml tree. If you knew that the haml tree would always have a single root, then you could return a collection of all the roots.

Early on when developing haml I had artificially inserted a root node at the top of every haml tree, but later found this behavior was unexpected and caused styling issues most of the time. Not to mention it can really bloat the DOM when using many small haml insertions.

The insertHaml I just added assumes that you're inserting into only one target node on the page and returns the same jQuery collection that was given to the append function.

jsmestad commented 14 years ago

I think the insertHaml should only take one element at a time (similar to the before() and after() functions do now). So that would change the API into: $("div").insertHaml([".cool"]).end().insertHaml([".crazy"])

We may also look at doing beforeHaml() and afterHaml() to mirror the jQuery functions.

creationix commented 14 years ago

That may work, you're welcome to fork my repo and add in the changes. It sounds like you know that part of jQuery better than me anyway. I think I'd like a haml.before() and a haml.after().

Since it's not a trivial change, I won't have time to do it myself anytime soon. I got myself into too many projects at once.

jsmestad commented 14 years ago

I am in the same situation, so I totally understand. I think we may want to define revised API and an approach to handle these kinds of requests in the future to make everyones lives easier. Ill fork and start work when I can.