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

feat(ngAttr): implement conditional/interpolated attributes directive #4269

Closed
wants to merge 1 commit into from

Conversation

caitp
Copy link
Contributor

@caitp caitp commented Oct 3, 2013

Based on ngClass, this directive allows for attributes to be conditionally
applied to an element. It also supports attributes with interpolated values.

  1. Simple case: ng-attr="{checked: isChecked}"
    • The checked attribute is added (with no value) when isChecked is
      truthy,
      and removed when isChecked is falsy.
  2. With values: ng-attr="'data-whatever={{value}}'"
    • data-whatever has the value of value from scope
  3. Set of attributes with interpolated values: ng-attr="['attr1={{a}}', 'attr2={{b()}}']"
    • Space separation isn't quite enough, but this still allows for some
      interesting stuff.
  4. Conditional attributes with values: ng-attr="{'data-whatever=shmee': showWhatever&&!flurp()}"
    • I'm sure someone has a reason for doing this, I'm not sure what it is. But it's out there.

Someone asked about a way to do this the other night in #angularjs, so I wrote
this directive just for fun. I thought I'd submit it just in case anyone felt
it would be useful in any way.

If it feels useful, I'm happy to write up some good documentation for it --
However, I'm not totally sure what the real use cases for this are.

@mary-poppins
Copy link

Thanks for the PR!

  • Contributor signed CLA now or in the past
    • If you just signed, leave a comment here with your real name
  • PR's commit messages follow the commit message format

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

@btford
Copy link
Contributor

btford commented Oct 3, 2013

Thanks @caitp!

We'll definitely look at it, but you could change the prefix from ng- to something else (to avoid collisions if it does land in angular core) and publish it yourself on Bower. From a quick glance it doesn't seem like this relies on any particular private API that would require this to be merged into angular.js to work.

I did a writeup on Angular+Bower here, if you're interested: http://briantford.com/blog/angular-bower.html

@caitp
Copy link
Contributor Author

caitp commented Oct 3, 2013

I don't know how common a use-case it really is, but if it doesn't wind up in core it would probably be a good blag post to inform people about how they could do it, I guess. Anyway it seems to be passing on Travis apart from FF failing to start on SauceLabs, so I'll leave it there.

If people decide it's useful, ping me and I'll write up some docs for it.

@btford
Copy link
Contributor

btford commented Oct 4, 2013

Yeah, ignore Travis. I think we'll come back to this after 1.2.0 is out.

@JDvorak
Copy link

JDvorak commented Oct 17, 2013

This does not support data- prefix.

@caitp
Copy link
Contributor Author

caitp commented Oct 17, 2013

@JDvorak, as far as I'm aware, there's nothing preventing an ng-attr from working like so:

<div ng-attr="{'data-toggle=collapse': mobile, 'data-target=#somewidget': mobile}">
  ...
</div>

The attribute toggling doesn't depend on the attribute normalization engine, only the ng-attr attribute is concerned about that at all.

However I haven't looked at this in like 2 weeks, so it's quite possible that I am mistaken, and would love to see a demonstration that I'm wrong, if in fact I am!

@JDvorak
Copy link

JDvorak commented Oct 17, 2013

But you can't do data-ng-attr. It forces your html to be full of nonstandard attributes.

@caitp
Copy link
Contributor Author

caitp commented Oct 17, 2013

I don't believe there is anything preventing you from doing data-ng-attr or ng:attr or x-ng-attr. These are all handled by the attribute normalization process. Can you show that this doesn't work? Because if it doesn't, that sounds more like a sort of serious bug in RC2/3, rather than an issue with this patch.

@caitp
Copy link
Contributor Author

caitp commented Oct 17, 2013

#4478 appears to be, from my tests, not correct --- or an invalid bug report.

However, if there is in fact a problem with this, it is probably something browser-specific and hard to debug without more information about how to reproduce the issue.

@IgorMinar
Copy link
Contributor

I'm sorry, but I wasn't able to verify your CLA signature. CLA signature is required for any code contributions to AngularJS.

Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match.

If you signed the CLA as a corporation, please let me know the company's name.

Thanks a bunch!

PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR.
PS2: If you are a Googler, please sign the CLA as well to simplify the CLA verification process.

@IgorMinar
Copy link
Contributor

CLA signature verified! Thank you!

Someone from the team will now triage your PR and it will be processed based on the determined priority (doc updates and fixes with tests are prioritized over other changes).

@caitp caitp mentioned this pull request Dec 19, 2013
@lord2800
Copy link

+1

The ngAttr directive allows for attributes to be conditionally applied to an
element. It also supports attributes with interpolated values.

1. Simple case: `ng-attr="{checked: isChecked}"`

2. With values: `ng-attr="'data-whatever={{value}}'"`

3. Set of attributes with interpolated values:
   `ng-attr="['attr1={{a}}', 'attr2={{b()}}']"`

4. Conditional attributes with values:
   `ng-attr="{'data-whatever=shmee': showWhatever&&!flurp()}"`
@mrzmyr
Copy link

mrzmyr commented Jan 23, 2014

+1

1 similar comment
@xiehan
Copy link

xiehan commented Jan 28, 2014

+1

@wmertens
Copy link

Use case: providing case-sensitive attributes to svg elements. 👍 (although I didn't check if this PR actually does that 😅)

@tmaximini
Copy link

+1
it would be very helpful to add custom data-attrs depending on $index in an ng-repeat block for example. I also like the consistency with ng-class.

@soonlive
Copy link

+1

2 similar comments
@mikehayesuk
Copy link

+1

@quantizor
Copy link

+1

@tkrotoff
Copy link

tkrotoff commented Mar 6, 2014

@caitp I'm using your patch to achieve this without success:

<button ng-attr="{'ng-click=alert()': true}">Call alert()</button>
app.controller('AppCtrl', function($scope) {
  $scope.alert = function() {
    alert('Click!');
  };
});

Demo: http://plnkr.co/edit/9ZRWmf?p=preview

Do you see why it does not work? (ng-click="alert()" is of course appended to the DOM element).
It's like AngularJS is not "watching" element attributes.

@quantizor
Copy link

@tkrotoff It's correctly adding the attribute, the event listeners and watchers just aren't being bound interactively.

@caitp
Copy link
Contributor Author

caitp commented Mar 6, 2014

@tkrotoff: This patch does not compile directives which are added dynamically, or "un-compile" them when removed, that's a different problem entirely. It's actually very hard to implement that correctly because we don't have a way of implementing destructors in JS, as you would in a language like C++. Regardless, such mechanics have actually been suggested (by me), but it's not clear whether they will/can ever be implemented.

@tkrotoff
Copy link

tkrotoff commented Mar 6, 2014

Thanks, it's pretty clear now.

@kwpeters
Copy link

+1

@IgorMinar
Copy link
Contributor

Please publish this as non-core component in bower/npm. Meta-programing is awesome, but I think this takes it a step too far for a core component.

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

Successfully merging this pull request may close these issues.