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

Iteration API #12

Closed
timoxley opened this issue Jan 3, 2013 · 35 comments
Closed

Iteration API #12

timoxley opened this issue Jan 3, 2013 · 35 comments
Milestone

Comments

@timoxley
Copy link
Member

timoxley commented Jan 3, 2013

Adding child items to the parent element manually sucks. Also means the list itself isn't reactive, only it's elements (and only while they exist).

Also made more painful by #5 because you have to do a little dance to remove the parent and render the child element (e.g.<li> or <option>) as in example below:

<!-- Target Result -->
<ul>
  <li data-text="name">Tim</li>
  <li data-text="name">Bob</li>
</ul>
<div id="user-item-template">
  <li data-text="name"></li>
</div>
var userItemTemplate = document.getElementById('#user-item-template').innerHTML
var parentListEl = document.getElementById('#parentList')
var users = [{name: 'Tim'}, {name: 'Bob'}]

users.forEach(function(user) {
    var itemEl = domify(userItemTemplate)
    reactive(itemEl, react(user))
    itemEl = itemEl.children[0] //  Remove el from parent element. Gross.
    parentListEl.appendChild(itemEl)
})

I guess copying rivet's style again wouldn't be too bad:

<ul>
  <li data-each-todo="list.todos">
    <input type="checkbox" data-checked="todo.done">
    <span data-text="todo.summary"></span>
  </li>
<ul>

What do you think? Is there a better API?

@tj
Copy link
Member

tj commented Jan 4, 2013

I've got a branch with this stuff actually, forgot about it, there's a few things I wanted to try with composition but the basic iteration stuff I was just doing to do each="user in users" to scope as "item" and each="users" to implicitly scope which loses access to the parent view

@timoxley
Copy link
Member Author

timoxley commented Jan 4, 2013

sounds clean, do push

@matthewmueller
Copy link
Member

I'd really like for this to support iterating through an array of primitives. This was one of my main issues with rivets. Something like:

fruits = ['apple', 'orange', 'pear'];
<ul each="fruit in fruits">
  <li><span data-text="fruit"></span></li>
<ul>

outputs:

<ul>
  <li><span>apple</span></li>
  <li><span>orange</span></li>
  <li><span>pear</span></li>
<ul>

@timoxley
Copy link
Member Author

timoxley commented Jan 4, 2013

Perhaps formatters could be used via | to supply custom iteration… e.g.

<ul data-each="fruit in fruits | toObject">
  <li><span data-text="fruit"></span></li>
<ul>
var fruits = ['apple', 'orange', 'pear'];

var fruitView = {
  toObject: function(val) {
    return { fruit: val }
  }
}
// edit: the data-each="fruit in fruits" would require a key 'fruits'. 
reactive(el, {fruits: fruits}, fruitView)

// edit: perhaps 'this' could be used so you don't have to supply any object key
// e.g. <ul data-each="fruit in this | toObject">
reactive(el, fruits, fruitView)

@timoxley
Copy link
Member Author

timoxley commented Jan 4, 2013

Also, being able to iterate over a hash would be useful.

e.g.

var fruits = {
  apple: {
    price: 2
  },
  banana: {
    price: 7
  }
}

