feat(input): Allow postponing model update until input blur event by specifying ng-update-on="blur" so validation doesn't show up while user is still typing #1783

Closed
wants to merge 2 commits into
from

Projects

None yet

6 participants

@patgannon

No description provided.

@petebacondarwin
Member

I rather like this PR. Any reason why it shouldn't work?

@pkozlowski-opensource
  • 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
@pkozlowski-opensource

@patgannon Thinx for this PR. It will need a bit of work before it can be merged, though. You can see the checklist in my comment above.

@patgannon

Can you specifically comment on what needs to be done before it can get merged? Some of the un-checked checkboxes above don't seem to apply. For example, I have signed the CLA, I do think the PR improves existing core functionality, and please let me know if the PR failed ci.angularjs.org (and why/details), etc.

@pkozlowski-opensource

@patgannon Thnx for the update.
The 2 main remaining points are documentation and commit message.
We will execute tests on all browsers just before merging so don't worry about this for now.

@thedigitalman

This is a great feature, and I'm ready to start using it. So, what's the status on this?

@devakone

@patgannon get to the documenting :-)

@symblify

Just to note #2129 is similar but has some more functionality - it allows multiple events (including the Angular defaults) and also throttling of updates based on a time period.

@patgannon

Ok, sorry for the delay. I've added documentation for the ngUpdateOn directive. I attached the documentation to the textInputType() function, because I didn't see a better place to put it. Please advise if that's not the right place.

@petebacondarwin
Member

Hi @patgannon - thank you for your work on this PR but as @symblify points out there is a more complete solution offered in #2129, which I think is more likely to be merged. Do you think you could take a look at that and, using your experience of creating this PR, make comments or suggestions to get that up to a workable solution?
Thanks, Pete

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment