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

Allow input type to be overridden; Update date input to use type="number" #568

Merged
merged 4 commits into from
Mar 5, 2018

Conversation

36degrees
Copy link
Contributor

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-568 March 1, 2018 16:32 Inactive
This matches the existing behaviour in GOV.UK Elements [1]. As documented there, the `pattern` attribute is not valid HTML for inputs where the `type` attribute is `number`, but we add it anyway to force numeric keypads on iOS devices.

[1]: https://github.com/alphagov/govuk_elements/blob/5b6dc246861dbd2e821e62d153e6eb962830e1af/app/views/examples/example_date.html
[2]: http://bradfrost.com/blog/mobile/better-numerical-inputs-for-mobile-forms
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-568 March 2, 2018 09:09 Inactive
@joelanman
Copy link
Contributor

I have the same concerns here as with the max property. With type=number, some users will enter things and nothing happens at all - there's no messaging about what's gone wrong. However in this case it does have a significant advantage of displaying a more useful keyboard to touchscreen users. Is type="number" the only way to do that?

@edwardhorsford
Copy link
Contributor

From memory, type=tel more reliably produces a numeric keypad. I'd have to check our notes.

Nothing happens if a user types a non-numeric character, but I wonder if this is actually a problem in reality - it may help users catch issues quicker - and it's standard browser behavior.

@36degrees
Copy link
Contributor Author

This is just aligning Frontend with what we already have in Elements.

Copy link

@kr8n3r kr8n3r left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📏 (aka align)

@36degrees 36degrees merged commit 647ea9c into master Mar 5, 2018
@36degrees 36degrees deleted the input-type branch March 5, 2018 09:16
@NickColley
Copy link
Contributor

This needs an addition to the changelog @36degrees

36degrees added a commit that referenced this pull request Mar 6, 2018
These represent the changes from #568 – we accidentally omitted updating the changelog as part of that PR.
@kr8n3r kr8n3r mentioned this pull request Mar 6, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants