Skip to content

Commit

Permalink
Merge pull request #381 from alphagov/fix-focus-issues
Browse files Browse the repository at this point in the history
Fix focus issues
  • Loading branch information
tombye authored Jan 15, 2025
2 parents e65e6d0 + bcc23c5 commit 58e7c76
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 41 deletions.
20 changes: 12 additions & 8 deletions lib/assets/javascripts/_modules/table-of-contents.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,33 +104,37 @@
function openNavigation () {
$html.addClass('toc-open')

toggleBackgroundVisiblity(false)
updateAriaAttributes()
$toc.focus()
}

function closeNavigation () {
$html.removeClass('toc-open')

toggleBackgroundVisiblity(true)
updateAriaAttributes()
}

function toggleBackgroundVisiblity (visibility) {
$('.toc-open-disabled').attr('aria-hidden', visibility ? '' : 'true')
$openButton.focus()
}

function updateAriaAttributes () {
var tocIsVisible = $toc.is(':visible')
var openButtonIsVisible = $openButton.is(':visible')
var tocIsDialog = $openButton.is(':visible')

$($openButton).add($closeButton)
.attr('aria-expanded', tocIsVisible ? 'true' : 'false')

$toc.attr({
'aria-hidden': tocIsVisible ? 'false' : 'true',
role: openButtonIsVisible ? 'dialog' : null
role: tocIsDialog ? 'dialog' : null
})

$('.app-pane__content').attr('aria-hidden', (tocIsDialog && tocIsVisible) ? 'true' : 'false')

// only make main content pane focusable if it scrolls independently of the toc
if (!tocIsDialog) {
$('.app-pane__content').attr('tabindex', '0')
} else {
$('.app-pane__content').removeAttr('tabindex')
}
}

function preventingScrolling (callback) {
Expand Down
7 changes: 7 additions & 0 deletions lib/assets/stylesheets/modules/_app-pane.scss
Original file line number Diff line number Diff line change
Expand Up @@ -61,3 +61,10 @@
}
}

.app-pane__content:focus-visible,
.app-pane__content:has(main:focus-visible) {
outline: $govuk-focus-width solid transparent;
box-shadow:
0 0 0 4px $govuk-focus-colour,
0 0 0 8px $govuk-focus-text-colour;
}
2 changes: 1 addition & 1 deletion lib/source/layouts/core.erb
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
</div>
<% end %>

