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

Added Event Directives #2979

Closed
wants to merge 1 commit into from
Closed

Added Event Directives #2979

wants to merge 1 commit into from

Conversation

andi1984
Copy link
Contributor

I've added ng-focus and ng-blur as two further default event directives which should be available for users inside AngularJS.

@pkozlowski-opensource
Copy link
Member

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

@pkozlowski-opensource
Copy link
Member

@andi1984 Thnx for this pull request. It will require a bit more work before it can be considered for merging - I've included a checklist for you.

You can take a look at the similar PR that adds support for events to see how the documentation should be added:
f20646b

Also your commit message should be updated to follow the guidelines and mention that it closes
#1277

@matsko
Copy link
Contributor

matsko commented Jul 14, 2013

+1 :)

@vendethiel
Copy link

+1

@andi1984
Copy link
Contributor Author

@pkozlowski-opensource Thanks for the patience explaining how to commit my changes to form a proper pull request.

I've added the missing documentation (sorry for that, I should have included that directly) and tests in my last commit.

I also signed the Contributor License Agreement (CLA) a few mins ago.

@matsko
Copy link
Contributor

matsko commented Jul 14, 2013

@andi1984 please squash both commits together and use the commit message of the 2nd commit.

@andi1984
Copy link
Contributor Author

@matsko Done.

@pkozlowski-opensource
Copy link
Member

@andi1984 thnx for doing all the changes. One more thing - your last commit seems to be based on the 1-month old master and doesn't merge cleanly any more. Would you mind rebasing it on the latest master and do a forced push?

git rebase master
git push --force

@andi1984
Copy link
Contributor Author

@pkozlowski-opensource Sorry for the trouble.

Somehow I didn't get managed to rebase my old, already pushed commit (based on old master), with the newly merged master, I simply started from scratch due to the simplicity of my PR.

In future I will start branches for each single PR, keep them in sync with the master and commit them with one single commit to the master, to start a pull request. That's the way it should be done, I know. Based on git, I've to learn a lot because I'm not a git pro user ;-)

Hope my last commit fits your needs to merge it into angularjs master.

@andi1984
Copy link
Contributor Author

@pkozlowski-opensource Can you restart travis build because I couldn't. Somehow timed out...

@petebacondarwin
Copy link
Contributor

You can only restart a build by pushing new commits. Try: git commit --amend then git push -f

On 16 July 2013 23:20, Andreas Sander notifications@github.com wrote:

@pkozlowski-opensource https://github.com/pkozlowski-opensource Can you
restart travis build because I couldn't. Somehow timed out...


Reply to this email directly or view it on GitHubhttps://github.com//pull/2979#issuecomment-21078612
.

Added directives for focus and blur events.

Closes #1277
@andi1984
Copy link
Contributor Author

@petebacondarwin Thanks! I've gone through the commands right now. Hope it works as expected now.

@pkozlowski-opensource
Copy link
Member

Landed as 2bb27d4. Thank you! And hey, thnx for going through all this changes to get the commit right. Much appreciated.

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.

5 participants