angular / angular.js

AngularJS - HTML enhanced for web apps!
https://angularjs.org
MIT License
58.8k stars 27.48k forks source link

ngRepeat with DL / DD / DT #1891

Closed leeola closed 11 years ago

leeola commented 11 years ago

Is there any timeframe on when (and how?) ngRepeat is going to handle Definition Lists?

In case you are not familiar with the problem: Currently ngRepeat works with single elements. For most HTML, this works fine.. however for definition lists such as <dl>, this does not work. Why? Because definition lists uses multiple elements to define a single list item.

Now, there are multiple ways around this, but reading here suggests that Angular is going to use comments to support repeating elements.

Any idea on when this feature is going to be implemented? Or if there is a somewhat stable branch that can be merged in to address this? The existing solutions currently are very hacky, against standards, and prone to breaking code in IE/etc.

Thoughts?

edit: By "stable branch that can be merged", i meant a branch that i could fork to run on my site and address this issue until the code is officially merged in. My apologies for the poor wording :)

maxcan commented 11 years ago

second this. I've been having this issue for a while.

petebacondarwin commented 11 years ago

@IgorMinar did some work on a comment-based ng-repeat (https://github.com/angular/angular.js/pull/1646) but since comments don't play so well on all browsers it is not likely to be merged. Instead they are looking at an alternative solution, probably with either "start and end repeat tags" or "start-repeat and number of elements to repeat label"

maxcan commented 11 years ago

How about allowing templates inside pairs like {{ }}

maybe: {{! ng-repeat = "foo in l"

foo
{{ l }}

!}}

On a related note, i have a similar issue with Would there be large technical challenges to a second interpolation form like {{{ }}} that did not escape html characters but also didn't create a new span?

On Tue, Jan 29, 2013 at 12:38 PM, Pete Bacon Darwin < notifications@github.com> wrote:

@IgorMinar https://github.com/IgorMinar did some work on a comment-based ng-repeat (#1646https://github.com/angular/angular.js/issues/1646) but since comments don't play so well on all browsers it is not likely to be merged. Instead they are looking at an alternative solution, probably with either "start and end repeat tags" or "start-repeat and number of elements to repeat label"

— Reply to this email directly or view it on GitHubhttps://github.com/angular/angular.js/issues/1891#issuecomment-12856484.

zilles commented 11 years ago

I just ran into this issue with a table that wants to generate two 'td' elements per object in an array.

bowsersenior commented 11 years ago

+1 for adding this feature. Just ran into this same limitation.

es128 commented 11 years ago

:thumbsup:

jensens commented 11 years ago

Same problem here. I'd love to see something like "ng-repeat-children" repeating only the children but not the current element or a kind of "ng-omit-tag" (which removes the current tag but leaves children in place) to be used together with ng-repeat. This would avoid tons of invalid markup generated to workaround this problem.

lgalfaso commented 11 years ago

Do not fully understand the limitation that is under discussion. Why putting the ng-repeat on the parent element does work? http://jsfiddle.net/JyWdT/13/ Can someone create a fiddle that explains the issue in more detail?

zilles commented 11 years ago

lgalfaso, Your fiddle creates multiple definition lists each with one definition. The desired output is one definition list with multiple definitions.

Or in my case, I need to create a pair of "td" elements for every item in an array. Say I have an array,

[{name:dan, age:15}, {name:steve, age:21}]

and I need to output:

<tr><td>dan</td><td>15</td><td>steve</td><td>21</td></tr>

There is no way to accomplish that with angular at this time.

logicbomb commented 11 years ago

can't you do that by putting the ng-repeat on the tr element?

http://plnkr.co/edit/lJvkOpz0NnKWcfEeM4lE?p=preview

On Sun, Apr 21, 2013 at 2:24 PM, zilles notifications@github.com wrote:

lgalfaso, Your fiddle creates multiple definition lists each with one definition. The desired output is one definition list with multiple definitions.

Or in my case, I need to create a pair of "td" elements for every item in an array. Say I have an array,

[{name:dan, age:15}, {name:steve, age:21}]

and I need to output:

dan15steve21

There is no way to accomplish that with angular at this time.

— Reply to this email directly or view it on GitHubhttps://github.com/angular/angular.js/issues/1891#issuecomment-16716788 .

logicbomb commented 11 years ago

Oh, I see, disregard my previous comment On Apr 21, 2013 9:09 AM, "jason turim" jason.turim@gmail.com wrote:

can't you do that by putting the ng-repeat on the tr element?

http://plnkr.co/edit/lJvkOpz0NnKWcfEeM4lE?p=preview

On Sun, Apr 21, 2013 at 2:24 PM, zilles notifications@github.com wrote:

lgalfaso, Your fiddle creates multiple definition lists each with one definition. The desired output is one definition list with multiple definitions.

Or in my case, I need to create a pair of "td" elements for every item in an array. Say I have an array,

[{name:dan, age:15}, {name:steve, age:21}]

and I need to output:

dan15steve21

There is no way to accomplish that with angular at this time.

— Reply to this email directly or view it on GitHubhttps://github.com/angular/angular.js/issues/1891#issuecomment-16716788 .

stoussaint commented 11 years ago

As I just encounter the dl / dt + dd issue, I confirm that a new ng-repeat-children (or ng-repeat-inner) is a missing mandatory directive.

choffmeister commented 11 years ago

+1

lgalfaso commented 11 years ago

Proposal for a new directive ng-repeat-inner. If this proposal is ok, then will modify all the guides

bowsersenior commented 11 years ago

I think it would be better to use the same ng-repeat directive, but allow for that directive to be used on HTML comments, as the original post suggests.

Something like this should then work:

<dl>
  <!-- ng-repeat="(name, definition) in myList" -->
  <dt>{{name}}</dt>
  <dd>{{definition}}</dd>
  <!-- /ng-repeat -->
</dl>

Not sure exactly how the beginning and end of the block to be repeated would be achieved ( <!-- /ng-repeat --> was suggested by @ProLoser), but I think that using HTML comments would be an elegant solution.

maxcan commented 11 years ago

Given that minifiers can kill comments and other issues with comments, I don't think this is great. However, I like the idea of using ng-repeat. would it be feasible to include an attribute ng-repeat-single-root or something which when set to true will run the ng-repeat without the root element? For example:

{i}
{i}

would become:

1
1
2
2
3
3

On Tue, Apr 23, 2013 at 3:54 PM, Mani Tadayon notifications@github.comwrote:

I think it would be better to use the same ng-repeat directive, but allow for that directive to be used on HTML comments, as the original post suggests.

Something like this should then work:

{{name}}
{{definition}}

Not sure exactly how the beginning and end of the block to be repeated would be achieved (I made up the in my example), but I think that using HTML comments would be an elegant solution.

— Reply to this email directly or view it on GitHubhttps://github.com/angular/angular.js/issues/1891#issuecomment-16891970 .

zilles commented 11 years ago

maxcan, I can see the advantages of "single-root", since you keep the source html valid, but that construct doesn't let you add anything else inside the "dl" tag... A fixed definition for example that you want to include before your loop of dynamic definitions. lgalfaso's solution would handle that.

lgalfaso commented 11 years ago

@maxcan without a big rework or a performance hit, I do not think it is possible to add this property ng-repeat-single-root (well, add it and make it dynamic based on it). The rational is that there is a high chance that a change on this property would cause a recompilation [and nobody likes recompilations]

ProLoser commented 11 years ago

I like @bowsersenior's suggestion, but I'd prefer this as the closing comment: <!-- /ng-repeat -->

maxcan commented 11 years ago

That's a shame on the performance hit. Anyway, I strongly endorse @lgalfaso's solution, assuming you can put the directive in a div and not just a comment..

On Wed, Apr 24, 2013 at 1:58 PM, Dean Sofer notifications@github.comwrote:

I like @bowsersenior https://github.com/bowsersenior's suggestion, but I'd prefer this as the closing comment:

— Reply to this email directly or view it on GitHubhttps://github.com/angular/angular.js/issues/1891#issuecomment-16964155 .

bowsersenior commented 11 years ago

I concur with @ProLoser that <!-- /ng-repeat --> is a better closing tag in the HTML comment approach. One note, angular allows directives to be defined in HTML comments, so I don't think HTML comments can be discarded as a potential solution for this problem. Keep in mind that the core angular developers indicated they planned to use HTML comments to address this issue in this stack overflow question (as pointed out by the original poster):

maxcan commented 11 years ago

I dont mind discarding them as long as there is a way to do this without comments.

On Wed, Apr 24, 2013 at 2:41 PM, Mani Tadayon notifications@github.comwrote:

I concur with @ProLoser https://github.com/ProLoser that is a better closing tag in the HTML comment approach. One note, angular allows directives to be defined in HTML comments, so I don't think HTML comments can be discarded as a potential solution for this problem. Keep in mind that the core angular developers indicated they planned to use HTML comments to address this issue in this stack overflow question (as pointed out by the original poster):

— Reply to this email directly or view it on GitHubhttps://github.com/angular/angular.js/issues/1891#issuecomment-16969268 .

hsdk123 commented 11 years ago

+1 to @lgalfaso's ng-repeat-inner. I'd finally like something that just gets this done.

bsantucci commented 11 years ago

Knockout does this with comments. I'm trying to port something I did with the dl tag in knockout and it seems like that is not possible until this issue is addressed.

lgalfaso commented 11 years ago

This is my opinion, so without the input from @mhevery or @IgorMinar, take this with a grain of salt. I do see that making ng-repeat work with comments is more flexible, anyhow It does not look natural without a change to $compiler so it can handle a directive of type "between comments". The reason being that it looks odd that a directive needs to take care about nesting and looking outside of the element that it is given to do what it is suppose to

hsdk123 commented 11 years ago

If comments/no comments are the reason of an ongoing controversy, can't we just have them both? I find it difficult to understand that an issue as common as this hasn't been addressed for more than 3 months. If the reason behind this is mainly due to a controversy on implementation, it seems more preferable to have something actually implemented (and perhaps updated/added in the future) than wait around longer with nothing for something more "optimal".

In most cases though, it seems to me that if ng-repeat-inner refers to repeating a set of elements "within" a certain something, it implies that in most cases you've already created an exclusive outer container of some sort. Hence while a comment implementation might be more versatile (albeit perhaps an extra 1/2 lines of code that I'd like to avoid if at all possible), I feel that it may be less used (and perhaps should be less prioritised) than an in-element directive.

bowsersenior commented 11 years ago

The issue of whether to use HTML comments does not have any relation to the fact that this issue has been outstanding for a few months. I think all that is needed is some attention from the angular maintainers to decide the issue and implement a solution.

es128 commented 11 years ago

In case you missed it, see here https://github.com/angular/angular.js/pull/1646 regarding problems with comment-based repeaters. @lgalfaso solution is much more promising at this point.

bowsersenior commented 11 years ago

Thanks for pointing out #1646 . There is a comment in that discussion by @mhevery that proposes "a better way to do it" without showing any code (probably a formatting error):

I wish we knew what that "better way" was!

IgorMinar commented 11 years ago

The consensus in the core team is that currently the best way to approach this problem is one of the following (we haven't settled on syntax but implementation is almost the same for all of these):

all these examples contain two td tags, I did this on purpose even though it's illegal because I wanted to show that the repeater should allow repeating over arbitrary number of elements - it would be better to convert this examples to use table and table rows, but I'm about to go offline now and don't have time to refactor them.

Syntax A:

<dl>
  <dt ng-repeat="(name, definition) in myList">{{name}}</dt>
  <dd ng-repeat-next>{{definition}}</dd>
  <dd ng-repeat-next>{{definition}}</dd>
</dl>

Syntax B:

<dl>
  <dt ng-repeat-start="(name, definition) in myList">{{name}}</dt>
  <dd>{{definition}}</dd>
  <dd ng-repeat-end>{{definition}}</dd>
</dl>

Syntax C:

<dl>
  <dt ng-repeat="(name, definition) in myList" ng-repeat-start>{{name}}</dt>
  <dd>{{definition}}</dd>
  <dd ng-repeat-end>{{definition}}</dd>
</dl>

Syntax D:

<dl>
  <dt ng-repeat="(name, definition) in myList" ng-repeat-group>{{name}}</dt>
  <dd ng-repeat-group>{{definition}}</dd>
  <dd ng-repeat-group>{{definition}}</dd>
</dl>

Long term, once we have better browser support, we'll start using the new <template> element which will allow us to do simply this:

Syntax X:

<template>
  <dl>
    <ng repeat="(name, definition) in myList">
      <dt>{{name}}</dt>
      <dd>{{definition}}</dd>
      <dd>{{definition}}</dd>
    </ng>
  </dl>
</template>
IgorMinar commented 11 years ago

if anyone wants to craft a PR for this solution, it will get merged. Also if you have a preference for a particular syntax please speak out.

lgalfaso commented 11 years ago

Thanks for the input. From my point of view, syntax A is not clear for nested loops like the following

<dl>
  <dt ng-repeat="...">loop 1</dt>
  <dt ng-repeat="..." ng-repeat-next>loop 2</dt>
  <dd ng-repeat-next>is this within loop 2 or only loop 1</dd>
</dl>

Syntax B and C are about the same, I like both alternatives (even when I think syntax B is somehow better)

Unless I am missing something, syntax D has the same nested loops issues as syntax A

hsdk123 commented 11 years ago

I'd go for B if possible, purely due to having to write the least amount of letters. It also looks most symmetrical - I feel it would be easiest to manage due to the visual symmetry.

zilles commented 11 years ago

lgalfasos' comment points out an interesting dilemma. What if the nested loop in his example is on the first element instead of the second. Then all versions of the Syntax are now unclear, unless I'm missing something.

hsdk123 commented 11 years ago

I don't quite understand the difference between @lgalfaso 's proposed ng-repeat-inner and the ng repeat in Syntax X, albeit the <template> wrapping. Could someone explain?

es128 commented 11 years ago

I'm not sure I understand it either, but I'm guessing the <ng> tag does not become part of the DOM, thus avoiding an illegal nesting structure under dl.

As for the proposed syntaxes, I'd vote for B.

bowsersenior commented 11 years ago

I believe the syntax X case allows for a generic dummy wrapper element <ng> which is not part of the DOM and can host various directives. This is more generic than the ng-repeat-inner, which only applies to a specific use case for ng-repeat.

hsdk123 commented 11 years ago

If syntax X, the long term aim, is just a more generic version of ng-repeat-inner, I feel that ng-repeat-inner should also be (*not solely) adopted along one of the proposed A,B,C,D for the following:

  1. both syntax X and ng-repeat-inner have the same dom structure : if syntax X is the long term aim, it would be nice to have something now that aligns structure-wise to it similarly to prevent future refactoring.
  2. in most cases ng-repeat-inner would suffice anyway : the intent is to create an outer container to repeat inner elements.
es128 commented 11 years ago

@daegon123 the difference between any of @IgorMinar's listed syntaxes (including X) and ng-repeat-inner is that the syntaxes let you arbitrarily repeat a set of elements without a distinct container. For example, repeat a set of <tr>s while there are other non-repeating <tr>s in the table or tbody. In that case, the elements to repeat don't have a distinct parent to bind ng-repeat-inner to.

<table>
    <thead>...</thead>
    <tr ng-repeat-start="(name, definition) in myList">...</tr>
    <tr>...</tr>
    <tr ng-repeat-end>...</tr>
    <tr>...</tr>
    <tfoot>...</tfoot>
</table>
es128 commented 11 years ago

Or multiple definitions per object in a <dl>

<dl>
    <dt ng-repeat-start="...">{{item.term1}}</dt>
    <dd>{{item.def1}}</dd>
    <dt>{{item.term2}}</dt>
    <dd>{{item.def2}}</dd>
    <dt>{{item.term3}}</dt>
    <dd ng-repeat-end>{{item.def3}}</dd>
</dl>
hsdk123 commented 11 years ago

@es128 Thanks for the comments and code. That's exactly the reason why I proposed that ng-repeat-inner be accepted 'along' one of the other listed syntaxes, but seeing actual code in your comments has started to make me wonder if ng-repeat-inner is even needed. I'll post another comment if I come across something - thanks again!

IgorMinar commented 11 years ago

The problem with ng-repeat-inner is that it can't be used for in all the cases, for example:

<ul>
  <li>some static prefix item</li>
  <!-- repeat these nodes -->
    <li>{{ something }}</li>
    <li>{{ something else }}</li>
  <!-- repeat end -->
  <li>some static postfix item</li>
</ul>

because you can't put an artificial element (like div) into ul (per html spec), ng-repeat-inner can't be used here.

btw with template element, we will be able to put an artificial element in between ul and li.

Also don't get too attached to the syntax X, it was just a quick and dirty example of what we'll be eventually able to do the exact syntax is yet to be discussed, but while I'm at it, here is another one:

Syntax Y:

<ul>
  <li>some static prefix item</li>
  <template repeat="item in items">
    <li>{{ item.name }}</li>
    <li>{{ item.owner }}</li>
  </template>
  <li>some static postfix item</li>
</ul>
mlarcher commented 11 years ago

What about setting an id to the ng-repeat group ? Then we could freely attach items to it from any level. Something like

<dl>
    <dt ng-repeat="item in items" ng-repeat-group="someId">{{item.term}}</dt>
    <dd ng-repeat-with="someId">{{item.definition}}</dd>
</dl>

could allow the flexibility we need.

lgalfaso commented 11 years ago

Proposed implementation of syntax C, turns out that I think this is the syntax that would allow this same mechanism to be easy for other elements and directives

lrlopez commented 11 years ago

Nice work Lucas! I've spending the evening trying to implement syntax C. It works right, but nesting is not supported due to transclusion turning all ng-repeats into comments.

If I can not transverse the DOM looking for ng-repeat-start, I am not able to calculate the depth of ng-repeat-end attribute for matching. You're approach seems to be solve the issue...

lrlopez commented 11 years ago

Hmmm... something seems to be wrong. Docs are missing in your commit, so I may be I'm not using the right syntax.

Have a look into this: http://plnkr.co/edit/sVilxKaNrhNM4JrkuQ5r?p=preview

I expected the output for the non-nested case to be:

Term: elastic
     Static term
     This definition should be repeated for every term: 'elastic' term
Term: par
     Static term
     This definition should be repeated for every term: 'par' term

And for the nested one:

Term: elastic
     Static term
Subterm: superelastic
     Static subterm
     This should be repeater for every subterm: 'super' subterm from 'elastic' term
Subterm: subelastic
     Static subterm
     This should be repeater for every subterm: 'sub' subterm from 'elastic' term
     This definition should be repeated for every term: 'elastic' term
Term: par
     Static term
Subterm: superpar
     Static subterm
     This should be repeater for every subterm: 'super' subterm from 'par' term
Subterm: subpar
     Static subterm
     This should be repeater for every subterm: 'sub' subterm from 'par' term
     This definition should be repeated for every term: 'par' term

I'm I missing something?

lgalfaso commented 11 years ago

@lrlopez The example you put is not using the latest version of the code. Also, there is an extra </dl> at line 23

lrlopez commented 11 years ago

Thanks for the fast reply! You're right, there was an extra </dl> at line 23. I also updated the library. Nevertheless, I still can't get to work the nested example...

lgalfaso commented 11 years ago

Ok, it looks like the is a real issue with multi-element directives that are at the same level at the DOM. Updated the PR with a fix for this issue

ghost commented 11 years ago

Fixes such as ng-repeat-inner or ng-repeat-next only solve one of countless situations where binding behaviors to actual DOM nodes will limit you.

Instead of patching every directive individually you can solve this for the entire framework by adding support for virtual nodes which don't render in the actual tree.

The idea of using comments is similar, but it results in unsightly, hard to read code and you still need a way to name that node and close it somehow. Instead of inventing a new comment node syntax, use existing HTML syntax.

Think about it as a concept similar to, say, SASS silent classes:

<div id="myComplexList">
    <ng-virtual ng-repeat="...">
        <a></a>
        <b></b>
        <c></c>
    </ng-virtual>
</div>

Result:

<div id="myComplexList">
    <a></a>
    <b></b>
    <c></c>
    <a></a>
    <b></b>
    <c></c>
    <a></a>
    <b></b>
    <c></c>
    ...
</div>