Update input template tests to use jsdom #5017
Conversation
📋 StatsFile sizes
Modules
View stats and visualisations on the review app Action run for f751c3b |
Rendered HTML changes to npm packagediff --git a/packages/govuk-frontend/dist/govuk/components/input/template-disabled.html b/packages/govuk-frontend/dist/govuk/components/input/template-disabled.html
new file mode 100644
index 000000000..486045b79
--- /dev/null
+++ b/packages/govuk-frontend/dist/govuk/components/input/template-disabled.html
@@ -0,0 +1,6 @@
+<div class="govuk-form-group">
+ <label class="govuk-label" for="disabled-input">
+ Disabled input
+ </label>
+ <input class="govuk-input" id="disabled-input" name="" type="text" disabled>
+</div>
diff --git a/packages/govuk-frontend/dist/govuk/components/input/template-with-error-and-hint.html b/packages/govuk-frontend/dist/govuk/components/input/template-with-error-and-hint.html
new file mode 100644
index 000000000..6b14ce405
--- /dev/null
+++ b/packages/govuk-frontend/dist/govuk/components/input/template-with-error-and-hint.html
@@ -0,0 +1,12 @@
+<div class="govuk-form-group govuk-form-group--error">
+ <label class="govuk-label" for="with-error-hint">
+ National insurance number
+ </label>
+ <div id="with-error-hint-hint" class="govuk-hint">
+ It’s on your National Insurance card, benefit letter, payslip or P60. For example, ‘QQ 12 34 56 C’.
+ </div>
+ <p id="with-error-hint-error" class="govuk-error-message">
+ <span class="govuk-visually-hidden">Error:</span> Enter a National Insurance number in the correct format
+ </p>
+ <input class="govuk-input govuk-input--error" id="with-error-hint" name="with-error-hint" type="text" aria-describedby="with-error-hint-hint with-error-hint-error">
+</div>
diff --git a/packages/govuk-frontend/dist/govuk/components/input/template-with-error-message.html b/packages/govuk-frontend/dist/govuk/components/input/template-with-error-message.html
index e025b5b9c..35b100b90 100644
--- a/packages/govuk-frontend/dist/govuk/components/input/template-with-error-message.html
+++ b/packages/govuk-frontend/dist/govuk/components/input/template-with-error-message.html
@@ -2,11 +2,8 @@
<label class="govuk-label" for="input-with-error-message">
National Insurance number
</label>
- <div id="input-with-error-message-hint" class="govuk-hint">
- It’s on your National Insurance card, benefit letter, payslip or P60. For example, ‘QQ 12 34 56 C’.
- </div>
<p id="input-with-error-message-error" class="govuk-error-message">
- <span class="govuk-visually-hidden">Error:</span> Error message goes here
+ <span class="govuk-visually-hidden">Error:</span> Enter a National Insurance number in the correct format
</p>
- <input class="govuk-input govuk-input--error" id="input-with-error-message" name="test-name-3" type="text" aria-describedby="input-with-error-message-hint input-with-error-message-error">
+ <input class="govuk-input govuk-input--error" id="input-with-error-message" name="test-name-3" type="text" aria-describedby="input-with-error-message-error">
</div>
Action run for f751c3b |
Other changes to npm packagediff --git a/packages/govuk-frontend/dist/govuk/components/input/fixtures.json b/packages/govuk-frontend/dist/govuk/components/input/fixtures.json
index 5b5223d03..0563db9ac 100644
--- a/packages/govuk-frontend/dist/govuk/components/input/fixtures.json
+++ b/packages/govuk-frontend/dist/govuk/components/input/fixtures.json
@@ -38,19 +38,36 @@
"label": {
"text": "National Insurance number"
},
- "hint": {
- "text": "It’s on your National Insurance card, benefit letter, payslip or P60. For example, ‘QQ 12 34 56 C’."
- },
"id": "input-with-error-message",
"name": "test-name-3",
"errorMessage": {
- "text": "Error message goes here"
+ "text": "Enter a National Insurance number in the correct format"
}
},
"hidden": false,
"description": "",
"previewLayoutModifiers": [],
- "html": "<div class=\"govuk-form-group govuk-form-group--error\">\n <label class=\"govuk-label\" for=\"input-with-error-message\">\n National Insurance number\n </label>\n <div id=\"input-with-error-message-hint\" class=\"govuk-hint\">\n It’s on your National Insurance card, benefit letter, payslip or P60. For example, ‘QQ 12 34 56 C’.\n </div>\n <p id=\"input-with-error-message-error\" class=\"govuk-error-message\">\n <span class=\"govuk-visually-hidden\">Error:</span> Error message goes here\n </p>\n <input class=\"govuk-input govuk-input--error\" id=\"input-with-error-message\" name=\"test-name-3\" type=\"text\" aria-describedby=\"input-with-error-message-hint input-with-error-message-error\">\n</div>"
+ "html": "<div class=\"govuk-form-group govuk-form-group--error\">\n <label class=\"govuk-label\" for=\"input-with-error-message\">\n National Insurance number\n </label>\n <p id=\"input-with-error-message-error\" class=\"govuk-error-message\">\n <span class=\"govuk-visually-hidden\">Error:</span> Enter a National Insurance number in the correct format\n </p>\n <input class=\"govuk-input govuk-input--error\" id=\"input-with-error-message\" name=\"test-name-3\" type=\"text\" aria-describedby=\"input-with-error-message-error\">\n</div>"
+ },
+ {
+ "name": "with error and hint",
+ "options": {
+ "id": "with-error-hint",
+ "name": "with-error-hint",
+ "label": {
+ "text": "National insurance number"
+ },
+ "errorMessage": {
+ "text": "Enter a National Insurance number in the correct format"
+ },
+ "hint": {
+ "text": "It’s on your National Insurance card, benefit letter, payslip or P60. For example, ‘QQ 12 34 56 C’."
+ }
+ },
+ "hidden": false,
+ "description": "",
+ "previewLayoutModifiers": [],
+ "html": "<div class=\"govuk-form-group govuk-form-group--error\">\n <label class=\"govuk-label\" for=\"with-error-hint\">\n National insurance number\n </label>\n <div id=\"with-error-hint-hint\" class=\"govuk-hint\">\n It’s on your National Insurance card, benefit letter, payslip or P60. For example, ‘QQ 12 34 56 C’.\n </div>\n <p id=\"with-error-hint-error\" class=\"govuk-error-message\">\n <span class=\"govuk-visually-hidden\">Error:</span> Enter a National Insurance number in the correct format\n </p>\n <input class=\"govuk-input govuk-input--error\" id=\"with-error-hint\" name=\"with-error-hint\" type=\"text\" aria-describedby=\"with-error-hint-hint with-error-hint-error\">\n</div>"
},
{
"name": "with width-2 class",
@@ -290,6 +307,20 @@
"previewLayoutModifiers": [],
"html": "<div class=\"govuk-form-group\">\n <label class=\"govuk-label\" for=\"input-with-autocapitalize-off\">\n Autocapitalize is turned off\n </label>\n <input class=\"govuk-input\" id=\"input-with-autocapitalize-off\" name=\"autocapitalize\" type=\"text\" autocapitalize=\"none\">\n</div>"
},
+ {
+ "name": "disabled",
+ "options": {
+ "label": {
+ "text": "Disabled input"
+ },
+ "id": "disabled-input",
+ "disabled": true
+ },
+ "hidden": false,
+ "description": "",
+ "previewLayoutModifiers": [],
+ "html": "<div class=\"govuk-form-group\">\n <label class=\"govuk-label\" for=\"disabled-input\">\n Disabled input\n </label>\n <input class=\"govuk-input\" id=\"disabled-input\" name=\"\" type=\"text\" disabled>\n</div>"
+ },
{
"name": "with prefix",
"options": {
@@ -551,26 +582,6 @@
"previewLayoutModifiers": [],
"html": "<div class=\"govuk-form-group govuk-form-group--error\">\n <label class=\"govuk-label\" for=\"with-error-describedby\">\n With error describedBy\n </label>\n <p id=\"with-error-describedby-error\" class=\"govuk-error-message\">\n <span class=\"govuk-visually-hidden\">Error:</span> Error message\n </p>\n <input class=\"govuk-input govuk-input--error\" id=\"with-error-describedby\" name=\"with-error-describedby\" type=\"text\" aria-describedby=\"test-target-element with-error-describedby-error\">\n</div>"
},
- {
- "name": "with error and hint",
- "options": {
- "id": "with-error-hint",
- "name": "with-error-hint",
- "label": {
- "text": "With error and hint"
- },
- "errorMessage": {
- "text": "Error message"
- },
- "hint": {
- "text": "Hint"
- }
- },
- "hidden": true,
- "description": "",
- "previewLayoutModifiers": [],
- "html": "<div class=\"govuk-form-group govuk-form-group--error\">\n <label class=\"govuk-label\" for=\"with-error-hint\">\n With error and hint\n </label>\n <div id=\"with-error-hint-hint\" class=\"govuk-hint\">\n Hint\n </div>\n <p id=\"with-error-hint-error\" class=\"govuk-error-message\">\n <span class=\"govuk-visually-hidden\">Error:</span> Error message\n </p>\n <input class=\"govuk-input govuk-input--error\" id=\"with-error-hint\" name=\"with-error-hint\" type=\"text\" aria-describedby=\"with-error-hint-hint with-error-hint-error\">\n</div>"
- },
{
"name": "with error, hint and describedBy",
"options": {
Action run for f751c3b |
36degrees
force-pushed
the
jsdom-test-matchers
branch
from
ea3d9fc to
182b60e
Compare
| hint: | ||
| text: It’s on your National Insurance card, benefit letter, payslip or P60. For example, ‘QQ 12 34 56 C’. |
There was a problem hiding this comment.
question Thinking that hint might have been there to check where the error would be rendered relative to the hint. By removing it, and not having a test for it, we're no longer checking that it goes 'label, hint, error, field', which I guess is important from a design perspective.
There was a problem hiding this comment.
I'd be keen to add that hint back and add a test that checks the order of elements inside the govuk-form-group, what do you say?
There was a problem hiding this comment.
There's a separate example which includes both an error message and a hint further down:
With a test for the ordering of aria-describedby here:
I think it would make sense to make that example visible though, I'll update it to match this example before I removed the hint.
There was a problem hiding this comment.
I'd be keen to add that hint back and add a test that checks the order of elements inside the govuk-form-group, what do you say?
Do you mean the order they appear in the description, or in the DOM?
There was a problem hiding this comment.
I meant the order they appear in the DOM, sorry for the confusion. Thinking it's worth having that checked as it's been a design decision at some point (and a check the snapshot was doing for us), but won't die on that hill (don't think we have anything looking at the position in the DOM of the prefix and suffix, for example, which are equally tied to design decisions).
I think it would make sense to make that example visible though, I'll update it to match this example before I removed the hint.
That's a good shout 😊
There was a problem hiding this comment.
Had a look at Percy and didn't see anything beyond the default example that would test the order (visually at least).
The next commit will refactor the tests to make use of jsdom which requires us to update the filename so that we get the jsdom test environment. Doing this in two commits so that git shows the changes to the file rather than treating it as an addition and a deletion.
36degrees
force-pushed
the
jsdom-input-template-tests
branch
from
794a2cd to
e3a41af
Compare
36degrees
force-pushed
the
jsdom-input-template-tests
branch
from
e3a41af to
3be210a
Compare
There was a problem hiding this comment.
Neat, seems all test are there 🙌🏻 I'd say we can add tests for the DOM order at a later stage, I can add an issue for that.
Thinking we could update the tests like it('includes the hint') and similar tests to become tests checking the DOM order with something like expect($component.matches('.govuk-form-group .govuk-hint ~ .govuk-input')).toBe(true), a bit like what happens for checking the wrapper is here when prefix and suffix are used.
Nothing blocking for merging, but would be good if you could fix the quotes in the test for the 0 value (unless I'm mistaken) and document the reason for the addition of the paragraph hosting the description (a thumbs up on my question if my supposition was right should be enough of a trace for the future 😊 ), cheers!.
| const $component = $('.govuk-input') | ||
| expect($component.attr('pattern')).toBe('[0-9]*') | ||
| it('the input is named by the label', () => { | ||
| expect($component).toHaveAccessibleName($label.textContent.trim()) |
| hint: | ||
| text: It’s on your National Insurance card, benefit letter, payslip or P60. For example, ‘QQ 12 34 56 C’. |
There was a problem hiding this comment.
I meant the order they appear in the DOM, sorry for the confusion. Thinking it's worth having that checked as it's been a design decision at some point (and a check the snapshot was doing for us), but won't die on that hill (don't think we have anything looking at the position in the DOM of the prefix and suffix, for example, which are equally tied to design decisions).
I think it would make sense to make that example visible though, I'll update it to match this example before I removed the hint.
That's a good shout 😊
| hint: | ||
| text: It’s on your National Insurance card, benefit letter, payslip or P60. For example, ‘QQ 12 34 56 C’. |
There was a problem hiding this comment.
Had a look at Percy and didn't see anything beyond the default example that would test the order (visually at least).
packages/govuk-frontend/src/govuk/components/input/template.jsdom.test.js
Outdated
Show resolved
Hide resolved
| const $additionalDescription = document.createElement('p') | ||
| $additionalDescription.id = 'test-target-element' | ||
| $additionalDescription.textContent = 'Additional description' | ||
| document.body.appendChild($additionalDescription) |
There was a problem hiding this comment.
question Is this necessary for toHaveAccessibleDescription to function properly (finding the element with the relevant ID and computing its accessible name for use as a description, which it cannot do if the element is not in the DOM)?
There was a problem hiding this comment.
Yes, if we want to be able to use toHaveAccessibleDescription and be confident that the external element referenced by the describedBy option is included.
The alternative would be to assert against the value of the aria-describedby attribute itself (like the old tests did) but I felt it was better to have just one way of doing it?
There was a problem hiding this comment.
I agree, it's much nicer to use the 'toHaveAccessibleDescription' rather than look at the `aria-describedby. Just wanted to check I understood why that paragraph was there 😊
Also refactor the tests to: - move duplicated code (like rendering the same example of a component) into `beforeAll` blocks - use `toHaveAccessibleDescription` and `toHaveAccessibleName` where appropriate - not use snapshots
36degrees
force-pushed
the
jsdom-input-template-tests
branch
from
3be210a to
f751c3b
Compare
|
Force-pushed a change to remove the quotes around zero to resolve #5017 (comment). |
|
@36degrees Looks good to go ⛵ |
Update input template tests to use jsdom
Note
This PR is stacked on top of #5011 which must be merged first.
Also refactor the tests to:
beforeAllblockstoHaveAccessibleDescriptionandtoHaveAccessibleNamewhere appropriatePart of #5010.