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

what happened to ng-bind-attr? #1925

Closed
lanterndev opened this issue Jan 30, 2013 · 22 comments
Closed

what happened to ng-bind-attr? #1925

lanterndev opened this issue Jan 30, 2013 · 22 comments

Comments

@lanterndev
Copy link
Contributor

I need to write something like

<svg>
  <circle ng-bind-attr="{cx: cx, cy: cy, r: r}"></circle>
</svg>

because if I instead write

<svg>
  <circle cx="{{cx}}" cy="{{cy}}" r="{{r}}"></circle>
</svg>

the browser throws errors like

Error: Invalid value for <circle> attribute cx="{{ cx }}"

because Angular has not yet replaced the values of these attributes by the time the element is parsed.

It looks like Angular used to have ng-bind-attr which would solve exactly this problem, but it was taken out in 5502 / https://github.com/angular/angular.js/blob/master/CHANGELOG.md#100rc3-barefoot-telepathy-2012-03-29 and I have been unable to find any documentation of why it was taken out and what you're supposed to use instead. Can anyone shed some light on this?

@IgorMinar
Copy link
Contributor

we took it out because it didn't seem necessary. previously the template compiler depended on ng-bind-attr directive, but after a refactoring it was not needed any more.

this is where the attr interpolation is detected currently:

addAttrInterpolateDirective(node, directives, value, nName);

if we were to reintroduce the support for ngBindAttr it should work consistently with the regular interpolation.

I wonder if that means that we'd need to special case it in the compiler rather than implementing it as a regular directive.

@wmertens
Copy link

wmertens commented Feb 1, 2013

Question: Can ng-bind-attr handle case-sensitive attributes? For example the viewBox attribute on is case sensitive.

I was trying out the workaround shown for the svg warnings in #1050 but since ng treats attributes as case-insensitive I couldn't make it work for viewBox. A shame, really, because with ng:attr-viewBox or perhaps late:viewBox you could have a generic prefix to do late attribute additions in the same syntax as normal ones.

Perhaps a "late:" prefix could be used specifically to do late binding on attributes, leaving the attribute case exactly like it is? Then ng:src could also be written as late:src for example.

@gmp26
Copy link

gmp26 commented Feb 11, 2013

I'd welcome a resolution of this issue as angular bindings to SVG attributes currently fail in Chrome. If the values are interpolated then the fixes suggested in issue #1050 also fail. The following nasty hack does solve the problem by aliasing 'svgXXX' attributes to 'XXX' similar to ngSrc etc. It assumes templates for SVG include things like

    <svg svg-width="{{outerWidth}}" svg-height="{{outerHeight}}">

The hack inserts

// hack to avoid svg attribute errors - uses svgXXX alias for XXX
if(name.slice(0,3)==='svg') {
    attr.$set(name.slice(3).toLowerCase(), value);
}

in function addAttrInterpolateDirective after

attr.$set(name, value);

at:

attr.$set(name, value);

@wmertens
Copy link

@gmp26 you shouldn't do the .toLowerCase(), otherwise you can't set the viewBox attribute which is case sensitive.

I do like the generic prefix but would prefer something like "late:".

@gmp26
Copy link

gmp26 commented Feb 11, 2013

Agreed - there may be other cases apart from SVG and 'late' would then make more sense.
So do we end up with this? I don't think we want to match the colon here because that's one of the alternate syntaxes anyway.

if(name.slice(0,4)==='late') {
    attr.$set(name.slice(4), value);
}

@wmertens
Copy link

Hmm, isn't the colon a valid syntax in all browsers? Then "late:" would be a very specific prefix that is very unlikely to be part of real attributes.

BTW, you probably meant attr.$set(name.slice(5), value);, no?

Wout.

On Feb 11, 2013, at 11:59 , Mike Pearson notifications@github.com wrote:

Agreed - there may be other cases apart from SVG and 'late' would then make more sense.
So do we end up with this? I don't think we want to match the colon here because that's one of the alternate syntaxes anyway.

if(name.slice(0,4)==='late') {
attr.$set(name.slice(4), value);
}

Reply to this email directly or view it on GitHub..

@lrlopez
Copy link
Contributor

lrlopez commented Feb 24, 2013

I've just sent a PR that implements this feature. I hope it gets reviewed soon.

