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

Add details.polyfill.js #426

Merged
merged 6 commits into from
Oct 3, 2017
Merged
Show file tree
Hide file tree
Changes from 4 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
240 changes: 240 additions & 0 deletions javascripts/govuk/details.polyfill.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,240 @@
// <details> polyfill
// http://caniuse.com/#feat=details

// FF Support for HTML5's <details> and <summary>
// https://bugzilla.mozilla.org/show_bug.cgi?id=591737

// http://www.sitepoint.com/fixing-the-details-element/

;(function (global) {
'use strict'

var GOVUK = global.GOVUK || {}

GOVUK.details = {
NATIVE_DETAILS: typeof document.createElement('details').open === 'boolean',
KEY_ENTER: 13,
KEY_SPACE: 32,

// Create a started flag so we can prevent the initialisation
// function firing from both DOMContentLoaded and window.onload
started: false,

// Add event construct for modern browsers or IE
// which fires the callback with a pre-converted target reference
addEvent: function (node, type, callback) {
if (node.addEventListener) {
node.addEventListener(type, function (e) {
callback(e, e.target)
}, false)
} else if (node.attachEvent) {
node.attachEvent('on' + type, function (e) {
callback(e, e.srcElement)
})
}
},

removeEvent: function (node, type) {
if (node.removeEventListener) {
node.removeEventListener(type, function (e) {
}, false)
} else if (node.detachEvent) {
node.detachEvent('on' + type, function (e) {
})
}
},

// Cross-browser character code / key pressed
charCode: function (e) {
return (typeof e.which === 'number') ? e.which : e.keyCode
},

// Cross-browser preventing default action
preventDefault: function (e) {
if (e.preventDefault) {
e.preventDefault()
} else {
e.returnValue = false
}
},

// Handle cross-modal click events
addClickEvent: function (node, callback) {
GOVUK.details.addEvent(node, 'keypress', function (e, target) {
// When the key gets pressed - check if it is enter or space
if (GOVUK.details.charCode(e) === GOVUK.details.KEY_ENTER || GOVUK.details.charCode(e) === GOVUK.details.KEY_SPACE) {
if (target.nodeName.toLowerCase() === 'summary') {
// Prevent space from scrolling the page
// and enter from submitting a form
GOVUK.details.preventDefault(e)
// 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(e, target)
}
}
}
})

// Prevent keyup to prevent clicking twice in Firefox when using space key
GOVUK.details.addEvent(node, 'keyup', function (e, target) {
if (GOVUK.details.charCode(e) === GOVUK.details.KEY_SPACE) {
if (target.nodeName === 'SUMMARY') {
GOVUK.details.preventDefault(e)
}
}
})

GOVUK.details.addEvent(node, 'click', function (e, target) {
callback(e, target)
})
},

// Get the nearest ancestor element of a node that matches a given tag name
getAncestor: function (node, match) {
do {
if (!node || node.nodeName.toLowerCase() === match) {
break
}
node = node.parentNode
} while (node)

return node
},

// Initialisation function
addDetailsPolyfill: function (list, container) {
container = container || document.body
// If this has already happened, just return
// else set the flag so it doesn't happen again
if (GOVUK.details.started) {
return
}
GOVUK.details.started = true
// Get the collection of details elements, but if that's empty
// then we don't need to bother with the rest of the scripting
if ((list = container.getElementsByTagName('details')).length === 0) {
return
}
// else iterate through them to apply their initial state
var n = list.length
var i = 0
for (i; i < n; i++) {
var details = list[i]

// Save shortcuts to the inner summary and content elements
details.__summary = details.getElementsByTagName('summary').item(0)
details.__content = details.getElementsByTagName('div').item(0)

if (!details.__summary || !details.__content) {
return
}
// If the content doesn't have an ID, assign it one now
// which we'll need for the summary's aria-controls assignment
if (!details.__content.id) {
details.__content.id = 'details-content-' + i
}

// Add ARIA role="group" to details
details.setAttribute('role', 'group')

// Add role=button to summary
details.__summary.setAttribute('role', 'button')

// Add aria-controls
details.__summary.setAttribute('aria-controls', details.__content.id)

// Set tabIndex so the summary is keyboard accessible for non-native elements
// http://www.saliences.com/browserBugs/tabIndex.html
if (!GOVUK.details.NATIVE_DETAILS) {
details.__summary.tabIndex = 0
}

// Detect initial open state
var openAttr = details.getAttribute('open') !== null
if (openAttr === true) {
details.__summary.setAttribute('aria-expanded', 'true')
details.__content.setAttribute('aria-hidden', 'false')
} else {
details.__summary.setAttribute('aria-expanded', 'false')
details.__content.setAttribute('aria-hidden', 'true')
if (!GOVUK.details.NATIVE_DETAILS) {
details.__content.style.display = 'none'
}
}

// Create a circular reference from the summary back to its
// parent details element, for convenience in the click handler
details.__summary.__details = details

// If this is not a native implementation, create an arrow
// inside the summary
if (!GOVUK.details.NATIVE_DETAILS) {
var twisty = document.createElement('i')

if (openAttr === true) {
twisty.className = 'arrow arrow-open'
twisty.appendChild(document.createTextNode('\u25bc'))
} else {
twisty.className = 'arrow arrow-closed'
twisty.appendChild(document.createTextNode('\u25ba'))
}

details.__summary.__twisty = details.__summary.insertBefore(twisty, details.__summary.firstChild)
details.__summary.__twisty.setAttribute('aria-hidden', 'true')
}
}

// Bind a click event to handle summary elements
GOVUK.details.addClickEvent(container, function (e, summary) {
if (!(summary = GOVUK.details.getAncestor(summary, 'summary'))) {
return true
}
return GOVUK.details.statechange(summary)
})
},

// Define a statechange function that updates aria-expanded and style.display
// Also update the arrow position
statechange: function (summary) {
var expanded = summary.__details.__summary.getAttribute('aria-expanded') === 'true'
var hidden = summary.__details.__content.getAttribute('aria-hidden') === 'true'

summary.__details.__summary.setAttribute('aria-expanded', (expanded ? 'false' : 'true'))
summary.__details.__content.setAttribute('aria-hidden', (hidden ? 'false' : 'true'))

if (!GOVUK.details.NATIVE_DETAILS) {
summary.__details.__content.style.display = (expanded ? 'none' : '')

var hasOpenAttr = summary.__details.getAttribute('open') !== null
if (!hasOpenAttr) {
summary.__details.setAttribute('open', 'open')
} else {
summary.__details.removeAttribute('open')
}
}

if (summary.__twisty) {
summary.__twisty.firstChild.nodeValue = (expanded ? '\u25ba' : '\u25bc')
summary.__twisty.setAttribute('class', (expanded ? 'arrow arrow-closed' : 'arrow arrow-open'))
}

return true
},

destroy: function (node) {
GOVUK.details.removeEvent(node, 'click')
},

// Bind two load events for modern and older browsers
// If the first one fires it will set a flag to block the second one
// but if it's not supported then the second one will fire
init: function ($container) {
GOVUK.details.addEvent(document, 'DOMContentLoaded', GOVUK.details.addDetailsPolyfill)
GOVUK.details.addEvent(window, 'load', GOVUK.details.addDetailsPolyfill)
}
}
global.GOVUK = GOVUK
})(window)
2 changes: 2 additions & 0 deletions spec/manifest.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
var manifest = {
support: [
'../../node_modules/jquery/dist/jquery.js',
'../../javascripts/govuk/details.polyfill.js',
'../../javascripts/govuk/modules.js',
'../../javascripts/govuk/modules/auto-track-event.js',
'../../javascripts/govuk/primary-links.js',
Expand All @@ -19,6 +20,7 @@ var manifest = {
'../../javascripts/govuk/analytics/mailto-link-tracker.js'
],
test: [
'../unit/details.polyfill.spec.js',
'../unit/modules.spec.js',
'../unit/Modules/auto-track-event.spec.js',
'../unit/primary-links.spec.js',
Expand Down
78 changes: 78 additions & 0 deletions spec/unit/details.polyfill.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
/* global describe it expect beforeEach afterEach */

var $ = window.jQuery

describe('details-polyfill', function () {
'use strict'
var GOVUK = window.GOVUK

beforeEach(function (done) {
// Sample markup
this.$content = $(
'<details>' +
'<summary><span class="summary">Summary</span></summary>' +
'<div><p>Hidden content</p></div>' +
'</details>'
)

// Find elements
var $summaries = this.$content.find('summary')
var $hiddenContent = this.$content.find('div')

this.$summary1 = $summaries.eq(0)
this.$hiddenContent1 = $hiddenContent.eq(0)

// Add to page
$(document.body).append(this.$content)

setTimeout(function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be good to add a comment as to why this is needed.

Copy link
Contributor Author

@alex-ju alex-ju Sep 28, 2017

Choose a reason for hiding this comment

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

Essentially I needed to do asynchronous testing because of how the events are being chained.
I can avoid doing so, but I have to do more rewriting on the polyfill first. Let me know if you want me to try and get rid of the async test.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

done()
}, 1)
})

afterEach(function () {
this.detailsPolyfill = null
this.$content.remove()
})

describe('when details element follows the specified structure', function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

I haven't seen describe blocks used in this nested manner before – I guess it's just trying to describe the context, but I'm not convinced it's required. Most of it seems pretty self explanatory. Are there other benefits to using them that I'm not understanding?

Copy link
Contributor Author

@alex-ju alex-ju Sep 28, 2017

Choose a reason for hiding this comment

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

I'm not used with this way of nesting descriptions, I just followed other tests' structure in the frontend_toolkit. I can flatten them down.

Copy link
Contributor

Choose a reason for hiding this comment

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

Aah, sorry – for some reason I didn't even think to look at how the other specs were written. If we're broadly following the conventions of the other specs, let's leave them as they are and re-visit this when we look at moving this into Frontend.

describe('and when details.polyfill is initialised', function () {
describe('and when we have native details support', function () {
Copy link
Contributor

Choose a reason for hiding this comment

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

When we don't have native support? Surely if we have native support this doesn't do anything?

beforeEach(function () {
// Add show/hide content support
this.detailsPolyfill = GOVUK.details.addDetailsPolyfill()
GOVUK.details.started = false
})
it('should add the aria attributes to summary', function () {
expect(this.$summary1.attr('role')).toBe('button')
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we try and have one expectation per it block? For example 'it should add the button role to the summary', 'it should associate the summary with the content using aria-controls', 'it should indicate the collapsed state using aria-expanded'

expect(this.$summary1.attr('aria-controls')).toBe('details-content-0')
expect(this.$summary1.attr('aria-expanded')).toBe('false')
})

it('should add the aria attributes to hidden content', function () {
expect(this.$hiddenContent1).toBeTruthy()
expect(this.$hiddenContent1.attr('id')).toBe('details-content-0')
expect(this.$hiddenContent1.attr('aria-hidden')).toBe('true')
})

it('should hide the hidden content visually', function () {
expect(this.$hiddenContent1.is(':visible')).toBe(false)
})

it('should make the hidden content visible if its summary is clicked', function (done) {
// Initialise again
this.detailsPolyfill = GOVUK.details.addDetailsPolyfill()
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this needed?


// Trigger click on summary
this.$summary1.click()
expect(this.$content.attr('open')).toBe('open')
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be good to rework this to have one expectation per it block as well. Perhaps

describe: when the summary is clicked
it: should make the content visible
it: should indicate the expanded state of the summary using aria-expanded
it: should indicate the visible state of the content using aria-hidden

expect(this.$summary1.attr('aria-expanded')).toBe('true')
expect(this.$hiddenContent1.attr('aria-hidden')).toBe('false')
expect(this.$hiddenContent1.is(':visible')).toBe(true)

done()
})
})
})
})
})