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

Allow disabling autofocus on error summary #2494

Merged
merged 2 commits into from
Jan 14, 2022
Merged

Allow disabling autofocus on error summary #2494

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ffe1e04
Allow disabling autofocus on error summary
36degrees Jan 5, 2022
246fbab
Document in CHANGELOG
36degrees Jan 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ This change was introduced in [#2491: Prevent error summary from being re-focuse

- [#2475: Tweak whitespace in input component HTML for improved readability](https://github.com/alphagov/govuk-frontend/pull/2475)
- [#2491: Prevent error summary from being re-focused after it has been initially focused on page load](https://github.com/alphagov/govuk-frontend/pull/2491)
- [#2494: Allow disabling autofocus on error summary](https://github.com/alphagov/govuk-frontend/pull/2494)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a fix or a new feature? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I put a note in the commit message – Although this is technically a new feature, there are going to very few (if any) use cases outside of the Design System – so treating it as a fix seems sufficient.

Thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah sorry, I hadn't spotted that!

Ollie and I had a chat about this in our catch-up. We discussed it being a bit of an odd case where it's technically a new feature, but we're almost only adding it to fix our situation in the Design System. It may be beneficial to services (perhaps if they're showing/hiding the error summary on the client side) but we're not sure of any use cases like that for certain.

I'm happy to approve this as it is (listed as a fix). We did discuss the possibility of listing it as a feature if there are other new features in this release, but it seems weird to highlight this in a feature release if it's the only feature we're shipping but we don't recommend using it. We're not sure what else will be included in this release yet. but we should review the release notes when we are ready to release, so we'll have an opportunity to change this then.


## 4.0.0 (Breaking release)

Expand Down
4 changes: 4 additions & 0 deletions src/govuk/components/error-summary/error-summary.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ ErrorSummary.prototype.init = function () {
ErrorSummary.prototype.setFocus = function () {
var $module = this.$module

if ($module.getAttribute('data-disable-auto-focus') === 'true') {
return
}

// Set tabindex to -1 to make the element programmatically focusable, but
// remove it on blur as the error summary doesn't need to be focused again.
$module.setAttribute('tabindex', '-1')
Expand Down
18 changes: 18 additions & 0 deletions src/govuk/components/error-summary/error-summary.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,24 @@ describe('Error Summary', () => {
expect(tabindex).toBeNull()
})

describe('when auto-focus is disabled', () => {
it('does not have a tabindex attribute', async () => {
await page.goto(`${baseUrl}/components/error-summary/autofocus-disabled/preview`, { waitUntil: 'load' })

const tabindex = await page.$eval('.govuk-error-summary', el => el.getAttribute('tabindex'))

expect(tabindex).toBeNull()
})

it('does not focus on page load', async () => {
await page.goto(`${baseUrl}/components/error-summary/autofocus-disabled/preview`, { waitUntil: 'load' })

const activeElement = await page.evaluate(() => document.activeElement.dataset.module)

expect(activeElement).not.toBe('govuk-error-summary')
})
})

const inputTypes = [
// [description, input id, selector for label or legend]
['an input', 'input', 'label[for="input"]'],
Expand Down
9 changes: 9 additions & 0 deletions src/govuk/components/error-summary/error-summary.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ params:
type: object
required: false
description: HTML attributes (for example data attributes) to add to the error link anchor.
- name: disableAutoFocus
type: boolean
required: false
description: Prevent moving focus to the error summary when the page loads.
- name: classes
type: string
required: false
Expand Down Expand Up @@ -183,3 +187,8 @@ examples:
-
text: Descriptive link to the <b>question</b> with an error
href: '#error-1'
- name: autofocus disabled
hidden: true
data:
titleText: There is a problem
disableAutoFocus: true
1 change: 1 addition & 0 deletions src/govuk/components/error-summary/template.njk
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
<div class="govuk-error-summary
{%- if params.classes %} {{ params.classes }}{% endif %}" aria-labelledby="error-summary-title" role="alert"
{%- if params.disableAutoFocus %} data-disable-auto-focus="true"{% endif %}
{%- for attribute, value in params.attributes %} {{ attribute }}="{{ value }}"{% endfor %} data-module="govuk-error-summary">
<h2 class="govuk-error-summary__title" id="error-summary-title">
{{ params.titleHtml | safe if params.titleHtml else params.titleText }}
Expand Down