From 956f5d4c3ec5aeacaf20f6cbadea8e1812e1f3a9 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Tue, 15 Sep 2020 12:53:58 +0100 Subject: [PATCH] Make copy to clipboard work with prefixes The prefix was being included in the selection copied. --- app/assets/javascripts/apiKey.js | 20 +++++++++- tests/javascripts/apiKey.test.js | 39 ++++++++++++++++--- .../support/helpers/dom_interfaces.js | 2 +- 3 files changed, 52 insertions(+), 9 deletions(-) diff --git a/app/assets/javascripts/apiKey.js b/app/assets/javascripts/apiKey.js index 42e8c6cd4..cdd7aaf0e 100644 --- a/app/assets/javascripts/apiKey.js +++ b/app/assets/javascripts/apiKey.js @@ -27,11 +27,27 @@ ` }; + this.getRangeFromElement = function (keyElement) { + const range = document.createRange(); + let prefixIndex = -1; + + Array.from(keyElement.childNodes).forEach((el, idx) => { + if ((el.nodeType === 1) && el.classList.contains('govuk-visually-hidden')) { + prefixIndex = idx; + } + }); + + range.selectNodeContents(keyElement); + if (prefixIndex !== -1) { range.setStart(keyElement, prefixIndex + 1); } + + return range; + }; + this.copyKey = function(keyElement, callback) { var selection = window.getSelection ? window.getSelection() : document.selection, - range = document.createRange(); + range = this.getRangeFromElement(keyElement); + selection.removeAllRanges(); - range.selectNodeContents(keyElement); selection.addRange(range); document.execCommand('copy'); selection.removeAllRanges(); diff --git a/tests/javascripts/apiKey.test.js b/tests/javascripts/apiKey.test.js index c801bcd7e..8aa20c7f0 100644 --- a/tests/javascripts/apiKey.test.js +++ b/tests/javascripts/apiKey.test.js @@ -33,16 +33,12 @@ describe('API key', () => { }; - beforeAll(() => { + beforeEach(() => { // mock objects used to manipulate the page selection selectionMock = new helpers.SelectionMock(jest); rangeMock = new helpers.RangeMock(jest); - }); - - beforeEach(() => { - // plug gaps in JSDOM's API for manipulation of selections window.getSelection = jest.fn(() => selectionMock); document.createRange = jest.fn(() => rangeMock); @@ -325,7 +321,7 @@ describe('API key', () => { describe("If it's one of many in the page", () => { - test("the button should have a hidden suffix naming the id it is for", () => { + beforeEach(() => { // If 'thing' (what the id is) and 'name' (its specific idenifier on the page) are // different, it will be one of others called the same 'thing'. @@ -338,12 +334,31 @@ describe('API key', () => { helpers.triggerEvent(component.querySelector('button'), 'click'); + }); + + test("the button should have a hidden suffix naming the id it is for", () => { + const buttonSuffix = component.querySelector('button .govuk-visually-hidden'); expect(buttonSuffix).not.toBeNull(); expect(buttonSuffix.textContent).toEqual(' for Default'); }); + test("the copied selection shouldn't include 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 + expect(rangeMock.setStart).toHaveBeenCalled(); + expect(rangeMock.setStart.mock.calls[0][1]).toEqual(2); + + // reset any methods in the global space + window.queryCommandSupported = undefined; + window.getSelection = undefined; + document.createRange = undefined; + + }); + }); describe("If it's the only one on the page", () => { @@ -373,6 +388,18 @@ describe('API key', () => { }) + test("the copied selection should match the id", () => { + + // that selection (a range) shouldn't call setStart to avoid the prefix: + expect(rangeMock.setStart).not.toHaveBeenCalled(); + + // reset any methods in the global space + window.queryCommandSupported = undefined; + window.getSelection = undefined; + document.createRange = undefined; + + }); + }); }); diff --git a/tests/javascripts/support/helpers/dom_interfaces.js b/tests/javascripts/support/helpers/dom_interfaces.js index 903a1da42..30596a0a3 100644 --- a/tests/javascripts/support/helpers/dom_interfaces.js +++ b/tests/javascripts/support/helpers/dom_interfaces.js @@ -30,7 +30,7 @@ class DOMInterfaceMock { class RangeMock extends DOMInterfaceMock { constructor (jest) { - super(jest, { props: [], methods: ['selectNodeContents'] }); + super(jest, { props: [], methods: ['selectNodeContents', 'setStart'] }); } }