From f13fe1f6c5b00a6e6a92516d49bd3d93e80410ad Mon Sep 17 00:00:00 2001 From: Hanna Laakso Date: Thu, 6 Jun 2019 11:12:39 +0100 Subject: [PATCH 1/3] Don't append hash when error summary link clicked When error summary links are clicked, this adds a hash to the URL. As reported in https://github.com/alphagov/govuk-frontend/issues/1398, if the form is submitted again with further errors, the hash will stop the error summary being focused as should happen and instead the element with the hash ID is scrolled to which is confusing for the user. This behaviour was prevented in GOV.UK Elements but we were not sure at the time why this was necessary so it wasn't reintroduced in GOV.UK Frontend. As we need to use preventDefault() to stop the page scrolling to the form element itself and to show the error message above to make it clear what happened, we programmatically reintroduced the hash to the URL to replicate browser behaviour. Following the bug report #1398, the code that adds the hash is redundant and can be removed. Co-authored-by: Oliver Byford --- src/components/error-summary/error-summary.js | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/components/error-summary/error-summary.js b/src/components/error-summary/error-summary.js index 3b0d296bc6..eedf663f0f 100644 --- a/src/components/error-summary/error-summary.js +++ b/src/components/error-summary/error-summary.js @@ -63,16 +63,6 @@ ErrorSummary.prototype.focusTarget = function ($target) { return false } - // Prefer using the history API where possible, as updating - // window.location.hash causes the viewport to jump to the input briefly - // before then scrolling to the label/legend in IE10, IE11 and Edge (as tested - // in Edge 17). - if (window.history.pushState) { - window.history.pushState(null, null, '#' + inputId) - } else { - window.location.hash = inputId - } - // Scroll the legend or label into view *before* calling focus on the input to // avoid extra scrolling in browsers that don't support `preventScroll` (which // at time of writing is most of them...) From b1d0990d3f68e425f7b74d8ce620088bff4a5ad3 Mon Sep 17 00:00:00 2001 From: Hanna Laakso Date: Thu, 6 Jun 2019 11:12:49 +0100 Subject: [PATCH 2/3] Update changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5661ccf30d..e600b41bbc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -270,6 +270,12 @@ 🔧 Fixes: +- Stop appending hash when error summary link clicked + + This prevents incorrectly focusing the form element with the hash id, instead of the error summary, when form is re-submitted with the hash in the URL and there are further errors. + + ([PR #1435](https://github.com/alphagov/govuk-frontend/pull/1435)) + - Fix settings layer being implicitly dependant on itself. ([PR #1381](https://github.com/alphagov/govuk-frontend/pull/1381)) From e43c04a363d60edfbc8cf7d3bd9da30d89b8dbf6 Mon Sep 17 00:00:00 2001 From: Hanna Laakso Date: Thu, 6 Jun 2019 12:12:20 +0100 Subject: [PATCH 3/3] Change error summary test for checking hash in URL --- src/components/error-summary/error-summary.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/components/error-summary/error-summary.test.js b/src/components/error-summary/error-summary.test.js index 73bc06f68b..07426a8cfc 100644 --- a/src/components/error-summary/error-summary.test.js +++ b/src/components/error-summary/error-summary.test.js @@ -47,9 +47,9 @@ describe('Error Summary', () => { expect(legendOrLabelOffsetFromTop).toEqual(0) }) - it('updates the hash in the URL', async () => { + it('does not include a hash in the URL', async () => { const hash = await page.evaluate(() => window.location.hash) - expect(hash).toBe(`#${inputId}`) + expect(hash).toBe('') }) }) })