Make radios and checkboxes components easier to link to from error summary #1426

NickColley · 2019-06-04T16:45:48Z

Updates the radios, checkboxes and date input components so it's easier to link to their first items from the error summary component.

I have tried to simplify the date input component in a similar way but the complexity was the same after I changed it, if not worse. So if we want to make the date input component easier to use in the future we'll need to think about something else.

Closes #929

NickColley · 2019-06-04T16:48:08Z

app/views/full-page-examples/passport-details/index.js

@@ -8,7 +8,7 @@ module.exports = (app) => {
      body('passport-number')
        .exists()
        .not().isEmpty().withMessage('Enter your passport number'),
-      body('expiry-day')
+      body('expiry')


I'm not convinced making these changes on the date input component actually makes it any easier.

You can see that I was not able to simplify the logic in the validation section:

govuk-frontend/app/views/full-page-examples/passport-details/index.js

Lines 28 to 52 in d41467d

// If any of the date inputs error apply a general error.

const expiryNamePrefix = 'expiry'

const expiryErrors = Object.values(errors).filter(error => error.id.includes(expiryNamePrefix + '-'))

if (expiryErrors) {

const firstExpiryErrorId = expiryErrors[0].id

// Get the first error message and merge it into a single error message.

errors[expiryNamePrefix] = {

id: expiryNamePrefix,

href: '#' + firstExpiryErrorId

}

// Construct a single error message based on all three error messages.

errors[expiryNamePrefix].text = 'Enter your expiry '

if (expiryErrors.length === 3) {

errors[expiryNamePrefix].text += 'date'

} else {

errors[expiryNamePrefix].text += expiryErrors.map(error => error.text.replace('Enter your expiry ', '')).join(' and ')

}

}

let errorSummary = Object.values(errors)

if (expiryErrors) {

// Remove all other errors from the summary so we only have one message that links to the expiry input.

errorSummary = errorSummary.filter(error => !error.id.includes(expiryNamePrefix + '-'))

}

Is it worth changing the date input considering this?

Noticed that the full page example that features the date input doesnt work anymore, so maybe I will have to change this logic, will see if it makes it simpler or not, I suspect it'll be more complex than it already is...

36degrees

Thorough cleanup @NickColley, thanks!

36degrees · 2019-06-06T12:07:49Z

CHANGELOG.md

@@ -4,6 +4,35 @@

 💥 Breaking changes:

+- Make radios and checkboxes components easier to link to from the error summary component
+
+  The first input in the radios and checkboxes will have an `id` and `name` attribute that match, allowing for easier generation of error summary links.


This is only true if you haven't specified your own idPrefix, I think?

I wonder if we should be more explicit about what's actually changed here?

Perhaps something like…

The ID of the first input in a set of radios or checkboxes will no longer be suffixed with -1. This means that it will be the same as your idPrefix, or if idPrefix is not set it'll be the same as the name

We've made this change to make it easier to link from the error summary to radios or checkboxes.

36degrees · 2019-06-06T12:08:30Z

CHANGELOG.md

+
+  The first input in the radios and checkboxes will have an `id` and `name` attribute that match, allowing for easier generation of error summary links.
+
+  You should check that clicking an error link for a radios or checkboxes results in the first input being focused.


Is it clear enough what we mean by 'error link'? Would it be better to use something like:

Suggested change

You should check that clicking an error link for a radios or checkboxes results in the first input being focused.

You should check that clicking a link from the error summary to radios or checkboxes results in the first input being focused.

36degrees · 2019-06-06T12:12:16Z

CHANGELOG.md

+
+  You should check that clicking an error link for a radios or checkboxes results in the first input being focused.
+
+  To migrate you can update your error summary so it links to the first element, for example if you are using an `idPrefix` option of `question`, your error summary can now link to `#question` instead of `#question-1`.


Users might also be overriding the id of the first radio/checkbox, which I think is what we suggested in #929. In that case, they could remove the id as it won't be doing anything.

I figured since this wasnt required that I'd leave the up to people themselves rather than potentially confusing things... maybe should just have this in

I tried this and I'm worried that there's just a lot of steps being described that could confuse people so I wanted to focus on the things they definitely need to do...

There's no real harm leaving those item.ids around, it's just not necessary anymore.

nevermind I've included it after all..

CHANGELOG.md

+  If you cannot change your error summary, you could change your component to set the `item.id` option for the first input, for example:
+
+  ```javascript
+  {{ govukRadios({


36degrees · 2019-06-06T12:13:51Z

src/components/checkboxes/template.njk

+      {%- set id = item.id -%}
+    {%- else -%}
+      {%- set id = idPrefix -%}
+      {#- The first id should not have a prefix so it's easy to link to from the error summary component -#}


The number is a suffix, not a prefix 🙂

Suggested change

{#- The first id should not have a prefix so it's easy to link to from the error summary component -#}

{#- The first id should not have a number suffix so it's easy to link to from the error summary component -#}

i always get these mixed up lol :)

NickColley · 2019-06-06T13:38:09Z

src/components/checkboxes/template.njk

+      {%- set id = idPrefix -%}
+      {#- The first id should not have a prefix so it's easy to link to from the error summary component -#}
+      {%- if not loop.first -%}
+        {%- set id = id + "-" + loop.index -%}


I'll give this a go thanks for the comment

36degrees

👏

If the user has not specified an id for the first item, it will by default render without a suffix number which will make it easier to link to from the error summary as it'll match the `name` attribute.

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1426 June 4, 2019 16:46 Inactive

NickColley commented Jun 4, 2019

View reviewed changes

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1426 June 4, 2019 16:50 Inactive

NickColley force-pushed the make-components-easier-to-link-to-from-error-summary branch from 7c9706e to 36be88e Compare June 4, 2019 16:58

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1426 June 4, 2019 16:59 Inactive

NickColley force-pushed the make-components-easier-to-link-to-from-error-summary branch from 36be88e to 78691cd Compare June 5, 2019 11:08

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1426 June 5, 2019 11:08 Inactive

NickColley force-pushed the make-components-easier-to-link-to-from-error-summary branch from 78691cd to 8407a10 Compare June 5, 2019 11:23

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1426 June 5, 2019 11:23 Inactive

NickColley force-pushed the make-components-easier-to-link-to-from-error-summary branch from 8407a10 to 84f05a0 Compare June 5, 2019 11:24

NickColley marked this pull request as ready for review June 5, 2019 11:24

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1426 June 5, 2019 11:24 Inactive

NickColley added this to the v3.0.0 milestone Jun 5, 2019

NickColley added the breaking change label Jun 5, 2019

NickColley changed the title ~~Make components easier to link to from error summary~~ Make radios and checkboxes components easier to link to from error summary Jun 5, 2019

36degrees reviewed Jun 6, 2019

View reviewed changes

NickColley force-pushed the make-components-easier-to-link-to-from-error-summary branch from 84f05a0 to bb86303 Compare June 6, 2019 15:09

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1426 June 6, 2019 15:10 Inactive

NickColley force-pushed the make-components-easier-to-link-to-from-error-summary branch from bb86303 to 0b4ad6b Compare June 6, 2019 15:14

govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1426 June 6, 2019 15:14 Inactive

36degrees approved these changes Jun 6, 2019

View reviewed changes

NickColley added 3 commits June 6, 2019 17:19

Make radios easier to link to from error summary

0e82a94

If the user has not specified an id for the first item, it will by default render without a suffix number which will make it easier to link to from the error summary as it'll match the `name` attribute.

Make checkboxes easier to link to from error summary

eff7b5d

If the user has not specified an id for the first item, it will by default render without a suffix number which will make it easier to link to from the error summary as it'll match the `name` attribute.

Add CHANGELOG entry

a33f173

NickColley force-pushed the make-components-easier-to-link-to-from-error-summary branch from 0b4ad6b to a33f173 Compare June 6, 2019 16:19

govuk-design-system-ci deployed to govuk-frontend-review-pr-1426 June 6, 2019 16:20 View deployment

NickColley merged commit 0cf4bc0 into v3 Jun 6, 2019

NickColley deleted the make-components-easier-to-link-to-from-error-summary branch June 6, 2019 16:25

snyk-bot mentioned this pull request Feb 25, 2020

[Snyk] Upgrade govuk-frontend from 3.0.0 to 3.5.0 ministryofjustice/offender-categorisation#400

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Make radios and checkboxes components easier to link to from error summary #1426

Make radios and checkboxes components easier to link to from error summary #1426

NickColley commented Jun 4, 2019 •

edited

Loading

NickColley Jun 4, 2019

NickColley Jun 4, 2019

36degrees left a comment

36degrees Jun 6, 2019

36degrees Jun 6, 2019

36degrees Jun 6, 2019

36degrees Jun 6, 2019

NickColley Jun 6, 2019

NickColley Jun 6, 2019 •

edited

Loading

NickColley Jun 6, 2019

This comment was marked as resolved.

36degrees Jun 6, 2019

NickColley Jun 6, 2019

This comment was marked as resolved.

NickColley Jun 6, 2019

36degrees left a comment

	// If any of the date inputs error apply a general error.
	const expiryNamePrefix = 'expiry'
	const expiryErrors = Object.values(errors).filter(error => error.id.includes(expiryNamePrefix + '-'))
	if (expiryErrors) {
	const firstExpiryErrorId = expiryErrors[0].id
	// Get the first error message and merge it into a single error message.
	errors[expiryNamePrefix] = {
	id: expiryNamePrefix,
	href: '#' + firstExpiryErrorId
	}

	// Construct a single error message based on all three error messages.
	errors[expiryNamePrefix].text = 'Enter your expiry '
	if (expiryErrors.length === 3) {
	errors[expiryNamePrefix].text += 'date'
	} else {
	errors[expiryNamePrefix].text += expiryErrors.map(error => error.text.replace('Enter your expiry ', '')).join(' and ')
	}
	}

	let errorSummary = Object.values(errors)
	if (expiryErrors) {
	// Remove all other errors from the summary so we only have one message that links to the expiry input.
	errorSummary = errorSummary.filter(error => !error.id.includes(expiryNamePrefix + '-'))
	}


		The first input in the radios and checkboxes will have an `id` and `name` attribute that match, allowing for easier generation of error summary links.

		You should check that clicking an error link for a radios or checkboxes results in the first input being focused.

	You should check that clicking an error link for a radios or checkboxes results in the first input being focused.
	You should check that clicking a link from the error summary to radios or checkboxes results in the first input being focused.


		You should check that clicking an error link for a radios or checkboxes results in the first input being focused.

		To migrate you can update your error summary so it links to the first element, for example if you are using an `idPrefix` option of `question`, your error summary can now link to `#question` instead of `#question-1`.

	{#- The first id should not have a prefix so it's easy to link to from the error summary component -#}
	{#- The first id should not have a number suffix so it's easy to link to from the error summary component -#}

Make radios and checkboxes components easier to link to from error summary #1426

Make radios and checkboxes components easier to link to from error summary #1426

Conversation

NickColley commented Jun 4, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

36degrees left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NickColley Jun 6, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

Choose a reason for hiding this comment

36degrees left a comment

Choose a reason for hiding this comment

NickColley commented Jun 4, 2019 •

edited

Loading

NickColley Jun 6, 2019 •

edited

Loading