From 2138c6b24a73f33d8a9ba8185d73a9d8661da343 Mon Sep 17 00:00:00 2001
From: Andy Sellick <andy.sellick@digital.cabinet-office.gov.uk>
Date: Mon, 18 Sep 2017 09:45:20 +0000
Subject: [PATCH 1/3] Fix aria expanded and height of option select

- set aria-expanded to true by default when initialising rather than performing a check as it will always be open on load, unless it's set to be closed in which case the code will change it back to false anyway as it closes it
- fix for height issue by calling setheight on initialisation, code is structured already so that it won't get called again unless it needs to be
- fixes #860
---
 app/assets/javascripts/govuk-component/option-select.js | 5 ++++-
 app/views/govuk_component/docs/option_select.yml        | 2 ++
 2 files changed, 6 insertions(+), 1 deletion(-)

diff --git a/app/assets/javascripts/govuk-component/option-select.js b/app/assets/javascripts/govuk-component/option-select.js
index 8fe34d178..37603cede 100644
--- a/app/assets/javascripts/govuk-component/option-select.js
+++ b/app/assets/javascripts/govuk-component/option-select.js
@@ -43,6 +43,9 @@
       if (this.$optionSelect.data('closed-on-load') == true) {
         this.close();
       }
+      else {
+        this.setupHeight();
+      }
     }
   }
 
@@ -60,7 +63,7 @@
     $button.addClass('js-container-head');
     //Add type button to override default type submit when this component is used within a form
     $button.attr('type', 'button');
-    $button.attr('aria-expanded', this.isClosed());
+    $button.attr('aria-expanded', true);
     $button.attr('aria-controls', this.$optionSelect.find('.options-container').attr('id'));
     $button.html(jsContainerHeadHTML);
     $containerHead.replaceWith($button);
diff --git a/app/views/govuk_component/docs/option_select.yml b/app/views/govuk_component/docs/option_select.yml
index d23941fca..1eeee9131 100644
--- a/app/views/govuk_component/docs/option_select.yml
+++ b/app/views/govuk_component/docs/option_select.yml
@@ -1,6 +1,8 @@
 name: Option select
 description: A scrollable list of checkboxes to be displayed on a form where one might
   otherwise use a multi-select
+accessibility_criteria: |
+  Options on desktop should not be full width to let users interact with the scrollbar without accidentally clicking an option
 examples:
   default:
     data:

From 195e88b838964441a92fdc559c4d36006a352ef2 Mon Sep 17 00:00:00 2001
From: Andy Sellick <andy.sellick@digital.cabinet-office.gov.uk>
Date: Mon, 18 Sep 2017 10:04:04 +0000
Subject: [PATCH 2/3] Update option select test

- remove test to check setupheight is called on open, as now called on init
- add test to check that a height is set during init
---
 .../govuk-component/option-select-spec.js     | 39 +++++++++++++++++--
 1 file changed, 35 insertions(+), 4 deletions(-)

