Remove error-summary dependence on document.onload

[alex-ju]

The error-summary component is being initialised on an element using a data- attribute, which means that at the initialisation moment the element already exists is loaded in the DOM. Hence, we don't need to wait for document.onload to happen in order to set the focus on this element.

Background: We're using the error-summary component in a modal dialogue and we're planning to re-initialise the GOVUK Frontend components within the modal using GOVUKFrontend.initAll() (yeah, we need to scope it, but I'll raise a separate PR for that).
This component and Tabs are the only one relying on document.onload to happen.

[NickColley]

Regarding initialising GOV.UK Frontend twice: #1127

We also discussed a way to override this behaviour of focusing here: #1173

[NickColley]

@alex-ju what do you think about having a way to set this behaviour?

new ErrorSummary($errorSummary, { autofocus: false }).init()

or

<div data-module="error-summary" data-autofocus="false"></div>

then in your application code

// main.js
const errorSummaryInstance = new ErrorSummary($errorSummary, { autofocus: false }).init()

window.onload = () => {
  errorSummaryInstance.focus()
}

const modalErrorSummaryInstance = new ErrorSummary($modalErrorSummary, { autofocus: false }).init()

modal.onopen = () => {
  modalErrorSummaryInstance.focus()
}

See also: #1173

[NickColley]

If you're under time pressure there's also this hack you could consider: https://github.com/alphagov/govuk-design-system/blob/master/src/javascripts/govuk-frontend.js#L26

[hannalaakso]

@alex-ju We're planning to pick up #1223 to look into a potential solution. The outcome of the the related work would then help to fix this issue.

[alex-ju]

I think I wasn't very clear in my PR message. But with this change I'm not aiming for custom initialisation of the error-message component, but I'm rather questioning the redundancy of adding an onload event listener on a component that's being initialised based on a data- attribute, hence after the document is loaded. #1238 might solve this, but it was more of a sanity check from me that the addEventListener is not required.

[NickColley]

If we removed this, the component would always steal the users' focus when added to the page which may/may not be the correct behaviour...

It feels like fixing the underlying issues with initialising components will make these components more flexible in the long term.

I'm not a fan of over engineering so if you think this work: #1223 is unnecessary let me know and we can park it.

[hannalaakso]

I think that removing the event listener as @alex-ju proposes might actually make sense. I guess as @NickColley said the question is whether anyone would add an error summary dynamically to a page and not want it to have focus. Can we think of any scenarios where that might be necessary? The behaviour when the error summary is present on page load won't change. I think being able to specify options programmatically would be a good change too but I don't think it's necessary for solving this problem (despite me proposing it earlier in this thread).

I guess the question is also whether accepting this PR would be a breaking change. My hunch is "no", as relying on the current behaviour when adding an error summary to a page dynamically seems like such an odd edge case.

[NickColley]

OK chatted with @hannalaakso and @36degrees again on this one.

We think that your proposal is not a breaking change based on how GOV.UK Frontend needs to be used.

"JavaScript in GOV.UK Frontend requires HTML to be parsed first by the browser before it is initialised."

I have tested this locally and the error summary is still focused on page load as expected.

This means that the proposal I was suggesting is not a priority so we will close it and consider it again in the future if we get a clearer need for it.

[36degrees]

Needs a changelog entry, but happy with the change 👍

[NickColley]

Happy with this once you've added a CHANGELOG, thanks Alex

[NickColley]

I will add the CHANGELOG :)

[alex-ju]

I was away for a few days. Yep I get your point @NickColley - it's related to the way we do #1223, but I was wondering if they depend one on the other. Thanks for updating the branch!