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

Swap input type number with inputmode numeric - Text and Date input #666

Merged
merged 6 commits into from
Dec 11, 2020

Conversation

davidhunter08
Copy link
Contributor

@davidhunter08 davidhunter08 commented Dec 9, 2020

Description

Add inputmode and spellcheck options to Text input component Nunjucks macro.

Change type="number" to inputmode="numeric" on the Date input component.

Related issues

Checklist

@davidhunter08 davidhunter08 changed the title Add inputmode and spellcheck to input component nunjucks macro Add inputmode and spellcheck to input component nunjucks macro Dec 10, 2020
@davidhunter08 davidhunter08 changed the title Add inputmode and spellcheck to input component nunjucks macro Add inputmode and spellcheck options to input component Nunjucks macro Dec 10, 2020
@davidhunter08 davidhunter08 changed the title Add inputmode and spellcheck options to input component Nunjucks macro Swap input type number with inputmode numeric - Text and Date input Dec 11, 2020
@davidhunter08 davidhunter08 added the accessibility Accessibility related label Dec 11, 2020
Copy link
Collaborator

@mcheung-nhs mcheung-nhs left a comment

Choose a reason for hiding this comment

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

Need to update the README for date-input for the HTML markup to change the type="number"
https://github.com/nhsuk/nhsuk-frontend/blob/input-component-macro/packages/components/date-input/README.md

@davidhunter08
Copy link
Contributor Author

Need to update the README for date-input for the HTML markup to change the type="number"
https://github.com/nhsuk/nhsuk-frontend/blob/input-component-macro/packages/components/date-input/README.md

Thanks @mcheung-nhs. I've made those changes.

@mcheung-nhs
Copy link
Collaborator

Hi @davidhunter08 - can see the requested changes - thanks! Do we also need to add the type="text" in the HTML as that is what is output, from the macro
type="{{ params.type | default('text') }}"

@davidhunter08
Copy link
Contributor Author

Hi @davidhunter08 - can see the requested changes - thanks! Do we also need to add the type="text" in the HTML as that is what is output, from the macro
type="{{ params.type | default('text') }}"

Yep, you're right. I've made the changes.

@kevinkuszyk
Copy link

@davidhunter08 I may be a little late to the party here, but what's the rationale for using type="text" instead of type="number"?

Not only does it feel semantically incorrect, but you also lose some browser features (up / down arrow support on the keyboard and spinners in the UI).

We're catching up with these changes in the react components☝️, and before we apply I like to understand the context a little better.

Thanks!

/cc @KaiSpencer @Tomdango @frobernik

@mcheung-nhs
Copy link
Collaborator

Hi @kevinkuszyk, we were following guidance and research done by gov.uk:
https://technology.blog.gov.uk/2020/02/24/why-the-gov-uk-design-system-team-changed-the-input-type-for-numbers/

@kevinkuszyk
Copy link

Thanks @mcheung-nhs - that's exactly what I was looking for.

jacksonhyde pushed a commit to jacksonhyde/gds-react-components that referenced this pull request Oct 18, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants