From 3acf34015f7fa5a8d39bf631d49a46a6326e03af Mon Sep 17 00:00:00 2001
From: Robin Whittleton <robin.whittleton@digital.cabinet-office.gov.uk>
Date: Mon, 23 Jan 2017 16:04:21 +0000
Subject: [PATCH] Fix radio show/hide behaviour outside a form
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The show/hide JS assumes radio buttons are inside a form, as the default behaviour of radio buttons is to scope their name to the nearest enclosing form. If they’re outside a form the JS fails. We can alter it to just select every applicable radio on the page in the absence of a form ancestor.

Adding tests for this involved quite a bit of rework as it turns out that Jasmine will just run every beforeEach() in order rather than letting beforeEach()s lower in scope override the parents.
---
 javascripts/govuk/show-hide-content.js |   3 +-
 spec/unit/show-hide-content.spec.js    | 441 ++++++++++++++-----------
 2 files changed, 251 insertions(+), 193 deletions(-)

diff --git a/javascripts/govuk/show-hide-content.js b/javascripts/govuk/show-hide-content.js
index 5894f388..e72e0882 100644
--- a/javascripts/govuk/show-hide-content.js
+++ b/javascripts/govuk/show-hide-content.js
@@ -80,7 +80,8 @@
     function handleRadioContent ($control, $content) {
       // All radios in this group which control content
       var selector = selectors.radio + '[name=' + escapeElementName($control.attr('name')) + '][aria-controls]'
-      var $radios = $control.closest('form').find(selector)
+      var $form = $control.closest('form')
+      var $radios = $form.length ? $form.find(selector) : $(selector)
 
       // Hide content for radios in group
       $radios.each(function () {
diff --git a/spec/unit/show-hide-content.spec.js b/spec/unit/show-hide-content.spec.js
index c4ae20e0..b4ed4c3b 100644
--- a/spec/unit/show-hide-content.spec.js
+++ b/spec/unit/show-hide-content.spec.js
@@ -6,66 +6,6 @@ describe('show-hide-content', function () {
   'use strict'
   var GOVUK = window.GOVUK
 
-  beforeEach(function () {
-    // Sample markup
-    this.$content = $(
-
-      // Radio buttons (yes/no)
-      '<form>' +
-      '<label class="block-label" data-target="show-hide-radios">' +
-      '<input type="radio" name="single" value="no">' +
-      'Yes' +
-      '</label>' +
-      '<label class="block-label">' +
-      '<input type="radio" name="single" value="yes">' +
-      'No' +
-      '</label>' +
-      '<div id="show-hide-radios" class="panel js-hidden" />' +
-      '</form>' +
-
-      // Checkboxes (multiple values)
-      '<form>' +
-      '<label class="block-label" data-target="show-hide-checkboxes">' +
-      '<input type="checkbox" name="multiple[option1]">' +
-      'Option 1' +
-      '</label>' +
-      '<label class="block-label">' +
-      '<input type="checkbox" name="multiple[option2]">' +
-      'Option 2' +
-      '</label>' +
-      '<label class="block-label">' +
-      '<input type="checkbox" name="multiple[option3]">' +
-      'Option 3' +
-      '</label>' +
-      '<div id="show-hide-checkboxes" class="panel js-hidden" />' +
-      '</form>'
-    )
-
-    // Find radios/checkboxes
-    var $radios = this.$content.find('input[type=radio]')
-    var $checkboxes = this.$content.find('input[type=checkbox]')
-
-    // Two radios
-    this.$radio1 = $radios.eq(0)
-    this.$radio2 = $radios.eq(1)
-
-    // Three checkboxes
-    this.$checkbox1 = $checkboxes.eq(0)
-    this.$checkbox2 = $checkboxes.eq(1)
-    this.$checkbox3 = $checkboxes.eq(2)
-
-    // Add to page
-    $(document.body).append(this.$content)
-
-    // Show/Hide content
-    this.$radioShowHide = $('#show-hide-radios')
-    this.$checkboxShowHide = $('#show-hide-checkboxes')
-
-    // Add show/hide content support
-    this.showHideContent = new GOVUK.ShowHideContent()
-    this.showHideContent.init()
-  })
-
   afterEach(function () {
     if (this.showHideContent) {
       this.showHideContent.destroy()
@@ -74,176 +14,293 @@ describe('show-hide-content', function () {
     this.$content.remove()
   })
 
-  describe('when this.showHideContent = new GOVUK.ShowHideContent() is called', function () {
-    it('should add the aria attributes to inputs with show/hide content', function () {
-      expect(this.$radio1.attr('aria-expanded')).toBe('false')
-      expect(this.$radio1.attr('aria-controls')).toBe('show-hide-radios')
-    })
-
-    it('should add the aria attributes to show/hide content', function () {
-      expect(this.$radioShowHide.attr('aria-hidden')).toBe('true')
-      expect(this.$radioShowHide.hasClass('js-hidden')).toEqual(true)
-    })
-
-    it('should hide the show/hide content visually', function () {
-      expect(this.$radioShowHide.hasClass('js-hidden')).toEqual(true)
-    })
-
-    it('should do nothing if no radios are checked', function () {
-      expect(this.$radio1.attr('aria-expanded')).toBe('false')
-      expect(this.$radio2.attr('aria-expanded')).toBe(undefined)
-    })
-
-    it('should do nothing if no checkboxes are checked', function () {
-      expect(this.$radio1.attr('aria-expanded')).toBe('false')
-      expect(this.$radio2.attr('aria-expanded')).toBe(undefined)
+  describe('when the radios are inside a form', function () {
+    beforeEach(function () {
+      // Sample markup
+      this.$content = $(
+
+        // Radio buttons (yes/no)
+        '<form>' +
+        '<label class="block-label" data-target="show-hide-radios">' +
+        '<input type="radio" name="single" value="yes">' +
+        'Yes' +
+        '</label>' +
+        '<label class="block-label">' +
+        '<input type="radio" name="single" value="no">' +
+        'No' +
+        '</label>' +
+        '<div id="show-hide-radios" class="panel js-hidden" />' +
+        '</form>' +
+
+        // Checkboxes (multiple values)
+        '<form>' +
+        '<label class="block-label" data-target="show-hide-checkboxes">' +
+        '<input type="checkbox" name="multiple[option1]">' +
+        'Option 1' +
+        '</label>' +
+        '<label class="block-label">' +
+        '<input type="checkbox" name="multiple[option2]">' +
+        'Option 2' +
+        '</label>' +
+        '<label class="block-label">' +
+        '<input type="checkbox" name="multiple[option3]">' +
+        'Option 3' +
+        '</label>' +
+        '<div id="show-hide-checkboxes" class="panel js-hidden" />' +
+        '</form>'
+      )
+
+      // Find radios/checkboxes
+      var $radios = this.$content.find('input[type=radio]')
+      var $checkboxes = this.$content.find('input[type=checkbox]')
+
+      // Two radios
+      this.$radio1 = $radios.eq(0)
+      this.$radio2 = $radios.eq(1)
+
+      // Three checkboxes
+      this.$checkbox1 = $checkboxes.eq(0)
+      this.$checkbox2 = $checkboxes.eq(1)
+      this.$checkbox3 = $checkboxes.eq(2)
+
+      // Add to page
+      $(document.body).append(this.$content)
+
+      // Show/Hide content
+      this.$radioShowHide = $('#show-hide-radios')
+      this.$checkboxShowHide = $('#show-hide-checkboxes')
+
+      // Add show/hide content support
+      this.showHideContent = new GOVUK.ShowHideContent()
+      this.showHideContent.init()
     })
 
-    describe('with non-default markup', function () {
-      beforeEach(function () {
-        this.showHideContent.destroy()
+    describe('and when this.showHideContent = new GOVUK.ShowHideContent() is called', function () {
+      it('should add the aria attributes to inputs with show/hide content', function () {
+        expect(this.$radio1.attr('aria-expanded')).toBe('false')
+        expect(this.$radio1.attr('aria-controls')).toBe('show-hide-radios')
       })
 
-      it('should do nothing if a radio without show/hide content is checked', function () {
-        this.$radio2.prop('checked', true)
-
-        // Defaults changed, initialise again
-        this.showHideContent = new GOVUK.ShowHideContent().init()
-        expect(this.$radio1.attr('aria-expanded')).toBe('false')
+      it('should add the aria attributes to show/hide content', function () {
         expect(this.$radioShowHide.attr('aria-hidden')).toBe('true')
         expect(this.$radioShowHide.hasClass('js-hidden')).toEqual(true)
       })
 
-      it('should do nothing if a checkbox without show/hide content is checked', function () {
-        this.$checkbox2.prop('checked', true)
-
-        // Defaults changed, initialise again
-        this.showHideContent = new GOVUK.ShowHideContent().init()
-        expect(this.$checkbox1.attr('aria-expanded')).toBe('false')
-        expect(this.$checkboxShowHide.attr('aria-hidden')).toBe('true')
-        expect(this.$checkboxShowHide.hasClass('js-hidden')).toEqual(true)
+      it('should hide the show/hide content visually', function () {
+        expect(this.$radioShowHide.hasClass('js-hidden')).toEqual(true)
       })
 
-      it('should do nothing if checkboxes without show/hide content is checked', function () {
-        this.$checkbox2.prop('checked', true)
-        this.$checkbox3.prop('checked', true)
-
-        // Defaults changed, initialise again
-        this.showHideContent = new GOVUK.ShowHideContent().init()
-        expect(this.$checkbox1.attr('aria-expanded')).toBe('false')
-        expect(this.$checkboxShowHide.attr('aria-hidden')).toBe('true')
-        expect(this.$checkboxShowHide.hasClass('js-hidden')).toEqual(true)
+      it('should do nothing if no radios are checked', function () {
+        expect(this.$radio1.attr('aria-expanded')).toBe('false')
+        expect(this.$radio2.attr('aria-expanded')).toBe(undefined)
       })
 
-      it('should make the show/hide content visible if its radio is checked', function () {
-        this.$radio1.prop('checked', true)
-
-        // Defaults changed, initialise again
-        this.showHideContent = new GOVUK.ShowHideContent().init()
-        expect(this.$radio1.attr('aria-expanded')).toBe('true')
-        expect(this.$radioShowHide.attr('aria-hidden')).toBe('false')
-        expect(this.$radioShowHide.hasClass('js-hidden')).toEqual(false)
+      it('should do nothing if no checkboxes are checked', function () {
+        expect(this.$radio1.attr('aria-expanded')).toBe('false')
+        expect(this.$radio2.attr('aria-expanded')).toBe(undefined)
       })
 
-      it('should make the show/hide content visible if its checkbox is checked', function () {
-        this.$checkbox1.prop('checked', true)
-
-        // Defaults changed, initialise again
-        this.showHideContent = new GOVUK.ShowHideContent().init()
-        expect(this.$checkbox1.attr('aria-expanded')).toBe('true')
-        expect(this.$checkboxShowHide.attr('aria-hidden')).toBe('false')
-        expect(this.$checkboxShowHide.hasClass('js-hidden')).toEqual(false)
+      describe('with non-default markup', function () {
+        beforeEach(function () {
+          this.showHideContent.destroy()
+        })
+
+        it('should do nothing if a radio without show/hide content is checked', function () {
+          this.$radio2.prop('checked', true)
+
+          // Defaults changed, initialise again
+          this.showHideContent = new GOVUK.ShowHideContent().init()
+          expect(this.$radio1.attr('aria-expanded')).toBe('false')
+          expect(this.$radioShowHide.attr('aria-hidden')).toBe('true')
+          expect(this.$radioShowHide.hasClass('js-hidden')).toEqual(true)
+        })
+
+        it('should do nothing if a checkbox without show/hide content is checked', function () {
+          this.$checkbox2.prop('checked', true)
+
+          // Defaults changed, initialise again
+          this.showHideContent = new GOVUK.ShowHideContent().init()
+          expect(this.$checkbox1.attr('aria-expanded')).toBe('false')
+          expect(this.$checkboxShowHide.attr('aria-hidden')).toBe('true')
+          expect(this.$checkboxShowHide.hasClass('js-hidden')).toEqual(true)
+        })
+
+        it('should do nothing if checkboxes without show/hide content is checked', function () {
+          this.$checkbox2.prop('checked', true)
+          this.$checkbox3.prop('checked', true)
+
+          // Defaults changed, initialise again
+          this.showHideContent = new GOVUK.ShowHideContent().init()
+          expect(this.$checkbox1.attr('aria-expanded')).toBe('false')
+          expect(this.$checkboxShowHide.attr('aria-hidden')).toBe('true')
+          expect(this.$checkboxShowHide.hasClass('js-hidden')).toEqual(true)
+        })
+
+        it('should make the show/hide content visible if its radio is checked', function () {
+          this.$radio1.prop('checked', true)
+
+          // Defaults changed, initialise again
+          this.showHideContent = new GOVUK.ShowHideContent().init()
+          expect(this.$radio1.attr('aria-expanded')).toBe('true')
+          expect(this.$radioShowHide.attr('aria-hidden')).toBe('false')
+          expect(this.$radioShowHide.hasClass('js-hidden')).toEqual(false)
+        })
+
+        it('should make the show/hide content visible if its checkbox is checked', function () {
+          this.$checkbox1.prop('checked', true)
+
+          // Defaults changed, initialise again
+          this.showHideContent = new GOVUK.ShowHideContent().init()
+          expect(this.$checkbox1.attr('aria-expanded')).toBe('true')
+          expect(this.$checkboxShowHide.attr('aria-hidden')).toBe('false')
+          expect(this.$checkboxShowHide.hasClass('js-hidden')).toEqual(false)
+        })
       })
-    })
 
-    describe('and a show/hide radio receives a click', function () {
-      it('should make the show/hide content visible', function () {
-        this.$radio1.click()
-        expect(this.$radioShowHide.hasClass('js-hidden')).toEqual(false)
+      describe('and a show/hide radio receives a click', function () {
+        it('should make the show/hide content visible', function () {
+          this.$radio1.click()
+          expect(this.$radioShowHide.hasClass('js-hidden')).toEqual(false)
+        })
+
+        it('should add the aria attributes to show/hide content', function () {
+          this.$radio1.click()
+          expect(this.$radio1.attr('aria-expanded')).toBe('true')
+          expect(this.$radioShowHide.attr('aria-hidden')).toBe('false')
+          expect(this.$radioShowHide.hasClass('js-hidden')).toEqual(false)
+        })
       })
 
-      it('should add the aria attributes to show/hide content', function () {
-        this.$radio1.click()
-        expect(this.$radio1.attr('aria-expanded')).toBe('true')
-        expect(this.$radioShowHide.attr('aria-hidden')).toBe('false')
-        expect(this.$radioShowHide.hasClass('js-hidden')).toEqual(false)
+      describe('and a show/hide checkbox receives a click', function () {
+        it('should make the show/hide content visible', function () {
+          this.$checkbox1.click()
+          expect(this.$checkboxShowHide.hasClass('js-hidden')).toEqual(false)
+        })
+
+        it('should add the aria attributes to show/hide content', function () {
+          this.$checkbox1.click()
+          expect(this.$checkbox1.attr('aria-expanded')).toBe('true')
+          expect(this.$checkboxShowHide.attr('aria-hidden')).toBe('false')
+          expect(this.$checkboxShowHide.hasClass('js-hidden')).toEqual(false)
+        })
       })
-    })
 
-    describe('and a show/hide checkbox receives a click', function () {
-      it('should make the show/hide content visible', function () {
-        this.$checkbox1.click()
-        expect(this.$checkboxShowHide.hasClass('js-hidden')).toEqual(false)
+      describe('and a show/hide radio receives a click, but another group radio is clicked afterwards', function () {
+        it('should make the show/hide content hidden', function () {
+          this.$radio1.click()
+          this.$radio2.click()
+          expect(this.$radioShowHide.hasClass('js-hidden')).toEqual(true)
+        })
+
+        it('should add the aria attributes to show/hide content', function () {
+          this.$radio1.click()
+          this.$radio2.click()
+          expect(this.$radio1.attr('aria-expanded')).toBe('false')
+          expect(this.$radioShowHide.attr('aria-hidden')).toBe('true')
+        })
       })
 
-      it('should add the aria attributes to show/hide content', function () {
-        this.$checkbox1.click()
-        expect(this.$checkbox1.attr('aria-expanded')).toBe('true')
-        expect(this.$checkboxShowHide.attr('aria-hidden')).toBe('false')
-        expect(this.$checkboxShowHide.hasClass('js-hidden')).toEqual(false)
+      describe('and a show/hide checkbox receives a click, but another checkbox is clicked afterwards', function () {
+        it('should keep the show/hide content visible', function () {
+          this.$checkbox1.click()
+          this.$checkbox2.click()
+          expect(this.$checkboxShowHide.hasClass('js-hidden')).toEqual(false)
+        })
+
+        it('should keep the aria attributes to show/hide content', function () {
+          this.$checkbox1.click()
+          this.$checkbox2.click()
+          expect(this.$checkbox1.attr('aria-expanded')).toBe('true')
+          expect(this.$checkboxShowHide.attr('aria-hidden')).toBe('false')
+        })
       })
     })
 
-    describe('and a show/hide radio receives a click, but another group radio is clicked afterwards', function () {
-      it('should make the show/hide content hidden', function () {
-        this.$radio1.click()
-        this.$radio2.click()
-        expect(this.$radioShowHide.hasClass('js-hidden')).toEqual(true)
-      })
-
-      it('should add the aria attributes to show/hide content', function () {
-        this.$radio1.click()
-        this.$radio2.click()
-        expect(this.$radio1.attr('aria-expanded')).toBe('false')
-        expect(this.$radioShowHide.attr('aria-hidden')).toBe('true')
+    describe('before this.showHideContent.destroy() is called', function () {
+      it('document.body should have show/hide event handlers', function () {
+        var events = $._data(document.body, 'events')
+        expect(events && events.click).toContain(jasmine.objectContaining({
+          namespace: 'ShowHideContent',
+          selector: 'input[type="radio"][name="single"]'
+        }))
+        expect(events && events.click).toContain(jasmine.objectContaining({
+          namespace: 'ShowHideContent',
+          selector: '.block-label[data-target] input[type="checkbox"]'
+        }))
       })
     })
 
-    describe('and a show/hide checkbox receives a click, but another checkbox is clicked afterwards', function () {
-      it('should keep the show/hide content visible', function () {
-        this.$checkbox1.click()
-        this.$checkbox2.click()
-        expect(this.$checkboxShowHide.hasClass('js-hidden')).toEqual(false)
+    describe('and when this.showHideContent.destroy() is called', function () {
+      beforeEach(function () {
+        this.showHideContent.destroy()
       })
 
-      it('should keep the aria attributes to show/hide content', function () {
-        this.$checkbox1.click()
-        this.$checkbox2.click()
-        expect(this.$checkbox1.attr('aria-expanded')).toBe('true')
-        expect(this.$checkboxShowHide.attr('aria-hidden')).toBe('false')
+      it('should have no show/hide event handlers', function () {
+        var events = $._data(document.body, 'events')
+        expect(events && events.click).not.toContain(jasmine.objectContaining({
+          namespace: 'ShowHideContent',
+          selector: 'input[type="radio"][name="single"]'
+        }))
+        expect(events && events.click).not.toContain(jasmine.objectContaining({
+          namespace: 'ShowHideContent',
+          selector: '.block-label[data-target] input[type="checkbox"]'
+        }))
       })
     })
   })
 
-  describe('before this.showHideContent.destroy() is called', function () {
-    it('document.body should have show/hide event handlers', function () {
-      var events = $._data(document.body, 'events')
-      expect(events && events.click).toContain(jasmine.objectContaining({
-        namespace: 'ShowHideContent',
-        selector: 'input[type="radio"][name="single"]'
-      }))
-      expect(events && events.click).toContain(jasmine.objectContaining({
-        namespace: 'ShowHideContent',
-        selector: '.block-label[data-target] input[type="checkbox"]'
-      }))
+  describe('when the radios are outside of a form', function () {
+    beforeEach(function () {
+      // Sample markup
+      this.$content = $(
+        // Radio buttons (yes/no)
+        '<label class="block-label" data-target="show-hide-radios">' +
+        '<input type="radio" name="single" value="yes">' +
+        'Yes' +
+        '</label>' +
+        '<label class="block-label">' +
+        '<input type="radio" name="single" value="no">' +
+        'No' +
+        '</label>' +
+        '<div id="show-hide-radios" class="panel js-hidden" />'
+      )
+
+      // Find radios/checkboxes
+      var $radios = this.$content.find('input[type=radio]')
+
+      // Two radios
+      this.$radio1 = $radios.eq(0)
+      this.$radio2 = $radios.eq(1)
+
+      // Add to page
+      $(document.body).append(this.$content)
+
+      // Show/Hide content
+      this.$radioShowHide = $('#show-hide-radios')
+
+      // Add show/hide content support
+      this.showHideContent = new GOVUK.ShowHideContent()
+      this.showHideContent.init()
     })
-  })
 
-  describe('when this.showHideContent.destroy() is called', function () {
-    beforeEach(function () {
-      this.showHideContent.destroy()
+    it('should make the show/hide content visible if its radio is checked', function () {
+      this.$radio1.click()
+
+      // Defaults changed, initialise again
+      this.showHideContent = new GOVUK.ShowHideContent().init()
+      expect(this.$radio1.attr('aria-expanded')).toBe('true')
+      expect(this.$radioShowHide.attr('aria-hidden')).toBe('false')
+      expect(this.$radioShowHide.hasClass('js-hidden')).toEqual(false)
     })
 
-    it('should have no show/hide event handlers', function () {
-      var events = $._data(document.body, 'events')
-      expect(events && events.click).not.toContain(jasmine.objectContaining({
-        namespace: 'ShowHideContent',
-        selector: 'input[type="radio"][name="single"]'
-      }))
-      expect(events && events.click).not.toContain(jasmine.objectContaining({
-        namespace: 'ShowHideContent',
-        selector: '.block-label[data-target] input[type="checkbox"]'
-      }))
+    it('should do nothing if a radio without show/hide content is checked', function () {
+      this.$radio2.click()
+
+      // Defaults changed, initialise again
+      this.showHideContent = new GOVUK.ShowHideContent().init()
+      expect(this.$radio1.attr('aria-expanded')).toBe('false')
+      expect(this.$radioShowHide.attr('aria-hidden')).toBe('true')
+      expect(this.$radioShowHide.hasClass('js-hidden')).toEqual(true)
     })
   })
 })