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

Allow disabling autofocus on error summary #2494

Merged
merged 2 commits into from
Jan 14, 2022
Merged

Conversation

36degrees
Copy link
Contributor

Make it possible to disable the autofocus behaviour on the error summary by setting a data attribute on the module (or by setting disableAutoFocus if using the Nunjucks macro). This is consistent with the approach currently used for the notifcation banner.

The main use case for this is on the Design System, where the auto focus is problematic for the examples provided within the guidance – it causes the browser to scroll down to the first example on the page. At the minute, we are working around this by 'monkey-patching' the init method to remove the call to $module.focus.

Make it possible to disable the autofocus behaviour on the error summary by setting a data attribute on the module (or by setting disableAutoFocus if using the Nunjucks macro). This is consistent with the approach currently used for the notifcation banner.

The main use case for this is on the Design System, where the auto focus is problematic for the examples provided within the guidance – it causes the browser to scroll down to the first example on the page. At the minute, we are working around this by 'monkey-patching' the init method to remove the call to $module.focus [1].

[1]: alphagov/govuk-design-system#643
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2494 January 7, 2022 13:41 Inactive
@36degrees
Copy link
Contributor Author

This PR builds on the changes in #2491, which needs to be merged first.

@36degrees 36degrees marked this pull request as draft January 7, 2022 13:41
@lfdebrux
Copy link
Member

lfdebrux commented Jan 7, 2022

Does this need a changelog entry?

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.
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2494 January 7, 2022 13:43 Inactive
@36degrees
Copy link
Contributor Author

Give me a chance – don't know the PR number until I've opened the PR 😉

@@ -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.

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2494 January 10, 2022 17:23 Inactive
Base automatically changed from error-summary-prevent-refocusing to main January 14, 2022 10:58
@36degrees 36degrees marked this pull request as ready for review January 14, 2022 10:59
@36degrees 36degrees merged commit 974482f into main Jan 14, 2022
@36degrees 36degrees deleted the error-summary-disable-focus branch January 14, 2022 11:01
@owenatgov owenatgov mentioned this pull request Feb 8, 2022
andymantell added a commit to surevine/govuk-react-jsx that referenced this pull request Mar 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants