Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Append method of jqLite append only for the last target element #11446

Closed
mjeanroy opened this issue Mar 27, 2015 · 10 comments
Closed

Append method of jqLite append only for the last target element #11446

mjeanroy opened this issue Mar 27, 2015 · 10 comments

Comments

@mjeanroy
Copy link

Hi,

With jQuery, it is specified that append function should append "cloned copies of the inserted element for each target except for the last one".

But with jqLite, the same node is appended for each target, it means that the node element will be inserted only for the last target element.

Here is a very simple jsFiddle to reproduce (it uses angularjs 1.2.1 but I reproduced with angular.js 1.3.14) : https://jsfiddle.net/5o5h5hzf/

I agree, this could be easily fixed using native dom api, so I wonder if it's a known bug or if it's the expected behavior. If it's a bug, I can submit a PR if you want, otherwise it should probably be documented ?

@Narretz
Copy link
Contributor

Narretz commented Apr 2, 2015

So jqlite does not clone the element that is inserted if there are multiple targets, but always appends the same element. However, only the last target should get the original.
I think we should document this. jqlite was never meant to mirror the implemented functions 1:1. Since angular does not allow querying for elements, a case where a jqLite object consists of multiple objects is basically non-existent in the code base. I haven't looked into it, but it's possible that the jqLite "constructor" only takes the last element, which would explain the behavior.

@gkalpak
Copy link
Member

gkalpak commented Apr 2, 2015

What do you mean by "since angular does not allow querying for elements" ?

@gkalpak
Copy link
Member

gkalpak commented Apr 2, 2015

BTW, JQLite keeps all elements (not just the last one). And indeed appending to a collection of elements, will call appendChild on each element in the collection (meaning the appended element(s) will end up in the last collection element only).

Besides documenting this, we could even optimize it :)

@mjeanroy
Copy link
Author

mjeanroy commented Apr 3, 2015

@gkalpak That's exactly the problem, your description is much more useful than mine :)

@Narretz I agree, Angular does not allow to query for element, but it allows anyone to wrap array of nodes into a jqLite object.

I understand that it does not need to be fixed, but what do you think of this code:

    angular.element(document.getElementsByTagName('li'))
      .addClass('foo')
      .attr('foo', 'bar')
      .append(document.createElement('span'))
      .children().text('foo');

Don't you find weird that everything works as expected except for 'append' method ?

I missed it in my first comment, but the problem is the same for other methods appending element (prepend, before, after and wrap).

Anyway, thank you for your feedback !

@Narretz
Copy link
Contributor

Narretz commented Apr 3, 2015

It fixing this doesn't bloat the code, I can see it getting changed. If you want to give it a try and send a PR, you're very welcome.

@gkalpak
Copy link
Member

gkalpak commented Apr 4, 2015

I am not really sure how we could properly handle that in Angular. How is it possible to adequately clone an compiled element (with directives, behaviours, parent awareness etc) ?

@Narretz
Copy link
Contributor

Narretz commented Apr 13, 2015

@gkalpak That's true, I didn't even think of that. The same problem exists with full jquery, too. Noone ever complained about this though, so I think we should just document how jqlite differs from jquery. A PR to bring the behavior in line with jquery might still be considered if it doesn't bloat the code.

@mjeanroy
Copy link
Author

Hi, I will have time to submit a PR soon (probably at the end of the week), I hope this will help you to take the best decision.

mgol added a commit to mgol/angular.js that referenced this issue Mar 28, 2018
Contrary to jQuery jqLite's append doesn't clone elements so will not work
correctly when invoked on a jqLite object containing more than one DOM node.

Refs angular#11446
@mgol
Copy link
Member

mgol commented Mar 28, 2018

I submitted a docs PR for the current behavior: #16517.

mgol added a commit to mgol/angular.js that referenced this issue Mar 29, 2018
Contrary to jQuery jqLite's append doesn't clone elements so will not work
correctly when invoked on a jqLite object containing more than one DOM node.

Refs angular#11446
mgol added a commit to mgol/angular.js that referenced this issue Mar 29, 2018
Contrary to jQuery jqLite's append doesn't clone elements so will not work
correctly when invoked on a jqLite object containing more than one DOM node.

Refs angular#11446
mgol added a commit to mgol/angular.js that referenced this issue Mar 29, 2018
Contrary to jQuery jqLite's append doesn't clone elements so will not work
correctly when invoked on a jqLite object containing more than one DOM node.

Refs angular#11446
@Narretz
Copy link
Contributor

Narretz commented Apr 3, 2018

This is documented now. Closing as wontfix

@Narretz Narretz closed this as completed Apr 3, 2018
Narretz pushed a commit that referenced this issue Apr 3, 2018
…e object

Contrary to jQuery jqLite's append doesn't clone elements so will not work
correctly when invoked on a jqLite object containing more than one DOM node.

Refs #11446
Closes #16517
mgol added a commit that referenced this issue Apr 4, 2018
…e object

Contrary to jQuery jqLite's append doesn't clone elements so will not work
correctly when invoked on a jqLite object containing more than one DOM node.

Refs #11446
Closes #16517
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants