Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix aria expanded and height of option select #1146

Merged
merged 3 commits into from
Sep 20, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion app/assets/javascripts/govuk-component/option-select.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@
if (this.$optionSelect.data('closed-on-load') == true) {
this.close();
}
else {
this.setupHeight();
}
}
}

Expand All @@ -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);
Expand Down
9 changes: 4 additions & 5 deletions app/assets/stylesheets/govuk-component/_option-select.scss
Original file line number Diff line number Diff line change
Expand Up @@ -38,22 +38,21 @@
.options-container {
position: relative;
background-color: $page-colour;
overflow-y: scroll;
overflow-y: auto;
overflow-x: hidden;

@include media(desktop) {
max-height: 200px;
}

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%;
}

Expand Down
2 changes: 2 additions & 0 deletions app/views/govuk_component/docs/option_select.yml
Original file line number Diff line number Diff line change
@@ -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:
Expand Down
39 changes: 35 additions & 4 deletions spec/javascripts/govuk-component/option-select-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(){
Expand Down Expand Up @@ -124,7 +154,8 @@ describe('GOVUK.OptionSelect', function() {
expect($optionSelectHTML.hasClass('js-closed')).toBe(false);
});

it ('calls setupHeight()', function(){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why delete this test?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test assumes setupHeight gets called when the option select is opened, but that's now not always the case. setupHeight is called on init if the option select is in the expanded state, but not if it's collapsed. I'm writing other tests to fill this gap.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have now put this test back in a modified form.

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);
Expand Down