From c186288284b1450668a98729e9f0b417e1acbfac Mon Sep 17 00:00:00 2001 From: Jakub Miarka Date: Thu, 14 Nov 2019 18:49:41 +0000 Subject: [PATCH 1/2] Make sure IDs on collapsible navigation are unique Multiple nested pages were getting assigned the same ID (from the parent). This change appends the iterator index to the elements to make sure they're unique. Also added a extra nested page to the example docs and a test. --- CHANGELOG.md | 2 ++ .../another-nested-nested-page/index.html.md | 5 +++ .../_modules/collapsible-navigation.js | 8 +++-- .../collapsible-navigation-spec.js | 35 +++++++++++++++++++ 4 files changed, 47 insertions(+), 3 deletions(-) create mode 100644 example/source/nested-page/another-nested-nested-page/index.html.md create mode 100644 spec/javascripts/collapsible-navigation-spec.js diff --git a/CHANGELOG.md b/CHANGELOG.md index e954540c..2f45bc29 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- [Pull request #160: Make sure IDs on collapsible navigation are unique](https://github.com/alphagov/tech-docs-gem/pull/160) + ## 2.0.9 ### Fixes diff --git a/example/source/nested-page/another-nested-nested-page/index.html.md b/example/source/nested-page/another-nested-nested-page/index.html.md new file mode 100644 index 00000000..2cac582c --- /dev/null +++ b/example/source/nested-page/another-nested-nested-page/index.html.md @@ -0,0 +1,5 @@ +--- +title: Another nested nested page +--- + +# Another nested nested page diff --git a/lib/assets/javascripts/_modules/collapsible-navigation.js b/lib/assets/javascripts/_modules/collapsible-navigation.js index 56cf5309..c91f48e8 100644 --- a/lib/assets/javascripts/_modules/collapsible-navigation.js +++ b/lib/assets/javascripts/_modules/collapsible-navigation.js @@ -28,9 +28,11 @@ continue } $topLevelItem.addClass('collapsible') - $listing.addClass('collapsible__body') - .attr('id', id) - .attr('aria-expanded', 'false') + $listing.each(function (i) { + $(this).addClass('collapsible__body') + .attr('id', id + '-' + i) + .attr('aria-expanded', 'false') + }) $heading.addClass('collapsible__heading') .after('') $topLevelItem.on('click', '.collapsible__toggle', function (e) { diff --git a/spec/javascripts/collapsible-navigation-spec.js b/spec/javascripts/collapsible-navigation-spec.js new file mode 100644 index 00000000..592cf02a --- /dev/null +++ b/spec/javascripts/collapsible-navigation-spec.js @@ -0,0 +1,35 @@ +describe('Collapsible navigation', function () { + 'use strict' + + var module + var $navigation + + beforeEach(function () { + module = new GOVUK.Modules.CollapsibleNavigation() + $navigation = $(` + `) + module.start($navigation) + }) + + it('sanitizes headings to unique IDs correctly', function () { + $navigation.find('ul > li > ul').each(function (i) { + expect($(this)[0].id).toEqual('toc-nested-page-' + i) + }) + }) +}) From 511d1ce936b1bf127ecec76390967b555612fbc8 Mon Sep 17 00:00:00 2001 From: Jakub Miarka Date: Fri, 15 Nov 2019 10:48:50 +0000 Subject: [PATCH 2/2] Fixed aria-controls attribute When using multiple nested unique IDs the aria relationship broke. This change adds the list of the nested elements in the `aria-controls` attribute. --- .../_modules/collapsible-navigation.js | 7 +++- .../collapsible-navigation-spec.js | 42 +++++++++++-------- 2 files changed, 29 insertions(+), 20 deletions(-) diff --git a/lib/assets/javascripts/_modules/collapsible-navigation.js b/lib/assets/javascripts/_modules/collapsible-navigation.js index c91f48e8..2c03c662 100644 --- a/lib/assets/javascripts/_modules/collapsible-navigation.js +++ b/lib/assets/javascripts/_modules/collapsible-navigation.js @@ -28,13 +28,16 @@ continue } $topLevelItem.addClass('collapsible') + var arrayOfIds = [] $listing.each(function (i) { + var uniqueId = id + '-' + i + arrayOfIds.push(uniqueId) $(this).addClass('collapsible__body') - .attr('id', id + '-' + i) + .attr('id', uniqueId) .attr('aria-expanded', 'false') }) $heading.addClass('collapsible__heading') - .after('') + .after('') $topLevelItem.on('click', '.collapsible__toggle', function (e) { e.preventDefault() var $parent = $(this).parent() diff --git a/spec/javascripts/collapsible-navigation-spec.js b/spec/javascripts/collapsible-navigation-spec.js index 592cf02a..d85416e2 100644 --- a/spec/javascripts/collapsible-navigation-spec.js +++ b/spec/javascripts/collapsible-navigation-spec.js @@ -6,24 +6,24 @@ describe('Collapsible navigation', function () { beforeEach(function () { module = new GOVUK.Modules.CollapsibleNavigation() - $navigation = $(` - `) + $navigation = $( + '') module.start($navigation) }) @@ -32,4 +32,10 @@ describe('Collapsible navigation', function () { expect($(this)[0].id).toEqual('toc-nested-page-' + i) }) }) + + it('aria-controls on the button lists all the nested IDs', function () { + $navigation.find('button').each(function (i) { + expect($navigation.find('button').attr('aria-controls')).toEqual('toc-nested-page-0 toc-nested-page-1') + }) + }) })