-
Notifications
You must be signed in to change notification settings - Fork 324
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
Improve accessibility of details component by polyfilling only where the native element is not available #1523
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
f5cfa5f
Only polyfill details component if native does not exist
NickColley 37508de
Restructure details make clear what is polyfill code
NickColley f389e0a
Add example with elements that are not native
NickColley 633fd0f
Add CHANGELOG entry
NickColley e4ed929
Test details polyfill is not run on native element
NickColley File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,56 @@ | ||
{% from "back-link/macro.njk" import govukBackLink %} | ||
|
||
{% extends "layout.njk" %} | ||
|
||
{% block beforeContent %} | ||
{{ govukBackLink({ | ||
"href": "/" | ||
}) }} | ||
{% endblock %} | ||
|
||
{% block content %} | ||
<div class="govuk-grid-row"> | ||
<div class="govuk-grid-column-two-thirds"> | ||
|
||
<h1 class="govuk-heading-xl"> | ||
Details polyfill examples | ||
</h1> | ||
|
||
<p class="govuk-body"> | ||
In order to test our polyfill code for details we need to have examples that do not use the native elements. | ||
</p> | ||
|
||
|
||
<h2 class="govuk-heading-m"> | ||
Default | ||
</h2> | ||
|
||
<div class="govuk-details" data-module="govuk-details" id="default"> | ||
<summary class="govuk-details__summary"> | ||
<span class="govuk-details__summary-text"> | ||
Help with nationality | ||
</span> | ||
</summary> | ||
<div class="govuk-details__text"> | ||
We need to know your nationality so we can work out which elections you’re entitled to vote in. If you cannot provide your nationality, you’ll have to send copies of identity documents through the post. | ||
</div> | ||
</div> | ||
|
||
<h2 class="govuk-heading-m"> | ||
Expanded | ||
</h2> | ||
|
||
<div class="govuk-details" data-module="govuk-details" open id="expanded"> | ||
<summary class="govuk-details__summary"> | ||
<span class="govuk-details__summary-text"> | ||
Help with nationality | ||
</span> | ||
</summary> | ||
<div class="govuk-details__text"> | ||
We need to know your nationality so we can work out which elections you’re entitled to vote in. If you cannot provide your nationality, you’ll have to send copies of identity documents through the post. | ||
</div> | ||
</div> | ||
|
||
</div> | ||
</div> | ||
{% endblock %} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,58 +11,28 @@ import { generateUniqueID } from '../../common.js' | |
var KEY_ENTER = 13 | ||
var KEY_SPACE = 32 | ||
|
||
// Create a flag to know if the browser supports navtive details | ||
var NATIVE_DETAILS = typeof document.createElement('details').open === 'boolean' | ||
|
||
function Details ($module) { | ||
this.$module = $module | ||
} | ||
|
||
/** | ||
* Handle cross-modal click events | ||
* @param {object} node element | ||
* @param {function} callback function | ||
*/ | ||
Details.prototype.handleInputs = function (node, callback) { | ||
node.addEventListener('keypress', function (event) { | ||
var target = event.target | ||
// When the key gets pressed - check if it is enter or space | ||
if (event.keyCode === KEY_ENTER || event.keyCode === KEY_SPACE) { | ||
if (target.nodeName.toLowerCase() === 'summary') { | ||
// Prevent space from scrolling the page | ||
// and enter from submitting a form | ||
event.preventDefault() | ||
// Click to let the click event do all the necessary action | ||
if (target.click) { | ||
target.click() | ||
} else { | ||
// except Safari 5.1 and under don't support .click() here | ||
callback(event) | ||
} | ||
} | ||
} | ||
}) | ||
Details.prototype.init = function () { | ||
if (!this.$module) { | ||
return | ||
} | ||
|
||
// Prevent keyup to prevent clicking twice in Firefox when using space key | ||
node.addEventListener('keyup', function (event) { | ||
var target = event.target | ||
if (event.keyCode === KEY_SPACE) { | ||
if (target.nodeName.toLowerCase() === 'summary') { | ||
event.preventDefault() | ||
} | ||
} | ||
}) | ||
// If there is native details support, we want to avoid running code to polyfill native behaviour. | ||
var hasNativeDetails = typeof this.$module.open === 'boolean' | ||
|
||
node.addEventListener('click', callback) | ||
if (hasNativeDetails) { | ||
return | ||
} | ||
|
||
this.polyfillDetails() | ||
} | ||
|
||
Details.prototype.init = function () { | ||
Details.prototype.polyfillDetails = function () { | ||
var $module = this.$module | ||
|
||
if (!$module) { | ||
return | ||
} | ||
|
||
// Save shortcuts to the inner summary and content elements | ||
var $summary = this.$summary = $module.getElementsByTagName('summary').item(0) | ||
var $content = this.$content = $module.getElementsByTagName('div').item(0) | ||
|
@@ -92,9 +62,7 @@ Details.prototype.init = function () { | |
// | ||
// We have to use the camelcase `tabIndex` property as there is a bug in IE6/IE7 when we set the correct attribute lowercase: | ||
// See http://web.archive.org/web/20170120194036/http://www.saliences.com/browserBugs/tabIndex.html for more information. | ||
if (!NATIVE_DETAILS) { | ||
$summary.tabIndex = 0 | ||
} | ||
$summary.tabIndex = 0 | ||
|
||
// Detect initial open state | ||
var openAttr = $module.getAttribute('open') !== null | ||
|
@@ -104,20 +72,18 @@ Details.prototype.init = function () { | |
} else { | ||
$summary.setAttribute('aria-expanded', 'false') | ||
$content.setAttribute('aria-hidden', 'true') | ||
if (!NATIVE_DETAILS) { | ||
$content.style.display = 'none' | ||
} | ||
$content.style.display = 'none' | ||
} | ||
|
||
// Bind an event to handle summary elements | ||
this.handleInputs($summary, this.setAttributes.bind(this)) | ||
this.polyfillHandleInputs($summary, this.polyfillSetAttributes.bind(this)) | ||
} | ||
|
||
/** | ||
* Define a statechange function that updates aria-expanded and style.display | ||
* @param {object} summary element | ||
*/ | ||
Details.prototype.setAttributes = function () { | ||
Details.prototype.polyfillSetAttributes = function () { | ||
var $module = this.$module | ||
var $summary = this.$summary | ||
var $content = this.$content | ||
|
@@ -128,27 +94,54 @@ Details.prototype.setAttributes = function () { | |
$summary.setAttribute('aria-expanded', (expanded ? 'false' : 'true')) | ||
$content.setAttribute('aria-hidden', (hidden ? 'false' : 'true')) | ||
|
||
if (!NATIVE_DETAILS) { | ||
$content.style.display = (expanded ? 'none' : '') | ||
$content.style.display = (expanded ? 'none' : '') | ||
|
||
var hasOpenAttr = $module.getAttribute('open') !== null | ||
if (!hasOpenAttr) { | ||
$module.setAttribute('open', 'open') | ||
} else { | ||
$module.removeAttribute('open') | ||
} | ||
var hasOpenAttr = $module.getAttribute('open') !== null | ||
if (!hasOpenAttr) { | ||
$module.setAttribute('open', 'open') | ||
} else { | ||
$module.removeAttribute('open') | ||
} | ||
|
||
return true | ||
} | ||
|
||
/** | ||
* Remove the click event from the node element | ||
* Handle cross-modal click events | ||
* @param {object} node element | ||
* @param {function} callback function | ||
*/ | ||
Details.prototype.destroy = function (node) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also removes destroy as it doesnt seem functional? |
||
node.removeEventListener('keypress') | ||
node.removeEventListener('keyup') | ||
node.removeEventListener('click') | ||
Details.prototype.polyfillHandleInputs = function (node, callback) { | ||
node.addEventListener('keypress', function (event) { | ||
var target = event.target | ||
// When the key gets pressed - check if it is enter or space | ||
if (event.keyCode === KEY_ENTER || event.keyCode === KEY_SPACE) { | ||
if (target.nodeName.toLowerCase() === 'summary') { | ||
// Prevent space from scrolling the page | ||
// and enter from submitting a form | ||
event.preventDefault() | ||
// Click to let the click event do all the necessary action | ||
if (target.click) { | ||
target.click() | ||
} else { | ||
// except Safari 5.1 and under don't support .click() here | ||
callback(event) | ||
} | ||
} | ||
} | ||
}) | ||
|
||
// Prevent keyup to prevent clicking twice in Firefox when using space key | ||
node.addEventListener('keyup', function (event) { | ||
var target = event.target | ||
if (event.keyCode === KEY_SPACE) { | ||
if (target.nodeName.toLowerCase() === 'summary') { | ||
event.preventDefault() | ||
} | ||
} | ||
}) | ||
|
||
node.addEventListener('click', callback) | ||
} | ||
|
||
export default Details |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do wonder what the benefit is of adding the
open
attribute to a non-nativedetails
element. It seems a bit strange, but I guess it might help some AT announce the state? 🤷♂There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose if people have styled using the 'open' attribute?
details[open] p { color: red; }