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

Remove error summary override init function #2039

Merged
merged 3 commits into from
Feb 8, 2022

Conversation

vanitabarrett
Copy link
Contributor

Fixes #1666

Note: this PR uses a pre-release of GOV.UK Frontend. The first commit will need to be deleted before merging.

What

Replaces the JavaScript override of the error summary init function with the new disableAutoFocus macro option instead.

Doing this means we have to introduce code.njk files for our error summary component examples, so that users viewing the HTML and Nunjucks code examples don't see the disableAutoFocus macro option/data attribute being set and accidentally copy that into their own service.

There should be no visual/behavioural changes as a result of this PR.

  • The visual examples on the error summary page (or anywhere else they're used on the Design System website - I couldn't find any other uses) shouldn't capture focus
  • The HTML/Nunjucks code examples shouldn't show the disableAutoFocus macro option or the data-disable-auto-focus attribute

Why

When we release v4.0.1 of GOV.UK Frontend, it'll include a change to allow disableAutoFocus to be passed to the error summary component. We don't want auto focus on the design system website - if we did have it enabled then if someone visited https://design-system.service.gov.uk/components/error-summary/ they would be autofocused on the examples.

At the moment we have a line of JavaScript code that overrides the error summary init function to disable the auto focus behaviour. With v4.0.1, we can replace this and use the disableAutoFocus macro option instead.

@netlify
Copy link

netlify bot commented Jan 25, 2022

✔️ You can preview this change here:

🔨 Explore the source changes: ea7d0df

🔍 Inspect the deploy log: https://app.netlify.com/sites/govuk-design-system-preview/deploys/6202913bde80f000093e6fe1

😎 Browse the preview: https://deploy-preview-2039--govuk-design-system-preview.netlify.app

@vanitabarrett vanitabarrett requested a review from a team January 26, 2022 09:57
@36degrees
Copy link
Contributor

Oof, I hadn't twigged quite how many places we'd need to do this… that's quite a lot of 'duplicated' examples to maintain.

I feel like there must be a better way of doing this, but not sure what that is yet…

Maybe we should go with this and create an issue to try and find a better way? I still think this is a marginally better approach than monkey patching the init function.

@vanitabarrett
Copy link
Contributor Author

@36degrees Yep, I think this is still better than what we've got now. I'd be happy to go with this (once 4.0.1 is released) and write up a separate card for how we approach separate code examples generally.

@vanitabarrett
Copy link
Contributor Author

Written up an issue here #2041

Copy link
Contributor

@36degrees 36degrees left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍🏻

Might be worth converting to draft as a reminder to remove the pre-release commit?

Vanita Barrett added 3 commits February 8, 2022 15:48
Add a code.njk file so that we can provide HTML and Nunjucks code for these
examples without including the `disableAutoFocus attribute`
@vanitabarrett vanitabarrett marked this pull request as ready for review February 8, 2022 15:49
@vanitabarrett vanitabarrett merged commit c77662e into main Feb 8, 2022
@vanitabarrett vanitabarrett deleted the remove-error-summary-hack branch February 8, 2022 15:54
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.

Error Summary component init function is monkeypatched in GOV.UK Design System
2 participants