Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(input): add support for more input attributes #343

Closed

Conversation

matthewdenobrega
Copy link
Contributor

@matthewdenobrega matthewdenobrega commented Apr 24, 2016

Notes

  • I've included all the attributes from the MDN docs on input that I thought would potentially be useful and are reasonably widely supported
  • I haven't implemented any checking of attributes against the type of the input - for example checking that max is a date when the input type is date - my feeling is this should be up to the developer
  • I've followed the pattern of minLength (which was already implemented) and used camelCase for attributes like readOnly
  • I've been quite explicit with the tests - I can remove some if this is overkill
  • The handling of the spellCheck attribute is different from the other boolean attributes - this is because it is rendered differently (as ="true" or ="false") by Angular2

@googlebot googlebot added cla: yes PR author has agreed to Google's Contributor License Agreement labels Apr 24, 2016
@matthewdenobrega
Copy link
Contributor Author

@jelbourn @hansl can someone review this please?

@hansl
Copy link
Contributor

hansl commented May 9, 2016

@jelbourn: I'm fine with this. It also becomes a good benchmark of how much bindings are a bottleneck. I needed to do this anyway for data-table.

@hansl hansl added the pr: lgtm label May 9, 2016
@jelbourn
Copy link
Member

jelbourn commented May 9, 2016

@matthewdenobrega can you rebase this PR on master?

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

1 similar comment
@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

@googlebot googlebot added cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla and removed cla: yes PR author has agreed to Google's Contributor License Agreement labels May 10, 2016
@matthewdenobrega
Copy link
Contributor Author

I've rebased on master and updated the specs so they are more inline with the other specs for input. I'm not sure what the CLA warning is - I've signed the CLA and no-one else has contributed commits.

@jelbourn
Copy link
Member

@matthewdenobrega looks like your rebase didn't go very smoothly- 124 changed files.

@matthewdenobrega
Copy link
Contributor Author

@jelbourn I don't rebase often so maybe I'm missing something, but the only differences between master and my branch are my changes:(master...matthewdenobrega:input-attributes)

If something went wrong with the rebase I can branch off master again and redo the changes.

@jelbourn
Copy link
Member

@matthewdenobrega Not sure where it went awry, but the PR has 25 commits in it now (most of which are already in master). It might be easier to create a clean branch from upstream/master and then git cherry-pick your individual commits into it, then force-push back to this PR branch.

@matthewdenobrega
Copy link
Contributor Author

New PR opened: #447

@jelbourn
Copy link
Member

Closed in favor of #447

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: no PR author must sign Google's Contributor License Agreement: https://opensource.google.com/docs/cla
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants