Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

{{#each}} now supports objects. #272

Closed
wants to merge 6 commits into from

Conversation

rosshadden
Copy link
Contributor

At the jQuery conference a few days ago, @wycats and I discussed adding a for-in loop to base.js to support loops over objects. After giving it some thought, I decided it would be a better idea to just extend the existing {{#each}} helper, so as not to saturate the project with too many existing helpers.

Given the template:

{{#each colors}}
    <ul>
        <li>{{@key}}</li>
        <li>{{short}}</li>
        <li>{{long}}</li>
    </ul>
{{/each}}

and the data object:

{
    colors: {
        red: {
            short: 'f00',
            long: 'ff0000'
        },
        blue: {
            short: '00f',
            long: '0000ff'
        },
        green: {
            short: '0f0',
            long: '00ff00'
        }
    }
}

The following will be rendered:

<ul>
    <li>red</li>
    <li>f00</li>
    <li>ff0000</li>
</ul>
<ul>
    <li>blue</li>
    <li>00f</li>
    <li>0000ff</li>
</ul>
<ul>
    <li>green</li>
    <li>0f0</li>
    <li>00ff00</li>
</ul>

*Note: We had decided to use @key for the magic key reference, and I still want this to be the case. However, for now it is '_key' because I could not figure out the former.

Please let me know what you think.

Also added the 'key' key to looped objects.
My goal is to make this {{@key}}, but am still working on it.
I would also like to unobtrusively make @key or @Index work for arrays.
@rosshadden
Copy link
Contributor Author

Adding the _key reference mutates the object passed in, which is undesirable (but obvious).

On another note, I had originally tried storing the key reference as @key, but this meant in the template we had to refer to key via {{[@key]}} (note the square brackets). This is also undesirable.

@brianreavis
Copy link

Currently with Handlebars, if an array is empty it will trigger the inverse of the block. This commit will break that, but otherwise it looks good.

#238 (but this version doesn't provide the key)

@rosshadden
Copy link
Contributor Author

Good call, @brianreavis. Our pull requests started out pretty dissimilar, but now they are close. Should we talk about merging them into one, or let @wycats pick?

@brianreavis
Copy link

Nah, I like yours! No reason to not go with it :)

@rosshadden
Copy link
Contributor Author

I just saw a recent commit that adds support for the @data variables I was referring to above. I will update the code of this pull request accordingly first thing tomorrow.

@wagenet
Copy link
Collaborator

wagenet commented Jul 31, 2012

This seems like a good idea. Lets get confirmation from @wycats if we can.

@tchak
Copy link

tchak commented Aug 17, 2012

@rosshadden you should use Ember.isArray(context) as context instanceof Array will not get all the enumerables

@tchak
Copy link

tchak commented Aug 17, 2012

actually here is my proposition :

if (Ember.isArray(context) && !Ember.isEmpty(context)) {
  for (var j = context.length; i<j; i++) {
    if (data) { data.index = i; }
    ret = ret + fn(context[i], { data: data });
  }
} else if (Ember.typeOf(context) === 'object') {
  for (var key in context) {
    if (context.hasOwnProperty(key)) {
      if (data) { data.key = key; }
      ret = ret + fn(context[key], {data: data});
      i++;
    }
  }
}

@rosshadden
Copy link
Contributor Author

I agree with what you are saying, @tchak, however Handlebars is entirely isolated from Ember. Thus we do not have those functions at our disposal. Therefore there is a trade-off to make between either adding those functions in for this single use case or not being as thorough as possible in the type checking.

@tchak
Copy link

tchak commented Aug 18, 2012

Wow, sorry I thought it was an ember issue... My bad! (we have issues notifications in campfire for both, and some times I miss the origin :) )

@mpetrovich
Copy link
Contributor

For simple key-value object pairs, will {{this}} refer to the value?

For example, given this context:

items: {
    34594: "abc",
    43982: "def",
    20985: "ghi"
}

Would this be the correct template syntax?

{{#each items}}
    {{@key}} = {{this}}
{{/each}}

@rosshadden
Copy link
Contributor Author

@mpetrovich: If memory serves, {{this}} does refer to the object, however I am not able to give you a definite answer at the time being.

However, given a slightly modified example:

var items = {
    34594: {
        letters: "abc",
        numbers: 123
    },
    43982: {
        letters: "def",
        numbers: 456
    },
    20985: {
        letters: "ghi",
        numbers: 789
    }
};

The following works as expected:

{{#each items}}
    {{@key}} = {{letters}} and {{numbers}}
    {{@key}} = {{this.letters}} and {{this.numbers}}
{{/each}}

where {{this.letters}} is the same thing as {{letters}}.

@wycats
Copy link
Collaborator

wycats commented Sep 10, 2012

Can you please add a test?

@wycats
Copy link
Collaborator

wycats commented Oct 13, 2012

@rosshadden Any progress here?

@rosshadden
Copy link
Contributor Author

In the interest of getting this done and merged, it may be faster if someone else who knew what they were doing could add a test to it. Otherwise, I will begin doing so on Tuesday.

@mpetrovich
Copy link
Contributor

I'll add tests tomorrow morning.

@mpetrovich
Copy link
Contributor

In testing this, I realized that object properties don't have an implicit order in Javascript. Most browsers iterate over object properties in the order in which they were defined, whereas some (eg. Chrome and Opera) iterate over numeric (and numeric-like) properties (eg. 1, "2") before non-numeric properties (eg. "first", "foo"). More details here: http://stackoverflow.com/questions/280713/elements-order-in-a-for-in-loop

Given that object iteration order is undefined and inconsistent across browsers, should we move forward? If so, I'll complete the unit tests and we'll document that #each with objects shouldn't be used if order matters.

@wycats, what do you think?

@rosshadden
Copy link
Contributor Author

Yeah this was the reason I have a different kind of for loop for arrays vs. other objects. My take on this is that we are already accustomed to object items not being in order for iterations, so this should never be a shock to developers. I can't think of a situation in which object order would ever matter, because if it ever does the developer should be using an array instead.

@mpetrovich
Copy link
Contributor

@wycats This PR has been updated with those tests.

@mikesherov
Copy link

Don't worry, it still is just @rosshadden and @mpetrovich's commits.... I just rebased it so @wycats can pull it in.

@wycats
Copy link
Collaborator

wycats commented Oct 15, 2012

Merged @mikesherov's rebase

@wycats wycats closed this Oct 15, 2012
@kud
Copy link

kud commented Nov 22, 2012

Is there any particular stuff to do? coz it still doesn't work here when I use this code: https://gist.github.com/4130484 contrary to this one: https://gist.github.com/4130488

My code is pretty simple:

<div class="info-container">
        <div class="title"><h4>Casting</h4></div>
        {{#if casting}}
        {{#each casting}}
        <h5>{{this.status}}</h5>
        <ul class="list">
            {{#each this.casts}}
            <li>{{this.fullname}}</li>
            {{/each}}
        </ul>
        {{/each}}
        {{else}}
        <div class="description">
            <p>Aucune information disponible à ce sujet.</p>
        </div>
        {{/if}}
    </div>

Do you have any idea?

Thank you very much.

@wagenet
Copy link
Collaborator

wagenet commented Nov 23, 2012

@kud, have you tried doing it with the this.? If that still doesn't work, can you put together something on JSFiddle or JSBin to show the problem?

@kud
Copy link

kud commented Nov 26, 2012

@wagenet It's ok, sorry to have disturbed you.

In fact, I use bower, and as there's no precompiled version of handlebars in the official handlebars repo, I used this one: - handlebars git://github.com/components/handlebars.js

It was called as same version "1.0-rc1" but it seems to have still some bugs.

So I've download yours $ bower install handlebars.js (git://github.com/wycats/handlebars.js.git) and rake everything.

Now it works like a charm.

Thank you!

@wagenet
Copy link
Collaborator

wagenet commented Nov 26, 2012

@kud, the official rc1 was released a bit over two months ago so we've gotten some fixes in since then. Might be time to do a second RC soon. /cc @wycats

@bretweinraub
Copy link

Any pointers to an authoritative writeup of the issue with handlebars and {{each}} of objects? I see the problem, but only in some circumstances that seem totally unrelated (like the HTML outside the {{each}} helper......).

@rosshadden
Copy link
Contributor Author

As far as I know there isn't a problem with {{each}} on objects. I have been using it in production for months. If you are having issues try compiling handlebars.js from source. Or let me know / email me and I will send you the version I use so you do not have to compile (white elephant in the room: it's a bitch for a non-rails person to compile a project like this!).

@bretweinraub
Copy link

Sure I'll take the email : bretweinraub AT yahoo DOT com

For instance this works:

<table><tr>
{{#each courses}}
<tr>{{title}}</td></tr>
{{/each}}
</table>

But insert of a "td" tag before {{title}}, it stops! For example:

<table><tr>
{{#each courses}}
<tr><td>{{title}}</td></tr>
{{/each}}
</table>

Literally that piece of before {{title}} shuts the whole thing down. Obviously I would've taken this to stackoverflow but actually this thread seems to be the best thing out there..... Merci beacoup from Switzerland!

@mpetrovich
Copy link
Contributor

@bretweinraub Try making courses an array and see if the problem persists. If so, it's most likely a Handlebars parsing issue. If not, it might be a strange issue with {{#each}}

@bretweinraub
Copy link

Hi thanks for the responses.

I fixed the problem. The template was sitting in a "div"; and the browser ate my homework. Errr I mean it had randomly reorganized the text and was moved things around. It only does this with

elements ... everything else works fine.

Like this

<div id="templ">
   the template
</div>

There was a jQuery call $('#templ').html() that was extracting the template. MORAL of the story; don't do that.

We have now stuck the template into a URI encoded string; which since a PHP server generated it, of cours we can't use decodeURI(). But it all works, I can send example of how to do it to anyone who cares.

@mpetrovich
Copy link
Contributor

You could also put it in a <script type="text/x-template"> or similar, which will also prevent the browser from trying to do funky DOM parsing.

@bretweinraub
Copy link

genius! Where were you about 1000 keystrokes ago?

@dak
Copy link

dak commented Feb 19, 2013

The value of {{this}} appears as an empty string when iterating over an object in Safari 6 (6.0.2 - 8536.26.17 on OS X 10.8.2). It appears that "this" can still be passed to scripts and evaluated perfectly fine.

To make the bug more bizarre, it behaves as expected when the Web Inspector is open. It also works as expected in Google Chrome. I haven't tested other browsers.

@mpetrovich
Copy link
Contributor

@dak1 I've noticed a related issue that has the same symptoms (Safari 6 Mac, but only if dev tools are not open), where a template with 3 placeholders renders fine, but add a fourth and nothing renders. I'll try to create a reduced test case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants