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

feat($compile): Allow late binding attributes #2061

Closed
wants to merge 1 commit into from

Conversation

lrlopez
Copy link
Contributor

@lrlopez lrlopez commented 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 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}}" data-ng-attr-cy="{{cy}}" ng:Attr:r="{{r}}"></circle>
</svg>

Closes #1050
Closes #1925

@lrlopez
Copy link
Contributor Author

lrlopez commented Feb 24, 2013

The PR updates documentation and include relevant tests. I've already signed the CLA.

@lrlopez
Copy link
Contributor Author

lrlopez commented Feb 24, 2013

You can see a live-demo of the feature here: http://plnkr.co/edit/7GDN3qybyLIMsANvBIC9?p=preview

@IgorMinar
Copy link
Contributor

late prefix doesn't make it clear that this is an Angular core directive. ngAttr*?

@lrlopez
Copy link
Contributor Author

lrlopez commented Feb 24, 2013

@IgorMinar, fixed. Now it allows multiple prefixes: ng-attr-, ng:attr:, ng_attr_*, etc.

@IgorMinar
Copy link
Contributor

few minor implementation changes requested otherwise I like this PR a lot.

  • use directiveNormalize
  • ngAttr docs should be in a separate paragraph

PR Checklist (Minor Feature)

  • Contributor signed CLA now or in the past (if you just signed, leave a comment here with your real name for cross reference)
  • Feature improves existing core functionality
  • API is compatible with existing Angular apis and relevant standards (if applicable)
  • PR doesn't contain a breaking change
  • PR contains unit tests
  • PR contains e2e tests (if suitable)
  • PR contains documentation update
  • PR passes all tests on Travis (sanity)
  • PR passes all tests on ci.angularjs.org (cross-browser compatibility)
  • PR is rebased against recent master
  • PR is squashed into one commit per logical change
  • PR's commit messages are descriptive and allows us to autogenerate release notes (required commit message format)
  • All changes requested in review have been implemented

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
@lrlopez
Copy link
Contributor Author

lrlopez commented Feb 25, 2013

Done! I hope everything is ok now.

@IgorMinar
Copy link
Contributor

for some reason this change seems to cause timeouts on our CI server.. investigating...

@IgorMinar
Copy link
Contributor

landed as dc2fd96beec6700943e2bb2e0ed624decfb3a403

thanks!

@IgorMinar IgorMinar closed this Feb 27, 2013
@dparikh
Copy link

dparikh commented Mar 3, 2013

@lrlopez, the plunker link you have specified ( http://plnkr.co/edit/7GDN3qybyLIMsANvBIC9?p=preview ) does not render circle. I have tried this with rect also but nothing is rendered in the browser. console errors are gone.
following works (width & heigth attributes are not using ng-attr) :

<svg>
  <rect ng-attr-x="{{rec_.x}}" ng-attr-y="{{rec_.y}}" width="100" height="100" />
</svg>

but following does NOT work: (nothing rendered):

<svg>
  <rect ng-attr-x="{{rec_.x}}" ng-attr-y="{{rec_.y}}" ng-attr-width="{{rec_.width}}" ng-attr-height="{{rec_.height}}" /></svg>

@dparikh
Copy link

dparikh commented Mar 3, 2013

@lrlopez, In my previous post, I mentioned that circle is not rendered when used within angular to define attributes either by using ng-attr/ x-ng-attr/ ng:attr. When I debugged the generated circle element, I realized that the attribute names are InitCap and that seems the issue.
Here is the generated circle with dynamic attributes Cx, Cy, R, which should be cx, cy, r

<circle x-ng-attr-cx="{{cx}}" ng-attr-cy="{{cy}}" ng:attr:r="{{r}}" Cx="100" Cy="100" R="40"></circle>

The same issue is with rect attributes.
I tested this code with Chrome Version 25.0.1364.97 & FF Version 19.

I hope, this findings will help.

@lrlopez
Copy link
Contributor Author

lrlopez commented Mar 4, 2013

@dparikh The library used on the Plunker is based on an old version of the patch. I will try to update the link tomorrow, thanks for pointing it out

@dparikh
Copy link

dparikh commented Mar 4, 2013

@lrlopez Thanks. I saw the modified compile.js you have commited and tried the changes locally, which is working fine now.
One question though, I have been trying to achieve this from within a directive and not able to get the values replaced. All I get is empty attributes & no values (I have content JSON populated before following code).

Below is the content from console in Chrome:

<rect  x-ng-attr-width="{{content.width}}" 
          ng-attr-height="{{content.height}}" 
          x-ng-attr-x="{{content.x}}" 
          x-ng-attr-y="{{content.y}}" 
          width height x y>
</rect>

Any thoughts/pointers would be helpful.

Thanks

@lrlopez
Copy link
Contributor Author

lrlopez commented Mar 4, 2013

Hmmm... it sounds like 'content' is not on the scope. Can you show me a Plunker so I can have a look into the issue?

@dparikh
Copy link

dparikh commented Mar 5, 2013

@lrlopez Ok. I will create a plunker to reproduce my error and let you know.

@dparikh
Copy link

dparikh commented Mar 10, 2013

@lrlopez, I got it working as I wanted to at: http://plnkr.co/edit/yCh1Z7wbQKLY2BmQjnoU?p=preview

Thanks for your help..

@lrlopez
Copy link
Contributor Author

lrlopez commented Mar 11, 2013

Good to know it's solved...

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 this pull request may close these issues.

what happened to ng-bind-attr? "invalid value for attribute" errors at startup for svg elements
5 participants