From 7328649537ad2d015d6e2475f080bd472038a8f1 Mon Sep 17 00:00:00 2001 From: Tom Byers Date: Thu, 9 May 2019 16:36:31 +0100 Subject: [PATCH] Combine 'Change' and 'Done' buttons into one Includes addition of classes by JS to ensure CSS selectors don't have to reference the data attributes. --- .../javascripts/collapsibleCheckboxes.js | 90 +++++++------- .../stylesheets/components/checkboxes.scss | 18 +-- .../javascripts/collapsibleCheckboxes.test.js | 115 ++++++++---------- 3 files changed, 108 insertions(+), 115 deletions(-) diff --git a/app/assets/javascripts/collapsibleCheckboxes.js b/app/assets/javascripts/collapsibleCheckboxes.js index 43689115c..f6bb41fab 100644 --- a/app/assets/javascripts/collapsibleCheckboxes.js +++ b/app/assets/javascripts/collapsibleCheckboxes.js @@ -12,34 +12,37 @@ this.$formGroup = $(component); this.$fieldset = this.$formGroup.find('fieldset'); this.$checkboxes = this.$fieldset.find('input[type=checkbox]'); - this.summary.$el = this.$formGroup.find('.selection-summary'); this.fieldLabel = this.$formGroup.data('fieldLabel'); this.total = this.$checkboxes.length; + this.legendText = this.$fieldset.find('legend').text().trim(); + this.expanded = false; - const legendText = this.$fieldset.find('legend').text().trim(); + this.addHeadingHideLegend(); - this.addHeadingHideLegend(legendText); - this.addFooterAndDoneButton(legendText); + // generate summary and footer + this.summary.$el = this.$formGroup.find('.selection-summary'); + this.footer.$el = this.footer.getEl(this); + this.footer.update(this); // create summary from component pieces and match text to current selection - this.summary.addContent(legendText, this.fieldLabel); + this.summary.addContent(this.legendText, this.fieldLabel); this.summary.update(this.getSelection(), this.total, this.fieldLabel); this.$fieldset.before(this.summary.$el); + // add custom classes + this.$formGroup.addClass('selection-wrapper'); + this.$fieldset.addClass('selection-content'); + // hide checkboxes this.$fieldset.hide(); - this.expanded = false; - - // set semantic relationships with aria attributes - this.addARIAToButtons(); this.bindEvents(); }; this.getSelection = function() { return this.$checkboxes.filter(':checked').length; }; - this.addHeadingHideLegend = function(legendText) { + this.addHeadingHideLegend = function() { const headingLevel = this.$formGroup.data('heading-level') || '2'; - this.$heading = $(`${legendText}`); + this.$heading = $(`${this.legendText}`); this.$fieldset.before(this.$heading); this.$fieldset.find('legend').addClass('visuallyhidden'); @@ -54,17 +57,11 @@ }[field] || `No ${field}s`) }, addContent: function(legendText, fieldLabel) { - const $content = $(`

- - -

`); - - this.$text = $content.find('.selection-summary__text'); - this.$changeButton = $content.find('button'); + this.$text = $(`

`); if (fieldLabel === 'folder') { this.$text.addClass('selection-summary__text--folders'); } - this.$el.append($content); + this.$el.append(this.$text); }, update: function(selection, total, field) { let template; @@ -80,34 +77,38 @@ this.$text.html(this.templates[template](selection, total, field)); } }; - this.addFooterAndDoneButton = function (legendText) { - this.$footer = $(`

- -
`); + this.footer = { + buttonContent: { + change: (fieldLabel) => `Choose ${fieldLabel}s`, + done: (fieldLabel) => `Done choosing ${fieldLabel}s` + }, + getEl: function (module) { + const buttonState = module.expanded ? 'done' : 'change'; + const buttonContent = this.buttonContent[buttonState](module.fieldLabel); - this.$doneButton = this.$footer.find('.button'); - this.$fieldset.append(this.$footer); - }; - this.addARIAToButtons = function () { - const aria = { - 'aria-expanded': this.expanded, - 'aria-controls': this.$fieldset.attr('id') - }; + return $(`
+ +
`); + }, + update: function (module) { + this.$el.remove(); + this.$el = this.getEl(module); - this.summary.$changeButton.attr(aria); - this.$doneButton.attr(aria); + module.$formGroup.append(this.$el); + } }; this.expand = function(e) { if (e !== undefined) { e.preventDefault(); } if (!this.expanded) { this.$fieldset.show(); - this.summary.$changeButton.attr('aria-expanded', true); - this.$doneButton.attr('aria-expanded', true); this.expanded = true; + this.footer.update(this); } // shift focus whether expanded or not @@ -118,22 +119,27 @@ if (this.expanded) { this.$fieldset.hide(); - this.summary.$changeButton.attr('aria-expanded', false); - this.$doneButton.attr('aria-expanded', false); this.expanded = false; + this.footer.update(this); } // shift focus whether expanded or not _focusTextElement(this.summary.$text); }; + this.handleClick = function(e) { + if (this.expanded) { + this.collapse(e); + } else { + this.expand(e); + } + }; this.handleSelection = function(e) { this.summary.update(this.getSelection(), this.total, this.fieldLabel); }; this.bindEvents = function() { const self = this; - this.summary.$changeButton.on('click', this.expand.bind(this)); - this.$doneButton.on('click', this.collapse.bind(this)); + this.$formGroup.on('click', '.button', this.handleClick.bind(this)); this.$checkboxes.on('click', this.handleSelection.bind(this)); // take summary out of tab order when focus moves diff --git a/app/assets/stylesheets/components/checkboxes.scss b/app/assets/stylesheets/components/checkboxes.scss index f2ea72826..9ec3fa9d0 100644 --- a/app/assets/stylesheets/components/checkboxes.scss +++ b/app/assets/stylesheets/components/checkboxes.scss @@ -1,8 +1,8 @@ .selection-summary { .selection-summary__text { - display: inline-block; padding: .526315em .789473em .263157em 0px; + margin-bottom: $gutter / 2; &:focus { outline: none; @@ -76,13 +76,16 @@ } +.selection-content { + margin-bottom: ($gutter / 3) * 2; + + .checkboxes-nested { + margin-bottom: 0; + } +} + .selection-footer { clear: both; - padding-top: $gutter / 6; - - @include media(tablet) { - padding-top: $gutter / 3; - } .button-secondary { // revert full-width button for smaller screens @@ -92,8 +95,7 @@ } // styles specific to the collapsible checkboxes module -[data-module=collapsible-checkboxes] { - +.selection-wrapper { fieldset:focus { outline: none; } diff --git a/tests/javascripts/collapsibleCheckboxes.test.js b/tests/javascripts/collapsibleCheckboxes.test.js index e52b4d8f8..db0636b84 100644 --- a/tests/javascripts/collapsibleCheckboxes.test.js +++ b/tests/javascripts/collapsibleCheckboxes.test.js @@ -89,6 +89,13 @@ describe('Collapsible fieldset', () => { }); + test("adds the right classes to the group and fieldset", () => { + + expect(formGroup.classList.contains('selection-wrapper')).toBe(true); + expect(fieldset.classList.contains('selection-content')).toBe(true); + + }); + test("adds a heading before the selected fieldset", () => { const heading = helpers.element(fieldset).getPreviousSibling( @@ -101,12 +108,10 @@ describe('Collapsible fieldset', () => { test("adds the right content and classes to the summary", () => { - const summary = formGroup.querySelector('.selection-summary'); + const summary = formGroup.querySelector('.selection-summary__text'); - expect(summary.querySelector('p')).not.toBeNull(); - expect(summary.querySelector('p .selection-summary__text')).not.toBeNull(); - debugger; - expect(summary.querySelector('p .selection-summary__text').classList.contains('selection-summary__text--folders')).toBe(true); + expect(summary).not.toBeNull(); + expect(summary.classList.contains('selection-summary__text--folders')).toBe(true); }); @@ -118,36 +123,18 @@ describe('Collapsible fieldset', () => { }); - test("has a change button", () => { + test("has a button to expand the fieldset", () => { - const changeButton = document.querySelector('.selection-summary .button'); + const button = formGroup.querySelector('.button'); - expect(changeButton).not.toBeNull(); - expect(changeButton.textContent).toEqual('Change Folders this team member can see'); + expect(button).not.toBeNull(); + expect(button.textContent.trim()).toEqual('Choose folders'); }); - test("has a 'Done' button", () => { + test("has the correct aria attributes on the button", () => { - const nextEl = fieldset.querySelector('.button'); - - expect(helpers.element(nextEl).nodeName).toEqual('button'); - - }); - - test("has the correct aria attributes on both buttons", () => { - - const changeButton = document.querySelector('.selection-summary .button'); - const doneButton = fieldset.querySelector('.button'); - - // check change button - expect(helpers.element(changeButton).hasAttributesSetTo({ - 'aria-controls': fieldset.getAttribute('id'), - 'aria-expanded': 'false' - })).toBe(true); - - // check done button - expect(helpers.element(doneButton).hasAttributesSetTo({ + expect(helpers.element(formGroup.querySelector('.button')).hasAttributesSetTo({ 'aria-controls': fieldset.getAttribute('id'), 'aria-expanded': 'false' })).toBe(true); @@ -170,7 +157,7 @@ describe('Collapsible fieldset', () => { const summaryText = document.querySelector('.selection-summary__text'); // default state is for none to be selected - expect(summaryText.textContent).toEqual("No folders (only templates outside a folder)"); + expect(summaryText.textContent.trim()).toEqual("No folders (only templates outside a folder)"); }); @@ -186,7 +173,7 @@ describe('Collapsible fieldset', () => { const summaryText = document.querySelector('.selection-summary__text'); - expect(summaryText.textContent).toEqual("3 of 10 folders"); + expect(summaryText.textContent.trim()).toEqual("3 of 10 folders"); }); @@ -200,7 +187,7 @@ describe('Collapsible fieldset', () => { const summaryText = document.querySelector('.selection-summary__text'); - expect(summaryText.textContent).toEqual("All folders"); + expect(summaryText.textContent.trim()).toEqual("All folders"); }); @@ -217,20 +204,14 @@ describe('Collapsible fieldset', () => { }); - describe("when 'change' is clicked", () => { - - let changeButton; - let doneButton; + describe("when button is clicked while the fieldset is collapsed", () => { beforeEach(() => { // start module window.GOVUK.modules.start(); - doneButton = fieldset.querySelector('.button'); - changeButton = document.querySelector('.selection-summary .button'); - - helpers.triggerEvent(changeButton, 'click'); + helpers.triggerEvent(formGroup.querySelector('.button'), 'click'); }); @@ -239,37 +220,39 @@ describe('Collapsible fieldset', () => { expect(helpers.element(fieldset).is('hidden')).toBe(false); }); + test("it focuses the fieldset", () => { expect(document.activeElement).toBe(fieldset); }); + test("it uses ARIA to mark the checkboxes as expanded", () => { - expect(changeButton.getAttribute('aria-expanded')).toEqual('true'); - expect(doneButton.getAttribute('aria-expanded')).toEqual('true'); + expect(formGroup.querySelector('.button').getAttribute('aria-expanded')).toEqual('true'); }); + + test("it changes it's text to indicate it's new action", () => { + + expect(formGroup.querySelector('.button').textContent.trim()).toEqual("Done choosing folders"); + + }); + }); - describe("when 'done' is clicked", () => { - - let changeButton; - let doneButton; + describe("when button is clicked when the fieldset is expanded", () => { beforeEach(() => { // start module window.GOVUK.modules.start(); - doneButton = fieldset.querySelector('.button'); - changeButton = document.querySelector('.selection-summary .button'); - // show the checkboxes - helpers.triggerEvent(changeButton, 'click'); + helpers.triggerEvent(formGroup.querySelector('.button'), 'click'); - // click the done button - helpers.triggerEvent(doneButton, 'click'); + // click the button + helpers.triggerEvent(formGroup.querySelector('.button'), 'click'); }); @@ -287,8 +270,13 @@ describe('Collapsible fieldset', () => { test("it uses ARIA to mark the checkboxes as collapsed", () => { - expect(changeButton.getAttribute('aria-expanded')).toEqual('false'); - expect(doneButton.getAttribute('aria-expanded')).toEqual('false'); + expect(formGroup.querySelector('.button').getAttribute('aria-expanded')).toEqual('false'); + + }); + + test("it changes it's text to indicate it's new action", () => { + + expect(formGroup.querySelector('.button').textContent.trim()).toEqual("Choose folders"); }); }); @@ -296,8 +284,7 @@ describe('Collapsible fieldset', () => { describe("when the selection changes", () => { const showCheckboxes = () => { - changeButton = document.querySelector('.selection-summary .button'); - helpers.triggerEvent(changeButton, 'click'); + helpers.triggerEvent(formGroup.querySelector('.button'), 'click'); }; const checkFirstCheckbox = () => { @@ -321,8 +308,6 @@ describe('Collapsible fieldset', () => { }); }; - let changeButton; - describe("from some to none the summary updates to reflect that", () => { test("if fields are called 'folders'", () => { @@ -341,7 +326,7 @@ describe('Collapsible fieldset', () => { // click the first checkbox helpers.triggerEvent(checkboxes[0], 'click'); - expect(summaryText.textContent).toEqual("No folders (only templates outside a folder)"); + expect(summaryText.textContent.trim()).toEqual("No folders (only templates outside a folder)"); }); @@ -361,7 +346,7 @@ describe('Collapsible fieldset', () => { // click the first checkbox helpers.triggerEvent(checkboxes[0], 'click'); - expect(summaryText.textContent).toEqual("No team members (only you)"); + expect(summaryText.textContent.trim()).toEqual("No team members (only you)"); }); @@ -381,7 +366,7 @@ describe('Collapsible fieldset', () => { // click the first checkbox helpers.triggerEvent(checkboxes[0], 'click'); - expect(summaryText.textContent).toEqual("No arbitrary things"); + expect(summaryText.textContent.trim()).toEqual("No arbitrary things"); }); @@ -405,7 +390,7 @@ describe('Collapsible fieldset', () => { // click the first checkbox helpers.triggerEvent(checkboxes[1], 'click'); - expect(summaryText.textContent).toEqual("9 of 10 folders"); + expect(summaryText.textContent.trim()).toEqual("9 of 10 folders"); }); @@ -425,7 +410,7 @@ describe('Collapsible fieldset', () => { // click the first checkbox helpers.triggerEvent(checkboxes[1], 'click'); - expect(summaryText.textContent).toEqual("9 of 10 team members"); + expect(summaryText.textContent.trim()).toEqual("9 of 10 team members"); }); @@ -448,7 +433,7 @@ describe('Collapsible fieldset', () => { helpers.triggerEvent(checkboxes[0], 'click'); - expect(summaryText.textContent).toEqual("All folders"); + expect(summaryText.textContent.trim()).toEqual("All folders"); }); @@ -467,7 +452,7 @@ describe('Collapsible fieldset', () => { helpers.triggerEvent(checkboxes[0], 'click'); - expect(summaryText.textContent).toEqual("All team members"); + expect(summaryText.textContent.trim()).toEqual("All team members"); });