Templates of type SVG do not transclude elements properly

[christang]

This may be related to issue #7265, which was closed after feature release. (Issues #6778 and #5418 appear to be related, but duplicates.)

In that release, directives of type SVG and MathML became possible using replace: true. However, this currently works only with directives that do not have transclusions. For example:

    <p>SVG within transclusion (not yet fixed):</p>
    <div class="example">
      <svg height="30">
        <g-with-transclusion>
          <rect height="25" width="25" />
        </g-with-transclusion>
      </svg>
    </div>

and directive:

  .directive('gWithTransclusion', function () {
    return {
      template: '<g ng-transclude />',
      restrict: 'E',
      replace: true,
      transclude: true
    }
  })

fails to render the inner rectangle. Inspecting the elements reveal they're in the wrong namespace. The new feature hasn't been extended to transclusions yet.

Plunker:

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

Way to fix: let's add the feature to transclusions.

[caitp]

I don't think we can fix this easily. The problem is, the HTML Parser tells us we have HTMLUnknownElements, and not SVG nodes.

The only way that we can turn them into SVG nodes, is to A) wait until we have a template, B) wrap them in an element like the parent element, and C) re-compile the compiled nodes.

It's doable, but it's not very good, you know? Maybe I'll see if I can make this work, but I wouldn't really expect much. The jenga tower is pretty shaky already

[christang]

I got. It was much easier to deal with replacing the parent since you had it all in the template. I'll see if I can come up with any clever ideas and ping the thread if I do

[caitp]

@christang you might try out that patch and see if you can find things that are broken with it.

It's pretty crap and not ship-able at all right now, so it needs a lot of work to get there.

[christang]

Most of it looks good, but for one case with svg in the template, the transclude function is injecting strings of undefined rather than the expected node. I've updated the plunker here http://plnkr.co/edit/pGB1RWFKYMvUW0ImHqyE?p=preview. I figure extra eyes might help in case I missed something.

[caitp]

Because the first childNodes are textNodes, I guess. That's an easy fix

[christang]

Got it. Stripping out the whitespace makes it work perfectly

[caitp]

It's not actually that easy to fix, because we do craziness like wrap text nodes in spans, and that breaks the SVG element... Might be possible to do away with the span wrapping.

[christang]

I've noticed that in the code. Just curious. Why do we do the span wrapping
in the first place?

On Wed, May 7, 2014 at 3:57 PM, Caitlin Potter notifications@github.comwrote:

It's not actually that easy to fix, because we do craziness like wrap text
nodes in spans, and that breaks the SVG element... Might be possible to do
away with the span wrapping.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/7383#issuecomment-42475376
.

[caitp]

the span-wrapping predates my time contributing to angular, so I don't know exactly. It may have been to simplify data binding, I'm not sure --- actually, probably something about text nodes not being able to have an expando store in JQ or something

[caitp]

@christang so I brought it up with Igor, and yeah, it's kinda too much craziness to add. So you're going to have to use a real <svg> if you want to transclude SVG content.

I don't think we can land a fix for it (although people are certainly welcome to try, I'll review em).

I'm going to close this because I don't think this is really feasible in a good way.

[christang]

no worries @caitp. there's lots of new expressivity in the last few days of
commits to keep me happy for a while. appreciate your working along with
the problem, cheers!

On Wed, May 7, 2014 at 5:48 PM, Caitlin Potter notifications@github.comwrote:

@christang https://github.com/christang so I brought it up with Igor,
and yeah, it's kinda too much craziness to add. So you're going to have to
use a real if you want to transclude SVG content.

I don't think we can land a fix for it (although people are certainly
welcome to try, I'll review em).

I'm going to close this because I don't think this is really feasible in a
good way.

—
Reply to this email directly or view it on GitHubhttps://github.com//issues/7383#issuecomment-42487478
.

[merlinchen]

@christang @caitp I'm confused. commits f0e12ea introduced a type property on directive. Shouldn't the directive above written in this way?

.directive('gWithTransclusion', function () {
    return {
      template: '<g ng-transclude />',
      restrict: 'E',
      replace: true,
      transclude: true,
      type: 'svg'
    }
  })

[christang]

The issue is that if you now want to transclude into gWithTransclusion what
would have originally have been considered an svg node (e.g. circle, rect,
...), that directive itself would need to be written as a directive with
type svg.

On Mon, Aug 4, 2014 at 10:27 PM, merlinchen notifications@github.com
wrote:

@christang https://github.com/christang @caitp
https://github.com/caitp I'm confused. issue #7265
#7265 introduced a type
property on directive. Shouldn't the directive above written in this way?

.directive('gWithTransclusion', function () {
return {
template: '',
restrict: 'E',
replace: true,
transclude: true
type: 'svg'
}
})

—
Reply to this email directly or view it on GitHub
#7383 (comment).

[antoinepairet]

@caitp @christang
I came across this issue through SO (http://stackoverflow.com/questions/23305024) and I found the plunkr provided by @christang very useful. I tested the different directives against beta 18, 19 and snapshot. There are regressions between 18 and 19 (snapshot gives the same results as 19).

beta18: http://plnkr.co/edit/0nNPCIV8WiRybl6rU6Wd
beta19: http://plnkr.co/edit/IgXMeZ6vNHPLxZOs0Sbu

Summary of the 'results':
18 19
ok ok - SVG embedded in the source page - no directive
ok ok - Whole SVG embedded using directive - svgSnippet
ok KO - Fragment of SVG embedded within an svg container - svgInternal
ok KO - SVG within transclusion - gWithTransclusion
ok ok - SVG within element transclusion - withElementTransclusion
KO ok - Transclusions within SVG root - svgRootWithTransclusion

Should I open a new issue?

[caitp]

@antoinepairet the type directive property has changed to templateNamespace. If you make that change, the beta 19 version looks identical to the other version. (I was not in favour of the breaking change, but it's a relatively new feature so it hopefully doesn't bite too many people).

[antoinepairet]

👍 Perfect. Many thanks for the quick answer. That's how it goes when using a beta version. :-)

[christang]

@antoinepairet https://github.com/antoinepairet Thanks for the note. I've
updated the SO :-)

On Wed, Aug 27, 2014 at 5:14 PM, Antoine Pairet notifications@github.com
wrote:

[image: 👍] Perfect. Many thanks for the quick answer. That's how it
goes when using a beta version. :-)

—
Reply to this email directly or view it on GitHub
#7383 (comment).

[merlinchen]

@caitp @antoinepairet @christang There is a regression between 18 and 19 when using svg directive with ngRepeat.
beta 18 http://plnkr.co/edit/SqE1uIaJIrOA2dW8C6TH?p=preview
beta 19 http://plnkr.co/edit/BBmO3r5oQTY4Ki3Wo8Ly?p=preview

When click the add button, a new element will be pushed into the array. In beta 18, everything works fine. But in beta 19, the rect didn't add correctly.

[caitp]

Hmm, that is weird, it is repeating <my-rect> but not replacing it with the template. @tbosch what do you think?

[caitp]

@merlinchen could you please file a separate bug for that so we can track it correctly? it does look like a regression to me

[merlinchen]

@caitp #8808