From e82970d490e86c4d73847e8c3f6745684c1ecef0 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Mon, 9 May 2022 12:38:18 +0100 Subject: [PATCH 1/2] Remove whitespace around API key in JS templates We removed whitespace in the HTML of the copy to clipboard component in https://github.com/alphagov/notifications-admin/pull/4236/files When the Javascript on the page loads it re-renders the component, using HTML which is embedded in the .js file. This means we also need to apply the same change to the .js file to remove any extraneous whitespace. --- app/assets/javascripts/copyToClipboard.js | 4 +--- tests/javascripts/copyToClipboard.test.js | 18 +++++++++++------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/app/assets/javascripts/copyToClipboard.js b/app/assets/javascripts/copyToClipboard.js index 03d5325d4..74dcc207c 100644 --- a/app/assets/javascripts/copyToClipboard.js +++ b/app/assets/javascripts/copyToClipboard.js @@ -7,9 +7,7 @@ const states = { 'valueVisible': (options) => ` - - ${options.valueLabel ? '' + options.thing + ': ' : ''}${options.value} - + ${options.valueLabel ? '' + options.thing + ': ' : ''}${options.value} ${options.onload ? '' : options.thing + ' returned to page, press button to copy to clipboard'} diff --git a/tests/javascripts/copyToClipboard.test.js b/tests/javascripts/copyToClipboard.test.js index 3757a7dfd..d2f984566 100644 --- a/tests/javascripts/copyToClipboard.test.js +++ b/tests/javascripts/copyToClipboard.test.js @@ -25,9 +25,7 @@ describe('copy to clipboard', () => { ${options.thing}
- - ${(options.name === options.thing) ? '' + options.thing + ': ' : ''}${apiKey} - + ${(options.name === options.thing) ? '' + options.thing + ': ' : ''}${apiKey}
`; @@ -152,6 +150,12 @@ describe('copy to clipboard', () => { }); + test("It should render the 'thing' without extra whitespace", () => { + + expect(component.querySelector('.copy-to-clipboard__value').textContent).toBe('00000000-0000-0000-0000-000000000000'); + + }); + }); describe("If it's one of many in the page", () => { @@ -346,11 +350,11 @@ describe('copy to clipboard', () => { test("the copied selection (range) should start after the prefix of the id", () => { - // that selection (a range) should have a startOffset past the first two nodes: - // index 0: text node containing the whitespace before the prefix - // index 1: the prefix node + // that selection (a range) should have a startOffset of 1: + // index 0: the prefix node + // index 1: the value node expect(rangeMock.setStart).toHaveBeenCalled(); - expect(rangeMock.setStart.mock.calls[0][1]).toEqual(2); + expect(rangeMock.setStart.mock.calls[0][1]).toEqual(1); // reset any methods in the global space window.queryCommandSupported = undefined; From 6f856fdece9a4907f7e9a1993f873fb01cef1810 Mon Sep 17 00:00:00 2001 From: Chris Hill-Scott Date: Tue, 10 May 2022 10:33:27 +0100 Subject: [PATCH 2/2] Make terminology around visually hidden prefix clearer and more consistent Co-authored-by: Ben Thorner --- tests/javascripts/copyToClipboard.test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/javascripts/copyToClipboard.test.js b/tests/javascripts/copyToClipboard.test.js index d2f984566..6511aa6d5 100644 --- a/tests/javascripts/copyToClipboard.test.js +++ b/tests/javascripts/copyToClipboard.test.js @@ -348,10 +348,10 @@ describe('copy to clipboard', () => { }); - test("the copied selection (range) should start after the prefix of the id", () => { + test("the copied selection (range) should start after visually hidden prefix", () => { // that selection (a range) should have a startOffset of 1: - // index 0: the prefix node + // index 0: the visually hidden prefix node, for example "Template ID: " or "API key: " // index 1: the value node expect(rangeMock.setStart).toHaveBeenCalled(); expect(rangeMock.setStart.mock.calls[0][1]).toEqual(1);