From 84b02ee57adee5e679cda8da36049127d7c507fe Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Fri, 26 Apr 2019 16:24:41 +0100 Subject: [PATCH 1/3] Remove reference to global 'this' from listEntry Also removes setting of the `$` variable to jQuery. This can mess up the unit tests if they run against JSDOM, which doesn't set the global `this` to `window`, as browsers do. We don't set `$` in any other module scripts and adding it to `this` without making explicit that `this` means `window` isn't useful. --- app/assets/javascripts/listEntry.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/assets/javascripts/listEntry.js b/app/assets/javascripts/listEntry.js index 07a5ffdfe..938a0a121 100644 --- a/app/assets/javascripts/listEntry.js +++ b/app/assets/javascripts/listEntry.js @@ -1,10 +1,6 @@ (function (Modules) { - 'use strict'; - var root = this, - $ = this.jQuery; - var lists = [], listEntry, ListEntry; From a6f39c08de4bba9dc327b82800b38784e813a1bf Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Fri, 26 Apr 2019 19:21:34 +0100 Subject: [PATCH 2/3] Dedupe list-entry shared attributes The `getSharedAttributes` method gathers all attributes, and [DOM tokens](https://developer.mozilla.org/en-US/docs/Web/API/Element/classList) in the `class` attribute not controlled by the module for re-applying in the `render` method. Because it gathered them for all items, many duplicates ended up being added to new items. Browser engines were tidying up the duplicates but we shouldn't be adding them in the first place. --- app/assets/javascripts/listEntry.js | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/app/assets/javascripts/listEntry.js b/app/assets/javascripts/listEntry.js index 938a0a121..b5dd7c88e 100644 --- a/app/assets/javascripts/listEntry.js +++ b/app/assets/javascripts/listEntry.js @@ -58,6 +58,7 @@ getAttributesHTML = function (attrsByElm) { var attrStr = '', elmIdx = attrsByElm.length, + existingAttributes = [], elmAttrs, attrIdx; @@ -65,7 +66,11 @@ elmAttrs = attrsByElm[elmIdx]; attrIdx = elmAttrs.length; while (attrIdx--) { - attrStr += attributeTemplate.render({ 'name': elmAttrs[attrIdx].name, 'value': elmAttrs[attrIdx].value }); + // prevent duplicates + if ($.inArray(elmAttrs[attrIdx].name, existingAttributes) === -1) { + attrStr += attributeTemplate.render({ 'name': elmAttrs[attrIdx].name, 'value': elmAttrs[attrIdx].value }); + existingAttributes.push(elmAttrs[attrIdx].name); + } } } return attrStr; From b9a2cc1ec54e4518a4d8a19c399902718d3f51e9 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Fri, 26 Apr 2019 19:29:41 +0100 Subject: [PATCH 3/3] Add tests for listEntry.js --- tests/javascripts/listEntry.test.js | 337 ++++++++++++++++++++++++++++ 1 file changed, 337 insertions(+) create mode 100644 tests/javascripts/listEntry.test.js diff --git a/tests/javascripts/listEntry.test.js b/tests/javascripts/listEntry.test.js new file mode 100644 index 000000000..80c67e3ae --- /dev/null +++ b/tests/javascripts/listEntry.test.js @@ -0,0 +1,337 @@ +beforeAll(() => { + // set up jQuery & HoganJS + window.jQuery = require('jquery'); + $ = window.jQuery; + Hogan = require('hogan.js'); + + // load module code + require('govuk_frontend_toolkit/javascripts/govuk/modules.js'); + require('../../app/assets/javascripts/listEntry.js'); +}); + +afterAll(() => { + window.jQuery = null; + $ = null; + + delete window.GOVUK; +}); + +describe("List entry", () => { + const domains = [ + 'gov.uk', + 'dwp.gov.uk', + 'hmrc.gov.uk', + 'defra.gov.uk', + 'beis.gov.uk', + 'dcms.gov.uk', + 'dfe.gov.uk', + 'did.gov.uk', + 'dvla.gov.uk', + 'dvsa.gov.uk' + ]; + let inputList; + + const triggerEvent = (el, evtType) => { + const evt = new Event(evtType, { + bubbles: true, + cancelable: true + }); + + el.dispatchEvent(evt); + }; + + const setFieldValues = (num) => { + const items = inputList.querySelectorAll('.list-entry input[type=text]'); + + while (num--) { + items[num].setAttribute('value', domains[num]); + } + }; + + beforeEach(() => { + + // set up DOM + let entries = () => { + let result = ''; + + for (let idx = 0; idx < 10; idx++) { + result += ` +
+ + +
`; + } + + return result; + }; + + document.body.innerHTML = + `
+ + + Domain names + + For example cabinet-office.gov.uk + + + +
+ ${entries()} } +
+
`; + + inputList = document.querySelector('.input-list'); + + }); + + afterEach(() => { + + document.body.innerHTML = ''; + + }); + + describe("On page load", () => { + + test("Should remove all the fields except the first 2 if no values are present", () => { + + // start module + window.GOVUK.modules.start(); + + expect(inputList.querySelectorAll('.list-entry').length).toEqual(2); + + }); + + test("Should remove all the fields except those first 2, and leave the first field as is if it has a value", () => { + + let fields = inputList.querySelectorAll('.list-entry input[type=text]'); + + // set value of first field + fields[0].setAttribute('value', domains[0]); + + // start module + window.GOVUK.modules.start(); + + // re-select fields, based on updated DOM + fields = inputList.querySelectorAll('.list-entry input[type=text]'); + + expect(fields.length).toEqual(2); + expect(fields[0].getAttribute('value')).toEqual(domains[0]); + + }); + + test("Should remove all the fields except those first 2, and leave the second field as is if it has a value", () => { + + let fields = inputList.querySelectorAll('.list-entry input[type=text]'); + + // set value of first field + fields[1].setAttribute('value', domains[1]); + + // start module + window.GOVUK.modules.start(); + + // re-select fields, based on updated DOM + fields = inputList.querySelectorAll('.list-entry input[type=text]'); + + expect(fields.length).toEqual(2); + expect(fields[1].getAttribute('value')).toEqual(domains[1]); + + }); + test("Should remove all the fields except those with values if we have 4 values", () => { + + const fourDomains = domains.slice(0, 4); + let fields = inputList.querySelectorAll('.list-entry input[type=text]'); + + // set values of first 4 fields + fourDomains.forEach((domain, idx) => { fields[idx].setAttribute('value', domain) }); + + // start module + window.GOVUK.modules.start(); + + // re-select fields, based on updated DOM + fields = inputList.querySelectorAll('.list-entry input[type=text]'); + + expect(fields.length).toEqual(4); + fourDomains.forEach((domain, idx) => { + + expect(fields[idx].getAttribute('value')).toEqual(domain); + + }); + + }); + test("Should add 'remove' buttons to all fields except the first", () => { + + // start module + window.GOVUK.modules.start(); + + inputList.querySelectorAll('.list-entry').forEach((listEntry, idx) => { + + if (idx === 0) { + expect(listEntry.querySelector('.list-entry-remove')).toBeNull(); + } else { + expect(listEntry.querySelector('.list-entry-remove')).not.toBeNull(); + } + + }); + + }); + + test("Should add an 'add feature' button to the bottom of the list", () => { + + // start module + window.GOVUK.modules.start(); + + const listItems = inputList.children; + + expect(listItems[listItems.length - 1]); + + }); + + test("Should copy all shared attributes into the new fields", () => { + + inputList.querySelectorAll('.list-entry input[type=text]').forEach((field, idx) => { + + field.setAttribute('aria-describedby', 'hint1'); + field.classList.add('top-level-domain'); + + }); + + // start module + window.GOVUK.modules.start(); + + // re-select fields, based on updated DOM + fields = inputList.querySelectorAll('.list-entry input[type=text]'); + + expect(fields[0].getAttribute('aria-describedby')).toEqual('hint1'); + expect(fields[0].classList.contains('top-level-domain')).toBe(true); + }); + }); + + describe("When the 'remove' button is clicked", () => { + + beforeEach(() => { + setFieldValues(10); + + // start module + window.GOVUK.modules.start(); + }); + + test("Should remove the associated field", () => { + + triggerEvent(inputList.querySelectorAll('.list-entry-remove')[0], 'click'); + + // list started with 10 fields + expect(inputList.querySelectorAll('.list-entry').length).toEqual(9); + + }); + + test("Should leave the list with the right numbers", () => { + + triggerEvent(inputList.querySelectorAll('.list-entry-remove')[0], 'click'); + + const newNums = Array.from( + inputList.querySelectorAll('.text-box-number-label') + ) + .map((itemNum, idx) => { + return parseInt(itemNum.lastChild.nodeValue, 10); + }); + + expect(newNums).toEqual([1,2,3,4,5,6,7,8,9]); + + }); + + test("Should leave the list with the right values if you remove the last one", () => { + + // the first list item doesn't have a 'remove' button so there are only 9 for 10 items + triggerEvent(inputList.querySelectorAll('.list-entry-remove')[8], 'click'); + + // the items have their values set to the 10 domains + const expectedValues = domains.slice(0, -1); + const itemValues = Array.from( + inputList.querySelectorAll('.list-entry input[type=text]') + ) + .map(item => item.getAttribute('value')); + + expect(itemValues).toEqual(expectedValues); + + }); + + test("Should leave the list with the right values if you remove the second one", () => { + + // the first 'remove' button is attached to the second list item + triggerEvent(inputList.querySelectorAll('.list-entry-remove')[0], 'click'); + + // the items have their values set to the 10 domains + const expectedValues = domains.slice(); + const itemValues = Array.from( + inputList.querySelectorAll('.list-entry input[type=text]') + ) + .map(item => item.getAttribute('value')); + + // remove second item + expectedValues.splice(1, 1) + + expect(itemValues).toEqual(expectedValues); + + }); + + test("Should add the 'add' button if the added question is the 10th field", () => { + + triggerEvent(inputList.querySelectorAll('.list-entry-remove')[8], 'click'); + + expect(inputList.querySelector('.list-entry-add')).not.toBeNull(); + + }); + }); + + describe("When the 'add feature' button is clicked", () => { + let addButton; + + test("Should add a new field", () => { + + // start module + window.GOVUK.modules.start(); + + triggerEvent(inputList.querySelector('.list-entry-add'), 'click'); + + // inputList defaults to 2 items + expect(inputList.querySelectorAll('.list-entry').length).toEqual(3); + + }); + test("Should update the number of fields users are allowed to enter if one is removed", () => { + + // start module + window.GOVUK.modules.start(); + + triggerEvent(inputList.querySelectorAll('.list-entry-remove')[0], 'click'); + + expect(inputList.querySelector('.list-entry-add').textContent.trim()) + .toEqual('Add another domain (9 remaining)'); + + }); + test("Should update the number of fields users are allowed to enter if one is added", () => { + + // start module + window.GOVUK.modules.start(); + + triggerEvent(inputList.querySelector('.list-entry-add'), 'click'); + + expect(inputList.querySelector('.list-entry-add').textContent.trim()) + .toEqual('Add another domain (7 remaining)'); + + }); + test("Should remove the 'add' button if the added question is the 10th field", () => { + + setFieldValues(9); + + // start module + window.GOVUK.modules.start(); + + triggerEvent(inputList.querySelector('.list-entry-add'), 'click'); + + expect(inputList.querySelector('.list-entry-add')).toBeNull(); + + }); + }); +});