diff --git a/spec/javascripts/govuk-component/option-select-spec.js b/spec/javascripts/govuk-component/option-select-spec.js
index 056a71e7a..f6fd80a32 100644
--- a/spec/javascripts/govuk-component/option-select-spec.js
+++ b/spec/javascripts/govuk-component/option-select-spec.js
@@ -66,24 +66,54 @@ describe('GOVUK.OptionSelect', function() {
 
 
   it('instantiates a closed option-select if data-closed-on-load is true', function(){
-    $closedOnLoadFixture = $('<div class="govuk-option-select" data-closed-on-load=true></div>');
+    closedOnLoadFixture = '<div class="govuk-option-select" data-closed-on-load=true>' +
+                            '<div class="container-head js-container-head"></div>' +
+                          '</div>';
+    $closedOnLoadFixture = $(closedOnLoadFixture);
+
     $('body').append($closedOnLoadFixture);
     optionSelect = new GOVUK.OptionSelect({$el:$closedOnLoadFixture});
     expect(optionSelect.isClosed()).toBe(true);
+    expect($closedOnLoadFixture.find('button').attr('aria-expanded')).toBe('false');
   });
 
   it('instantiates an open option-select if data-closed-on-load is false', function(){
-    $openOnLoadFixture = $('<div class="govuk-option-select" data-closed-on-load="false"></div>');
+    openOnLoadFixture = '<div class="govuk-option-select" data-closed-on-load=false>' +
+                            '<div class="container-head js-container-head"></div>' +
+                          '</div>';
+    $openOnLoadFixture = $(openOnLoadFixture);
+
     $('body').append($openOnLoadFixture);
     optionSelect = new GOVUK.OptionSelect({$el:$openOnLoadFixture});
     expect(optionSelect.isClosed()).toBe(false);
+    expect($openOnLoadFixture.find('button').attr('aria-expanded')).toBe('true');
   });
 
   it('instantiates an open option-select if data-closed-on-load is not present', function(){
-    $openOnLoadFixture = $('<div class="govuk-option-select"></div>');
+    openOnLoadFixture = '<div class="govuk-option-select">' +
+                          '<div class="container-head js-container-head"></div>' +
+                        '</div>';
+    $openOnLoadFixture = $(openOnLoadFixture);
+
     $('body').append($openOnLoadFixture);
     optionSelect = new GOVUK.OptionSelect({$el:$openOnLoadFixture});
     expect(optionSelect.isClosed()).toBe(false);
+    expect($openOnLoadFixture.find('button').attr('aria-expanded')).toBe('true');
+  });
+
+  it ('sets the height of the options container as part of initialisation', function(){
+    expect($optionSelectHTML.find('.options-container').attr('style')).toContain('height');
+  });
+
+  it ('doesn\'t set the height of the options container as part of initialisation if closed-on-load is true', function(){
+    closedOnLoadFixture = '<div class="govuk-option-select" data-closed-on-load=true>' +
+                            '<div class="options-container"></div>' +
+                          '</div>';
+    $closedOnLoadFixture = $(closedOnLoadFixture);
+
+    $('body').append($closedOnLoadFixture);
+    optionSelect = new GOVUK.OptionSelect({$el:$closedOnLoadFixture});
+    expect($closedOnLoadFixture.find('.options-container').attr('style')).not.toContain('height');
   });
 
   describe('replaceHeadWithButton', function(){
@@ -124,7 +154,8 @@ describe('GOVUK.OptionSelect', function() {
       expect($optionSelectHTML.hasClass('js-closed')).toBe(false);
     });
 
-    it ('calls setupHeight()', function(){
+    it ('calls setupHeight() if a height has not been set', function(){
+      $optionSelectHTML.find('.options-container').attr('style', '');
       spyOn(optionSelect, "setupHeight");
       optionSelect.open();
       expect(optionSelect.setupHeight.calls.count()).toBe(1);

From a161dae34f3e0bf6c686f453d8d29af8d03c99fa Mon Sep 17 00:00:00 2001
From: Andy Sellick <andy.sellick@digital.cabinet-office.gov.uk>
Date: Tue, 19 Sep 2017 08:48:56 +0000
Subject: [PATCH 3/3] CSS improvements to option select

- use display block on labels rather than inline block and having to set widths
- add a bit of padding right to labels to smarten them up a bit
- change overflow-y from scroll to auto on label container so scrollbar isn't always present
---
 .../stylesheets/govuk-component/_option-select.scss      | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/app/assets/stylesheets/govuk-component/_option-select.scss b/app/assets/stylesheets/govuk-component/_option-select.scss
index e720810f3..ed3136dcd 100644
--- a/app/assets/stylesheets/govuk-component/_option-select.scss
+++ b/app/assets/stylesheets/govuk-component/_option-select.scss
@@ -38,7 +38,7 @@
   .options-container {
     position: relative;
     background-color: $page-colour;
-    overflow-y: scroll;
+    overflow-y: auto;
     overflow-x: hidden;
 
     @include media(desktop) {
@@ -46,14 +46,13 @@
     }
 
     label {
-      @include inline-block;
-      padding: 7px 0 7px 35px;
+      display: block;
+      padding: 7px 7px 7px 35px;
       border-bottom: 1px solid $border-colour;
-      width: 100%;
       cursor: pointer;
 
       @include media(desktop) {
-        // leave room for the scroll bars on desktop
+        // leave space to interact with the scroll bars on desktop
         width: 90%;
       }