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

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
patgannon wants to merge 2 commits into
angular:masterfrom
patgannon:master
Closed

patgannon wants to merge 2 commits into
angular:masterfrom
patgannon:master

Conversation

@patgannon

Copy link
Copy Markdown

No description provided.

@petebacondarwin

Copy link
Copy Markdown
Contributor

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

@pkozlowski-opensource

Copy link
Copy Markdown
Member
  • 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

Copy link
Copy Markdown
Member

@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

Copy link
Copy Markdown
Author

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

Copy link
Copy Markdown
Member

@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

Copy link
Copy Markdown

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

@devakone

Copy link
Copy Markdown

@patgannon get to the documenting :-)

@symblify

Copy link
Copy Markdown

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

Copy link
Copy Markdown
Author

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

Copy link
Copy Markdown
Contributor

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 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.

6 participants