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.

Sign up for GitHub

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

Jump to bottom

LGA-1407 numbers #1013

Merged
merged 4 commits into from
Sep 23, 2020
Merged

LGA-1407 numbers #1013

merged 4 commits into from
Sep 23, 2020

Conversation

MalcolmVonMoJ
Copy link
Contributor

What does this pull request do?

Adds a pattern and inputmode=numeric to the integer entry fields for number of depentants and number of children.
Removes the same from the currency fields as per GDS guidelines.

Any other changes that would benefit highlighting?

Intentionally left blank.

Checklist

  • Provided JIRA ticket number in the title, e.g. "LGA-152: Sample title"

@MalcolmVonMoJ MalcolmVonMoJ requested a review from a team as a code owner September 23, 2020 12:39
mayowaadeniyi
mayowaadeniyi reviewed Sep 23, 2020
View reviewed changes
Copy link
Contributor

@mayowaadeniyi mayowaadeniyi left a comment

Choose a reason for hiding this comment

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

Error messages won't be uniform with these changes, but I am assuming as you said in your last commit, these are the recommended changes by GDS?

image

@MalcolmVonMoJ
Copy link
Contributor Author

Yes, that's right. The inclusion of the pattern element which brings up these HTML errors help get the correct keyboard for mobile devices. I shall look into this again to be sure.

@MalcolmVonMoJ
Copy link
Contributor Author

I've added a novalidate at the top, which should have been there anyway. So that issue is now solved.

mayowaadeniyi
mayowaadeniyi reviewed Sep 23, 2020
View reviewed changes
Copy link
Contributor

@mayowaadeniyi mayowaadeniyi left a comment

Choose a reason for hiding this comment

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

Very minor but could we change the * to a + on the controlled_by=form pattern attribute so it has to be a number? Or we could remove it entirely since it won't be validated anyway 😅

@MalcolmVonMoJ
Copy link
Contributor Author

I don't know - so no. I think we should keep it as the GDS pattern. They say:

it's used to trigger the numeric keypad on iOS < 12.2

We could test to see whether + or something else has the same effect, but I don't see the point.

mayowaadeniyi
mayowaadeniyi approved these changes Sep 23, 2020
View reviewed changes
Copy link
Contributor

@mayowaadeniyi mayowaadeniyi left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@MalcolmVonMoJ MalcolmVonMoJ merged commit 4e951af into master Sep 23, 2020
@MalcolmVonMoJ MalcolmVonMoJ deleted the feature/LGA-1407-numbers branch September 23, 2020 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mayowaadeniyi mayowaadeniyi mayowaadeniyi approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@MalcolmVonMoJ @mayowaadeniyi