Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

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

Open
jsmestad opened this Issue · 14 comments

2 participants

@jsmestad

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
Owner

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

@jsmestad

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
Owner

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

$("").insertHaml() ?

@creationix
Owner

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

@jsmestad

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

@creationix
Owner

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

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

@creationix
Owner

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

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
Owner

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

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
Owner

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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.