<div class="app-pane__content toc-open-disabled" aria-label="Content" tabindex="0">
<div class="app-pane__content toc-open-disabled" aria-label="Content">
<main id="content" class="technical-documentation" data-module="anchored-headings">
<%= yield %>
<%= partial "layouts/page_review" %>
Expand Down
95 changes: 63 additions & 32 deletions spec/javascripts/table-of-contents-spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ describe('Table of contents', function () {
var $html
var $tocBase
var $toc
var $mainContentPane
var $closeButton
var $openButton
var $tocStickyHeader
Expand All @@ -13,28 +14,33 @@ describe('Table of contents', function () {
beforeAll(function () {
$html = $('html')
$tocBase = $(
'<div class="toc" data-module="table-of-contents" tabindex="-1" aria-label="Table of contents">' +
'<div class="search" data-module="search" data-path-to-site-root="/">' +
'<form action="/search/index.html" method="get" role="search" class="search__form govuk-!-margin-bottom-4">' +
'<label class="govuk-label search__label" for="search">Search this documentation</label>' +
'<input type="text" id="search" name="q" class="govuk-input govuk-!-margin-bottom-0 search__input" aria-controls="search-results" placeholder="Search">' +
'<button type="submit" class="search__button">Search</button>' +
'</form>' +
'<div class="app-pane__toc">' +
'<div class="toc" data-module="table-of-contents" tabindex="-1" aria-label="Table of contents">' +
'<div class="search" data-module="search" data-path-to-site-root="/">' +
'<form action="/search/index.html" method="get" role="search" class="search__form govuk-!-margin-bottom-4">' +
'<label class="govuk-label search__label" for="search">Search this documentation</label>' +
'<input type="text" id="search" name="q" class="govuk-input govuk-!-margin-bottom-0 search__input" aria-controls="search-results" placeholder="Search">' +
'<button type="submit" class="search__button">Search</button>' +
'</form>' +
'</div>' +
'<button type="button" class="toc__close js-toc-close" aria-controls="toc" aria-label="Hide table of contents"></button>' +
'<nav id="toc" class="js-toc-list toc__list" aria-labelledby="toc-heading" data-module="collapsible-navigation">' +
'<ul>' +
'<li>' +
'<a href="/"><span>Technical Documentation Template</span></a>' +
'</li>' +
'<li>' +
'<a href="/"><span>Get started</span></a>' +
'</li>' +
'<li>' +
'<a href="/"><span>Configure your documentation site</span></a>' +
'</li>' +
'</ul>' +
'</nav>' +
'</div>' +
'<button type="button" class="toc__close js-toc-close" aria-controls="toc" aria-label="Hide table of contents"></button>' +
'<nav id="toc" class="js-toc-list toc__list" aria-labelledby="toc-heading" data-module="collapsible-navigation">' +
'<ul>' +
'<li>' +
'<a href="/"><span>Technical Documentation Template</span></a>' +
'</li>' +
'<li>' +
'<a href="/"><span>Get started</span></a>' +
'</li>' +
'<li>' +
'<a href="/"><span>Configure your documentation site</span></a>' +
'</li>' +
'</ul>' +
'</nav>' +
'</div>' +
'<div class="app-pane__content toc-open-disabled" aria-label="content">' +
'<main id="content"></main>' +
'</div>'
)

Expand All @@ -51,7 +57,7 @@ describe('Table of contents', function () {
})

beforeEach(function () {
$toc = $tocBase.clone()
var $tocClone = $tocBase.clone()

$html.find('body')
.append(
Expand All @@ -61,8 +67,10 @@ describe('Table of contents', function () {
'</button>' +
'</div>'
)
.append($toc)
.append($tocClone)

$toc = $tocClone.eq(0).find('.toc')
$mainContentPane = $tocClone.eq(1)
$closeButton = $toc.find('.js-toc-close')
$openButton = $html.find('.js-toc-show')

Expand All @@ -73,14 +81,15 @@ describe('Table of contents', function () {
// clear up any classes left on <html>
$html.removeClass('toc-open')
$html.find('body #toc-heading').remove()
$html.find('body .toc').remove()
$html.find('body .app-pane__toc').remove()
$html.find('body .app-pane__content').remove()
if ($tocStickyHeader && $tocStickyHeader.length) {
$tocStickyHeader.remove()
}
})

describe('when the module is started', function () {
it('on a mobile-size screen, it should mark the table of contents as hidden', function () {
it('on a mobile-size screen, it should hide the table of contents and stop the main content pane being focusable', function () {
// styles applied by this test simulate the styles media-queries will apply on real web pages
// the .mobile-size class hides the table of contents and the open button
$html.addClass('mobile-size') // simulate the styles media-queries will apply on real web pages
Expand All @@ -89,18 +98,20 @@ describe('Table of contents', function () {
module.start($toc)

expect($toc.attr('aria-hidden')).toEqual('true')
expect($mainContentPane.get(0).hasAttribute('tabindex')).toBe(false)

$html.removeClass('mobile-size')
})

it('on a desktop-size screen, it should mark the table of contents as visible', function () {
it('on a desktop-size screen, it should show the table of contents and make the main content pane focusable', function () {
// styles applied by this test simulate the styles media-queries will apply on real web pages
// by default, they show the table of contents

module = new GOVUK.Modules.TableOfContents()
module.start($toc)

expect($toc.attr('aria-hidden')).toEqual('false')
expect($mainContentPane.attr('tabindex')).toEqual('0')
})
})

Expand Down Expand Up @@ -156,10 +167,15 @@ describe('Table of contents', function () {

describe('if the open button is clicked', function () {
beforeEach(function () {
$html.addClass('mobile-size')
module = new GOVUK.Modules.TableOfContents()
module.start($toc)
})

afterEach(function () {
$html.removeClass('toc-open mobile-size')
})

it('the click event should be cancelled', function () {
var clickEvt = new $.Event('click')

Expand All @@ -168,7 +184,7 @@ describe('Table of contents', function () {
expect(clickEvt.isDefaultPrevented()).toBe(true)
})

it('the table of contents should show and be focused', function () {
it('the table of contents should show and be focused and the main content hidden', function () {
// detecting focus has proved unreliable so track calls to $toc.focus instead
var _focus = $.fn.focus
var tocFocusSpy = jasmine.createSpy('tocFocusSpy')
Expand All @@ -188,6 +204,7 @@ describe('Table of contents', function () {
$openButton.trigger(clickEvt)

expect($toc.attr('aria-hidden')).toEqual('false')
expect($mainContentPane.attr('aria-hidden')).toEqual('true')

expect(tocFocusSpy).toHaveBeenCalled()

Expand All @@ -197,7 +214,8 @@ describe('Table of contents', function () {
})

describe('if the close button is clicked', function () {
var clickEvt
var openClickEvt
var closeClickEvt

beforeEach(function () {
$html.addClass('mobile-size')
Expand All @@ -206,24 +224,35 @@ describe('Table of contents', function () {
module.start($toc)

// tocIsVisible = false // controls what $toc.is(':visible') returns, which will be controlled by CSS in a web page
clickEvt = new $.Event('click')
$closeButton.trigger(clickEvt)
openClickEvt = new $.Event('click')
closeClickEvt = new $.Event('click')

$openButton.trigger(openClickEvt)
$closeButton.trigger(closeClickEvt)
})

afterEach(function () {
$html.removeClass('mobile-size')
})

it('the click event should be cancelled', function () {
expect(clickEvt.isDefaultPrevented()).toBe(true)
expect(closeClickEvt.isDefaultPrevented()).toBe(true)
})

it('the table of contents should be hidden', function () {
expect($toc.attr('aria-hidden')).toEqual('true')
})

it('the button that triggered the dialog is refocused', function () {
expect(document.activeElement).toBe($openButton.get(0))
})

it('the main content area should be shown', function () {
expect($mainContentPane.attr('aria-hidden')).toEqual('false')
})
})

it('on mobile-size screens, when the table of contents is open and the escape key is activated, the table of contents should be hidden', function () {
it('on mobile-size screens, when the table of contents is open and the escape key is activated, the table of contents should be hidden and the main content shown', function () {
$html.addClass('mobile-size')

module = new GOVUK.Modules.TableOfContents()
Expand All @@ -236,6 +265,8 @@ describe('Table of contents', function () {
}))

expect($html.hasClass('toc-open')).toBe(false)
expect($toc.attr('aria-hidden')).toEqual('true')
expect($mainContentPane.attr('aria-hidden')).toEqual('false')

$html.removeClass('mobile-size')
})
Expand Down

0 comments on commit 58e7c76

Please sign in to comment.