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

feat(input.email): Input of type email support multiple attribute #1866

Closed
wants to merge 3 commits into from
Closed

feat(input.email): Input of type email support multiple attribute #1866

wants to merge 3 commits into from

Conversation

sja
Copy link

@sja sja commented Jan 23, 2013

Added support for HTML5 attribute multiple to enter more than one e-mail address per input field.
The mail addresses can be seperated by comma or semicolon.

Added support for HTML5 attribute `multiple` to enter more than one e-mail address per input field.
The mail addresses can be seperated by comma or semicolon.
@petebacondarwin
Copy link
Contributor

Hi Sebastian
Thanks for this. Have you signed the CLA?
Pete

@sja
Copy link
Author

sja commented Jan 23, 2013

Yes, I did.

@petebacondarwin
Copy link
Contributor

Looks good to me. I have pushed a build to the CI server: http://ci.angularjs.org/job/angular.js-pete/58/

…t more than two addresses

The regex did not match for more than two mail addresses.
The tests have not reflected the element change.
@sja
Copy link
Author

sja commented Jan 23, 2013

Sorry, but I found that just changing the input fields value did not re-validate with the new regex in the tests. But my tests have not covered the error in regex too.

@petebacondarwin
Copy link
Contributor

Nice fix. Please keep checking it.

@sja
Copy link
Author

sja commented Jan 24, 2013

What do you mean? Should I add tests? Or just "lean back while we integrate you stuff" and "keep working on angular.js"? ;-)

@petebacondarwin
Copy link
Contributor

I can't speak for the team but I will get it looked at during the next
round of PR reviews.
Do keep fixing bugs and suggesting features!

On 24 January 2013 07:04, Sebastian Janzen notifications@github.com wrote:

What do you mean? Should I add tests? Or just "lean back while we
integrate you stuff" and "keep working on angular.js"? ;-)


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

…te on single address to true

The regex did not match for just one mail addresses.
The tests have not reflected the element change.
@pkozlowski-opensource
Copy link
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
Member

@sja there are couple of things to take care of before this PR can be merged, I've included the checklist in the comment above. For now, could you squash commits so it is easier to review and update documentation?

@sja
Copy link
Author

sja commented Feb 19, 2013

Ok, I checked everything, but on testacular I get resource should not encode @ in url params FAILED (Chrome & FF). I guess this is not my fault.
I'll rebase on master later to check if this have been fixed.

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.

3 participants