reactive(el, fruits, {
  toObject: function(val) {
    return {
      fruit: val
    }
  }
)
<ul data-each="name, fruit in fruits">
  <li><span data-text="name"></span><span data-text="fruit.price"></span></li>
<ul>

Output:

<ul data-each="name, fruit in fruits">
  <li><span data-text="name">apple</span><span data-text="fruit.price">2</span></li>
  <li><span data-text="name">banana</span><span data-text="fruit.price">7</span></li>
<ul>

@timoxley
Copy link
Member Author

timoxley commented Jan 8, 2013

@visionmedia hey can you push that branch so i can have a play with it, even if it's not complete?

@karlbohlmark
Copy link
Contributor

@timoxley @matthewmueller I notice that you put the each directive on the parent of the element to be repeated. This diverges from how it is done in Rivets and I can't say I like the implication that you cannot have a template that expands into a list without a containg element. (This might be a reasonable restriction though)

Please share your reasoning!

..and yes I'd also like @visionmedia to share his implementation of this feature.

@tj
Copy link
Member

tj commented Jan 12, 2013

sorry haven't had time to get an example with it working yet to push the branch, it's in flux ATM

@matthewmueller
Copy link
Member

@karlbohlmark looping over a block is much more flexible than looping over a single element. Also, in rivets, they could have just changed:

<ul>
  <li data-each-tag="item.tags" data-text="tag:name"></li>
</ul>

to:

<ul data-each-tag="item.tags">
  <li data-text="name"></li>
</ul>

IMO this makes more sense.

@karlbohlmark
Copy link
Contributor

@matthewmueller I fail to see how it makes more sense to put the looping construct on any other element than the target for repetition.

Say you want a template for a table

<table>
  <caption>Please don't repeat me, I'm just a caption!</caption>
  <tr each="item in items"><td><button data-text="item.action"></button></td></tr>
</table>

How would you do this with the each attribute sitting on the parent of the item beeing iterated on? Or am I misunderstanding?

@matthewmueller
Copy link
Member

do you mean <td>? I don't really understand your example.

To be clear though: I'm not arguing against having a single repeating element. I could definitely see a use case for it, I just think iterating over a block is more useful.

@timoxley
Copy link
Member Author

@matthewmueller Above example has invalid html table structure, I think that's what's obscuring the intent… I believe it's trying to demonstrate a case where you have a repeated item which doesn't necessarily have a shared parent element, e.g.

<h1 data-text="essay.title"></h1>
<p each="paragraph in essay.paragraphs" data-text="paragraph"></p>

Otherwise you'd be forced have to have all the p elements wrapped inside a div or something

@karlbohlmark
Copy link
Contributor

Sorry, I accidently left out the td. @timoxley you got it. What's your take?

@karlbohlmark
Copy link
Contributor

Basically I see these scenarios:

  1. Repeat a single element
  2. Repeat an array of elements
  3. Repeat text content (could also be a mix of text and elements)

Approaches:
A) Put attribute on parent element
B) Put attribute of element to repeat.

A) means you always have to wrap with a parent element, but then you can handle 2) and 3) and some cases of 1)
B) means you can handle 1) without wrapping but for 2) and 3) you are forced to wrap.

If this was a compiled language I would suggest we introduced an <each> tag for when you have to do this forced wrapping, so that it wouldn't be reflected in the DOM, now I guess we have to live with it.

@timoxley
Copy link
Member Author

To be expressive, reactive should probably support both. I can't see any way you can safely differentiate which looping system to use using the same syntax though… perhaps just need a different keyword? in vs of? or from?

<p each="paragraph of essay.paragraphs" data-text="paragraph"></p>
<!-- vs. -->
<p each="paragraph from essay.paragraphs" data-text="paragraph"></p>

<dl data-each="def in definitions" data-id="definitions.name">
  <dt data-text="def.term"></dt>
  <dd data-text="def.definition"></dd>
</dl>

Or perhaps something completely different. I think we should probably just wait until there's any working implementation and then we can continue the speculating.

@tj
Copy link
Member

tj commented Jan 13, 2013

too many options is a -1 from me but I see what @karlbohlmark is saying, to me block was what I was drawn to as well, seems more natural coming from a for loop I suppose but I definitely agree that on the element itself is better. we can get a reasonable look without preprocessing though, each="foo in bar" isn't so bad, we just have to take a decent guess at which attributes might be used in the future haha

@matthewmueller
Copy link
Member

ahh my bad, I misunderstood what you were saying @karlbohlmark. You're just saying copy the parent (e.g. <tr>) along with it's children each iteration - that's fine with me.

@tj
Copy link
Member

tj commented Jan 13, 2013

initial stuff: https://github.com/component/reactive/compare/add;iteration

few notes:

  • recursion (though I would argue you should be separating these into separate views at this point)
  • ability to write this sort of thing without hacking ./lib would be nice but not a huge deal
  • how to handle changes

we could handle changes by simply clobbering and reiterating but that kinda gets back to just being a regular old shitty static html template engine, not ideal IMO

@tj
Copy link
Member

tj commented Jan 13, 2013

also instead of opting-in to access the parent as shown here: https://github.com/component/reactive/compare/add;iteration#L0R28

we could do the opposite where it's always scoped to the child object like here: https://github.com/component/reactive/compare/add;iteration#L0R20

and then use .name or @name some kind of identifier to reference the parent, though this would mess with recursion. Or .name references the child while name still references the parent

@weepy
Copy link

weepy commented Jan 13, 2013

could you just use "this" to reference the child.

@matthewmueller
Copy link
Member

+1 for scoping to the child object

I think clobbering and reiterating is the only way to go, I'm not sure it'd be worth the trouble to make it more granular than that.

@karlbohlmark
Copy link
Contributor

Do we save a detached clone of the template element to use in subsequent rendering?

@tj
Copy link
Member

tj commented Jan 15, 2013

@karlbohlmark yup, the first one is discarded ATM after use but we'll need to store it and make the thing reactive

@timoxley
Copy link
Member Author

RE accessing parent scope: handlebars uses ../. In reactive it might look something like this:

<span data-text="../name">Name</span>

While it looks a bit gawky, at least it has a familiar meaning.

Also +1 auto scoping to the child. This also matches handlebars behaviour:

  {{#each comments}}
  <h2><a href="/posts/{{../permalink}}#{{id}}">{{title}}</a></h2>
  <div>{{body}}</div>
  {{/each}}

@karlbohlmark
Copy link
Contributor

I think I'm -1 on auto scoping to the child, because that's more like a with statement and less like a familiar foreach loop construct.

@karlbohlmark
Copy link
Contributor

Introducing new names into scope is expected to be explicit. Principle of least surprise.

@timoxley
Copy link
Member Author

@karlbohlmark In the branch both formats work: each="friend in friends" creates a new var friend, and each="friends" autoscopes to child. I guess @visionmedia just needs to pick how to access parent scope.

@timoxley
Copy link
Member Author

@matthewmueller clobbering isn't very elegant and especially in the case of lists, could be very expensive/flickery. Would be cool if reactive could re-sort items automatically, ala http://substack.net/projects/sorta-vote/

data-sort="score"?

@weepy
Copy link

weepy commented Jan 28, 2013

I think a parent scope isn't always necessary ? Wouldn't you typically add
a reference to the child object.

What I think you do need is access to the original root of the view (like
KnockoutJS which uses $root).

@timoxley
Copy link
Member Author

@weepy parent scope is very useful. IIRC this was a pain point using mustache. You'd have to go in and write loops in your view code just to gain access to data you were just using 2 lines prior.

@timoxley
Copy link
Member Author

Another issue is passing an array directly to reactive:

var items = [item1, item2, etc]
reactive(el, items)

how would I refer to the array to iterate over it? perhaps this or $root or something:

<ul each="this">
  <li data-text="title"></li>
</ul>

@tj
Copy link
Member

tj commented Jan 28, 2013

to avoid the parent think .name vs name might be reasonable, .name for the while instead of @name for the parent. That whole concept would fall apart if you nest them but that's way too much shit for one view IMO haha

@domachine
Copy link

any updates on this? i'd be really interested in such a feature.

@defunctzombie
Copy link
Contributor

Done in next branch and will land for reactive 1.0

The syntax is as follows:

<ul>
    <li each="todos">{name}</li>
</ul>

The item with the each binding is the one iterated.

@timoxley
Copy link
Member Author

timoxley commented Mar 9, 2014

wow, awesome.

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

No branches or pull requests

7 participants