lrlopez added a commit to lrlopez/angular.js that referenced this issue Feb 24, 2013
Sometimes is not desirable to use interpolation on attributes because
the user agent parses them before the interpolation takes place. I.e:

<svg>
  <circle cx="{{cx}}" cy="{{cy}}" r="{{r}}"></circle>
</svg>

The snippet throws three browser errors, one for each attribute.

For some attributes, AngularJS fixes that behaviour introducing special
attributes like ng-href or ng-src.

This commit is a more general solution that allows prefixing any
attribute with "ngAttr", "ng-attr-", "ng:attr:" or "ng_attr_"  so it will
be set only when the binding is done. The prefix is then removed.

I.e:
<svg>
  <circle ngAttrCx="{{cx}}" ng-attr-cy="{{cy}}" ng:Attr:r="{{r}}"></circle>
</svg>

Closes angular#1925
lrlopez added a commit to lrlopez/angular.js that referenced this issue Feb 25, 2013
Sometimes is not desirable to use interpolation on attributes because
the user agent parses them before the interpolation takes place. I.e:

<svg>
  <circle cx="{{cx}}" cy="{{cy}}" r="{{r}}"></circle>
</svg>

The snippet throws three browser errors, one for each attribute.

For some attributes, AngularJS fixes that behaviour introducing special
directives like ng-href or ng-src.

This commit is a more general solution that allows prefixing any
attribute with "ng-attr-", "ng:attr:" or "ng_attr_"  so it will
be set only when the binding is done. The prefix is then removed.

I.e:
<svg>
  <circle ng_attr_cx="{{cx}}" ng-attr-cy="{{cy}}" ng:Attr:r="{{r}}"></circle>
</svg>

Closes angular#1050
Closes angular#1925
@wmertens
Copy link

👍

IgorMinar pushed a commit to IgorMinar/angular.js that referenced this issue Feb 27, 2013
Sometimes is not desirable to use interpolation on attributes because
the user agent parses them before the interpolation takes place. I.e:

<svg>
  <circle cx="{{cx}}" cy="{{cy}}" r="{{r}}"></circle>
</svg>

The snippet throws three browser errors, one for each attribute.

For some attributes, AngularJS fixes that behaviour introducing special
directives like ng-href or ng-src.

This commit is a more general solution that allows prefixing any
attribute with "ng-attr-", "ng:attr:" or "ng_attr_"  so it will
be set only when the binding is done. The prefix is then removed.

I.e:
<svg>
  <circle ng_attr_cx="{{cx}}" ng-attr-cy="{{cy}}" ng:Attr:r="{{r}}"></circle>
</svg>

Closes angular#1050
Closes angular#1925
@wcandillon
Copy link

Hello,

I'm also looking to use ng-bind-attr, what should I use instead?

@pkozlowski-opensource
Copy link
Member

@wcandillon See description in cf17c6a

@wcandillon
Copy link

@pkozlowski-opensource thank you so much! that solves it. Is there any reason why this directive is using string interpolation syntax? It feel inconsistent with ng-bind.

@cm325
Copy link

cm325 commented Oct 23, 2013

this appears to be missing from the docs?

@stanleygu
Copy link

There still seems to be an issue with case sensitive attributes, which is problematic for SVG. For example ng-attr-refX={{something}} gets converted to refx="0", when it should be refX="0". Perhaps a fix can be introduced around this line, where normalization isn't called if ng-attr is used.

@lrlopez
Copy link
Contributor

lrlopez commented Jan 21, 2014

Hmmm... It may not be so easy to fix. The attribute is normalized in the Angular-way so we can trigger functionality disregarding which notation is used in its naming.

In this line we convert the attribute to lowercase. I recall the purpose of that is avoiding that ng-attr-cx would trigger a Cx attribute, which is wrong. Sadly this has the side effect you are complaining about.

As I said before, I can't think of an easy fix:

  • We could implement a ng-csattr- directive (case-sensitive attribute) that skips the lowercase call. Problem is that we should need to detect manually all the different ways of specifying the attribute (i.e. data-ng-csattr-, ng:attr:csattr-, etc.) that won't be efficient nor future-proof.
  • We could implement a ng-objattr directive, or maybe ng-attrs, that would parse an Angular expression an convert the returned object into attributes, where the key would be the (case sensitive) attribute name and the value its content. I.e. ng-attrs="{'refX': something, 'cx': anotherthing}"

