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

Update ngEventDirs.js #5852

Closed
wants to merge 1 commit into from
Closed

Update ngEventDirs.js #5852

wants to merge 1 commit into from

Conversation

apolishch
Copy link
Contributor

Note priority of ng-click in documentation

@caitp
Copy link
Contributor

caitp commented Jan 17, 2014

The angular docs have a directive for a directive's priority: @priority 0 would be a more typical way to do this https://github.com/angular/angular.js/wiki/Writing-AngularJS-Documentation#angularjs-specific-ngdoc-directives

It might be good to add this for each of the ngEventDirs directives if it's going to be added to ngClick, but I believe if @priority is omitted, it's assumed that the value is 0, but this convention isn't really mentioned in any of the documents, so maybe a second opinion on this would be good.

@mary-poppins
Copy link

I'm sorry, but I wasn't able to verify your Contributor License Agreement (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 us 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.

@apolishch
Copy link
Contributor Author

I'll sign the CLA after work today, and add @priority flags to all the ngeventdirs (and squash commits).

@caitp
Copy link
Contributor

caitp commented Jan 17, 2014

You don't need to sign the CLA for docs fixes, despite what @mary-poppins says, but it will prevent the bot from bugging you about it in the future if you do, so go for it

@apolishch
Copy link
Contributor Author

CLA signed electronically. @priority added to all ng event directives

adding priority to all ng event directives

better syntax
caitp pushed a commit to caitp/angular.js that referenced this pull request Jan 20, 2014
I think that the assumption if the @priority is not defined hsould be that the priority is 0, BUT
I don't necessarily think that it's harmful to be explicit about this.

Closes angular#5852
@caitp
Copy link
Contributor

caitp commented Jan 20, 2014

I had a typo when I said @priority:, you actually shouldn't use the ':' sign.

Also, it would be good to put the @priority lines underneath the @element lines

I've amended the commit for you with these changes, but unfortunately the e2e test which makes a request to angularjs.org is failing right now, so I don't totally want to merge it just yet because of that

@caitp caitp closed this in 8b395ff Jan 20, 2014
@caitp
Copy link
Contributor

caitp commented Jan 20, 2014

Merged, thanks

jamesdaily pushed a commit to jamesdaily/angular.js that referenced this pull request Jan 27, 2014
The general assumption is that if @priority is not defined, the priority is 0. BUT it's not
necessarily harmful to be explicit about this.

Closes angular#5852
jamesdaily pushed a commit to jamesdaily/angular.js that referenced this pull request Jan 27, 2014
The general assumption is that if @priority is not defined, the priority is 0. BUT it's not
necessarily harmful to be explicit about this.

Closes angular#5852
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.

3 participants