I personally find the later more powerful and it may be a nice addition to Angular. I could prepare a PR this weekend if there's someone interested in this feature. Sadly, the core team is overwhelmed with myriads of PR so I wouldn't be too optimistic about it being considered...

@wmertens
Copy link

👍 for the ng-attrs object idea!

On Tue, Jan 21, 2014 at 9:16 PM, Luis Ramón López
notifications@git.luolix.topwrote:

Hmmm... It may not be so easy to fix. The attribute is normalized in the
Angular-way so we can trigger functionality disregarding which notation is
used in its naming.

In this linehttps://github.com/angular/angular.js/blob/cf17c6af475eace31cf52944afd8e10d3afcf6c0/src/ng/compile.js#L526we convert the attribute to lowercase. I recall the purpose of that is
avoiding that ng-attr-cx would trigger a Cx attribute, which is wrong.
Sadly this has the side effect you are complaining about.

As I said before, I can't think of an easy fix:

We could implement a ng-csattr- directive (case-sensitive attribute)
that skips the lowercase call. Problem is that we should need to
detect manually all the different ways of specifying the attribute (i.e.
data-ng-csattr-, ng:attr:csattr-, etc.) that won't be efficient nor
future-proof.

We could implement a ng-objattr directive, or maybe ng-attrs, that
would parse an Angular expression an convert the returned object into
attributes, where the key would be the (case sensitive) attribute name and
the value its content. I.e. ng-attrs="{'refX': something, 'cx':
anotherthing}"

I personally find the later more powerful and it may be a nice addition to
Angular. I could prepare a PR this weekend if there's someone interested in
this feature. Sadly, the core team is overwhelmed with myriads of PR so I
wouldn't be too optimistic about it being considered...


Reply to this email directly or view it on GitHubhttps://github.com//issues/1925#issuecomment-32958388
.

@lrlopez
Copy link
Contributor

lrlopez commented Jan 28, 2014

FYI, I've just found that @caitp proposed this feature in PR #4269

The syntax also allows specifying conditional attributes, which may be even more useful.

Can you test if it suits your needs? If it does, we could move the discussion there...

@stanleygu
Copy link

I haven't tested yet if PR #4269 works for this, but I came up with a hack/workaround that I used for this a while back. Posted here in case it is helpful to anyone.

.directive('caseSensitive', function() {
    return {
      link: function(scope, element, attr) {
        scope.attr = attr;
        var caseSensitive = attr.caseSensitive.split(',');
        angular.forEach(caseSensitive, function(a) {
          var lowercase = a.toLowerCase();
          scope.$watch('attr[lowercase]', function() {
            element[0].setAttribute(a, element[0].getAttribute(lowercase));
          });
        });
      }
    };
  })

Example Usage:

<marker case-sensitive="refX,refY" viewBox="0 0 10 10" markerWidth="30"
    markerHeight="30" ng-attr-refX="{{ref.x}}" ng-attr-refY="{{ref.y}}" orient="auto">
</marker>

@wmertens
Copy link

Nice solution @stanleygu ! Makes me wonder if it's not easier to have angular do the same by default for a hard-coded list of known-problematic attributes... Then it would all Just Work...

@quantizor
Copy link

Did something break recently regarding ng-attr-*? I can't seem to get it to work... http://plnkr.co/edit/J0scPYKFb2udjRPYiJYG?p=preview

Just need a sanity check.

@lrlopez
Copy link
Contributor

lrlopez commented Mar 3, 2014

Until someone looks into it, you could just use <h1 title="{{test}}">Hello Plunker!</h1>.

ng-attr-* was created for specific attributes that were parsed only once by the browser.

@quantizor
Copy link

@lrlopez Yup, for sure. It's just throwing warnings in the console that I'm doing a little fallback dance to avoid for the moment.

@mbenford
Copy link
Contributor

For what it's worth, I needed a way to set attributes without using interpolation, and came up with this very simple directive the gets the job done:

Code: https://gist.github.com/mbenford/dd7556f01963f49e5c7d
Demo: http://plnkr.co/edit/Om934mVy1iplZv1HRuST?p=preview

stanleygu added a commit to stanleygu/case-sensitive that referenced this issue Nov 5, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.