diff --git a/docs/contributing/coding-standards/js.md b/docs/contributing/coding-standards/js.md index a319d57a9a..ee5b8e10b7 100644 --- a/docs/contributing/coding-standards/js.md +++ b/docs/contributing/coding-standards/js.md @@ -5,25 +5,53 @@ JavaScript files have the same name as the component's folder name. Test files have a `.test` suffix placed before the file extension. ``` -checkboxes -├── checkboxes.mjs -└── checkboxes.test.js +component +├── component.mjs +└── component.test.js ``` ## Skeleton ```js -import { nodeListForEach } from '../vendor/common.mjs' +import '../../vendor/polyfills/Element.mjs' -function Checkboxes ($module) { - // code goes here +/** + * Component name + * + * @class + * @param {Element} $module - HTML element to use for component + */ +function Example ($module) { + if (!$module) { + // Return instance for method chaining + // using `new Example($module).init()` + return this + } + + this.$module = $module + + // Code goes here } -Checkboxes.prototype.init = function () { - // code goes here +/** + * Initialise component + * + * @returns {Example} Example component + */ +Example.prototype.init = function () { + // Check that required elements are present + if (!this.$module) { + return this + } + + // Code goes here + + // Return instance for assignment + // `var myExample = new Example($module).init()` + return this } -export default Checkboxes +export default Example ``` ## Use data attributes to initialise component JavaScript @@ -48,15 +76,15 @@ Use `/** ... */` for multi-line comments. Include a description, and specify typ ```js /** -* Get the nearest ancestor element of a node that matches a given tag name -* @param {object} node element -* @param {string} match tag name (e.g. div) -* @return {object} ancestor element -*/ - -function (node, match) { - // code goes here - return ancestor + * Get the first descendent (child) of an HTML element that matches a given tag name + * + * @param {Element} $element - HTML element + * @param {string} tagName - Tag name (for example 'div') + * @returns {Element} Ancestor element + */ +function ($element, tagName) { + // Code goes here + return $element.querySelector(tagName) } ``` @@ -73,52 +101,54 @@ Use the prototype design pattern to structure your code. Create a constructor and define any variables that the object needs. ```js -function Checkboxes ($module) { - // code goes here +function Example ($module) { + // Code goes here } ``` Assign methods to the prototype object. Do not overwrite the prototype with a new object as this makes inheritance impossible. ```js -// bad -Checkboxes.prototype = { +// Bad +Example.prototype = { init: function () { - // code goes here + // Code goes here } } -// good -Checkboxes.prototype.init = function () { - // code goes here +// Good +Example.prototype.init = function () { + // Code goes here } ``` When initialising an object, use the `new` keyword. ```js -// bad -var myCheckbox = Checkbox().init() +// Bad +var myExample = Example().init() -// good -var myCheckbox = new Checkbox().init() +// Good +var myExample = new Example().init() ``` ## Modules -Use ES6 modules (`import`/`export`) over a non-standard module system. You can always transpile to your preferred module system. +Use ECMAScript modules (`import`/`export`) over CommonJS and other formats. You can always transpile to your preferred module system. ```js -import { nodeListForEach } from '../vendor/common.mjs' -// code goes here -export default Checkboxes +import { closestAttributeValue } from '../common/index.mjs' + +// Code goes here +export function exampleHelper1 () {} +export function exampleHelper2 () {} ``` -Avoid using wildcard (`import * as nodeListForEach`) imports. +You must specify the file extension when using the import keyword. -You must specify the file extension for a file when importing it. +Avoid using namespace imports (`import * as namespace`) in code transpiled to CommonJS (or AMD) bundled code as this can prevent "tree shaking" optimisations. -Use default export over named export. +Prefer named exports over default exports to avoid compatibility issues with transpiler "synthetic default" as discussed in: https://github.com/alphagov/govuk-frontend/issues/2829 ## Polyfilling diff --git a/src/govuk/all.mjs b/src/govuk/all.mjs index f90ce7c139..7c35ea3518 100644 --- a/src/govuk/all.mjs +++ b/src/govuk/all.mjs @@ -104,7 +104,7 @@ export { * Config for all components * * @typedef {object} Config - * @property {HTMLElement} [scope=document] - Scope to query for components + * @property {Element} [scope=document] - Scope to query for components * @property {import('./components/accordion/accordion.mjs').AccordionConfig} [accordion] - Accordion config * @property {import('./components/button/button.mjs').ButtonConfig} [button] - Button config * @property {import('./components/character-count/character-count.mjs').CharacterCountConfig} [characterCount] - Character Count config diff --git a/src/govuk/common/closest-attribute-value.mjs b/src/govuk/common/closest-attribute-value.mjs index 155b9f4f8f..b794c74cb9 100644 --- a/src/govuk/common/closest-attribute-value.mjs +++ b/src/govuk/common/closest-attribute-value.mjs @@ -3,7 +3,7 @@ import '../vendor/polyfills/Element/prototype/closest.mjs' /** * Returns the value of the given attribute closest to the given element (including itself) * - * @param {HTMLElement} $element - The element to start walking the DOM tree up + * @param {Element} $element - The element to start walking the DOM tree up * @param {string} attributeName - The name of the attribute * @returns {string | undefined} Attribute value */ diff --git a/src/govuk/common/index.mjs b/src/govuk/common/index.mjs index 1b54b08aeb..ed20a96116 100644 --- a/src/govuk/common/index.mjs +++ b/src/govuk/common/index.mjs @@ -134,10 +134,13 @@ export function extractConfigByNamespace (configObject, namespace) { if (!configObject || typeof configObject !== 'object') { throw new Error('Provide a `configObject` of type "object".') } + if (!namespace || typeof namespace !== 'string') { throw new Error('Provide a `namespace` of type "string" to filter the `configObject` by.') } + var newObject = {} + for (var key in configObject) { // Split the key into parts, using . as our namespace separator var keyParts = key.split('.') diff --git a/src/govuk/components/accordion/accordion.mjs b/src/govuk/components/accordion/accordion.mjs index 860c07cdcf..df8d32b9f7 100644 --- a/src/govuk/components/accordion/accordion.mjs +++ b/src/govuk/components/accordion/accordion.mjs @@ -38,10 +38,16 @@ var ACCORDION_TRANSLATIONS = { * attribute, which also provides accessibility. * * @class - * @param {HTMLElement} $module - HTML element to use for accordion + * @param {Element} $module - HTML element to use for accordion * @param {AccordionConfig} [config] - Accordion config */ function Accordion ($module, config) { + if (!$module) { + // Return instance for method chaining + // using `new Accordion($module).init()` + return this + } + this.$module = $module var defaultConfig = { @@ -79,17 +85,24 @@ function Accordion ($module, config) { this.sectionSummaryFocusClass = 'govuk-accordion__section-summary-focus' this.sectionContentClass = 'govuk-accordion__section-content' - this.$sections = this.$module.querySelectorAll('.' + this.sectionClass) + var $sections = this.$module.querySelectorAll('.' + this.sectionClass) + if (!$sections.length) { + return this + } + + this.$sections = $sections this.browserSupportsSessionStorage = helper.checkForSessionStorage() } /** * Initialise component + * + * @returns {Accordion} Accordion component */ Accordion.prototype.init = function () { - // Check for module - if (!this.$module) { - return + // Check that required elements are present + if (!this.$module || !this.$sections) { + return this } this.initControls() @@ -98,6 +111,10 @@ Accordion.prototype.init = function () { // See if "Show all sections" button text should be updated var areAllSectionsOpen = this.checkIfAllSectionsOpen() this.updateShowAllButton(areAllSectionsOpen) + + // Return instance for assignment + // `var myAccordion = new Accordion($module).init()` + return this } /** @@ -145,13 +162,17 @@ Accordion.prototype.initSectionHeaders = function () { /** * Loop through section headers * - * @param {HTMLElement} $section - Section element + * @param {Element} $section - Section element * @param {number} index - Section index * @this {Accordion} */ function ($section, index) { - // Set header attributes var $header = $section.querySelector('.' + this.sectionHeaderClass) + if (!$header) { + return + } + + // Set header attributes this.constructHeaderMarkup($header, index) this.setExpanded(this.isExpanded($section), $section) @@ -168,7 +189,7 @@ Accordion.prototype.initSectionHeaders = function () { /** * Construct section header * - * @param {HTMLElement} $header - Section header + * @param {Element} $header - Section header * @param {number} index - Section index */ Accordion.prototype.constructHeaderMarkup = function ($header, index) { @@ -176,6 +197,10 @@ Accordion.prototype.constructHeaderMarkup = function ($header, index) { var $heading = $header.querySelector('.' + this.sectionHeadingClass) var $summary = $header.querySelector('.' + this.sectionSummaryClass) + if (!$span || !$heading) { + return + } + // Create a button element that will replace the '.govuk-accordion__section-button' span var $button = document.createElement('button') $button.setAttribute('type', 'button') @@ -233,7 +258,7 @@ Accordion.prototype.constructHeaderMarkup = function ($header, index) { $button.appendChild(this.getButtonPunctuationEl()) // If summary content exists add to DOM in correct order - if (typeof ($summary) !== 'undefined' && $summary !== null) { + if ($summary) { // Create a new `span` element and copy the summary line content from the original `div` to the // new `span` // This is because the summary line text is now inside a button element, which can only contain @@ -273,7 +298,13 @@ Accordion.prototype.constructHeaderMarkup = function ($header, index) { * @param {Event} event - Generic event */ Accordion.prototype.onBeforeMatch = function (event) { - var $section = event.target.closest('.' + this.sectionClass) + var $fragment = event.target + if (!$fragment) { + return + } + + // Handle when fragment is inside section + var $section = $fragment.closest('.' + this.sectionClass) if ($section) { this.setExpanded(true, $section) } @@ -282,7 +313,7 @@ Accordion.prototype.onBeforeMatch = function (event) { /** * When section toggled, set and store state * - * @param {HTMLElement} $section - Section element + * @param {Element} $section - Section element */ Accordion.prototype.onSectionToggle = function ($section) { var expanded = this.isExpanded($section) @@ -305,7 +336,7 @@ Accordion.prototype.onShowOrHideAllToggle = function () { /** * Loop through section headers * - * @param {HTMLElement} $section - Section element + * @param {Element} $section - Section element * @this {Accordion} */ function ($section) { @@ -322,7 +353,7 @@ Accordion.prototype.onShowOrHideAllToggle = function () { * Set section attributes when opened/closed * * @param {boolean} expanded - Section expanded - * @param {HTMLElement} $section - Section element + * @param {Element} $section - Section element */ Accordion.prototype.setExpanded = function (expanded, $section) { var $showHideIcon = $section.querySelector('.' + this.upChevronIconClass) @@ -330,6 +361,13 @@ Accordion.prototype.setExpanded = function (expanded, $section) { var $button = $section.querySelector('.' + this.sectionButtonClass) var $content = $section.querySelector('.' + this.sectionContentClass) + if (!$showHideIcon || + !$showHideText || + !$button || + !$content) { + return + } + var newButtonText = expanded ? this.i18n.t('hideSection') : this.i18n.t('showSection') @@ -381,7 +419,7 @@ Accordion.prototype.setExpanded = function (expanded, $section) { /** * Get state of section * - * @param {HTMLElement} $section - Section element + * @param {Element} $section - Section element * @returns {boolean} True if expanded */ Accordion.prototype.isExpanded = function ($section) { @@ -447,7 +485,7 @@ var helper = { /** * Set the state of the accordions in sessionStorage * - * @param {HTMLElement} $section - Section element + * @param {Element} $section - Section element */ Accordion.prototype.storeState = function ($section) { if (this.browserSupportsSessionStorage) { @@ -471,7 +509,7 @@ Accordion.prototype.storeState = function ($section) { /** * Read the state of the accordions from sessionStorage * - * @param {HTMLElement} $section - Section element + * @param {Element} $section - Section element */ Accordion.prototype.setInitialState = function ($section) { if (this.browserSupportsSessionStorage) { @@ -495,7 +533,7 @@ Accordion.prototype.setInitialState = function ($section) { * into thematic chunks. * See https://github.com/alphagov/govuk-frontend/issues/2327#issuecomment-922957442 * - * @returns {HTMLElement} DOM element + * @returns {Element} DOM element */ Accordion.prototype.getButtonPunctuationEl = function () { var $punctuationEl = document.createElement('span') diff --git a/src/govuk/components/button/button.mjs b/src/govuk/components/button/button.mjs index 3f5cd09544..c66911e3ec 100644 --- a/src/govuk/components/button/button.mjs +++ b/src/govuk/components/button/button.mjs @@ -12,11 +12,13 @@ var DEBOUNCE_TIMEOUT_IN_SECONDS = 1 * JavaScript enhancements for the Button component * * @class - * @param {HTMLElement} $module - HTML element to use for button + * @param {Element} $module - HTML element to use for button * @param {ButtonConfig} [config] - Button config */ function Button ($module, config) { if (!$module) { + // Return instance for method chaining + // using `new Button($module).init()` return this } @@ -26,6 +28,7 @@ function Button ($module, config) { var defaultConfig = { preventDoubleClick: false } + this.config = mergeConfigs( defaultConfig, config || {}, @@ -35,14 +38,21 @@ function Button ($module, config) { /** * Initialise component + * + * @returns {Button} Button component */ Button.prototype.init = function () { + // Check that required elements are present if (!this.$module) { - return + return this } this.$module.addEventListener('keydown', this.handleKeyDown) this.$module.addEventListener('click', this.debounce.bind(this)) + + // Return instance for assignment + // `var myButton = new Button($module).init()` + return this } /** @@ -58,7 +68,13 @@ Button.prototype.init = function () { Button.prototype.handleKeyDown = function (event) { var $target = event.target - if ($target.getAttribute('role') === 'button' && event.keyCode === KEY_SPACE) { + // Handle space bar only + if (event.keyCode !== KEY_SPACE) { + return + } + + // Handle elements with [role="button"] only + if ($target.getAttribute('role') === 'button') { event.preventDefault() // prevent the page from scrolling $target.click() } diff --git a/src/govuk/components/character-count/character-count.mjs b/src/govuk/components/character-count/character-count.mjs index 6303004ca2..f28f361b83 100644 --- a/src/govuk/components/character-count/character-count.mjs +++ b/src/govuk/components/character-count/character-count.mjs @@ -53,11 +53,18 @@ var CHARACTER_COUNT_TRANSLATIONS = { * of the available characters/words has been entered. * * @class - * @param {HTMLElement} $module - HTML element to use for character count + * @param {Element} $module - HTML element to use for character count * @param {CharacterCountConfig} [config] - Character count config */ function CharacterCount ($module, config) { if (!$module) { + // Return instance for method chaining + // using `new CharacterCount($module).init()` + return this + } + + var $textarea = $module.querySelector('.govuk-js-character-count') + if (!$textarea) { return this } @@ -101,11 +108,11 @@ function CharacterCount ($module, config) { } else if ('maxlength' in this.config && this.config.maxlength) { this.maxLength = this.config.maxlength } else { - return + return this } this.$module = $module - this.$textarea = $module.querySelector('.govuk-js-character-count') + this.$textarea = $textarea this.$visibleCountMessage = null this.$screenReaderCountMessage = null @@ -115,15 +122,20 @@ function CharacterCount ($module, config) { /** * Initialise component + * + * @returns {CharacterCount} Character count component */ CharacterCount.prototype.init = function () { // Check that required elements are present - if (!this.$textarea) { - return + if (!this.$module || !this.$textarea) { + return this } var $textarea = this.$textarea var $textareaDescription = document.getElementById($textarea.id + '-info') + if (!$textareaDescription) { + return this + } // Inject a decription for the textarea if none is present already // for when the component was rendered with no maxlength, maxwords @@ -172,6 +184,10 @@ CharacterCount.prototype.init = function () { ) this.updateCountMessage() + + // Return instance for assignment + // `var myCharacterCount = new CharacterCount($module).init()` + return this } /** diff --git a/src/govuk/components/character-count/character-count.unit.test.mjs b/src/govuk/components/character-count/character-count.unit.test.mjs index 88aa6f92b4..5b75f2ab04 100644 --- a/src/govuk/components/character-count/character-count.unit.test.mjs +++ b/src/govuk/components/character-count/character-count.unit.test.mjs @@ -1,12 +1,25 @@ import CharacterCount from './character-count.mjs' describe('CharacterCount', () => { + let $container + let $textarea + + beforeAll(() => { + $container = document.createElement('div') + $textarea = document.createElement('textarea') + + // Component checks that required elements are present + $textarea.classList.add('govuk-js-character-count') + $container.appendChild($textarea) + }) + describe('formatCountMessage', () => { describe('default configuration', () => { let component + beforeAll(() => { - // The component won't initialise if we don't pass it an element - component = new CharacterCount(document.createElement('div')) + const $div = $container.cloneNode(true) + component = new CharacterCount($div) }) const cases = [ @@ -37,7 +50,8 @@ describe('CharacterCount', () => { describe('i18n', () => { describe('JavaScript configuration', () => { it('overrides the default translation keys', () => { - const component = new CharacterCount(document.createElement('div'), { + const $div = $container.cloneNode(true) + const component = new CharacterCount($div, { i18n: { charactersUnderLimit: { one: 'Custom text. Count: %{count}' } } }) @@ -47,7 +61,8 @@ describe('CharacterCount', () => { }) it('uses specific keys for when limit is reached', () => { - const component = new CharacterCount(document.createElement('div'), { + const $div = $container.cloneNode(true) + const component = new CharacterCount($div, { i18n: { charactersAtLimit: 'Custom text.', wordsAtLimit: 'Different custom text.' @@ -61,7 +76,7 @@ describe('CharacterCount', () => { describe('lang attribute configuration', () => { it('overrides the locale when set on the element', () => { - const $div = document.createElement('div') + const $div = $container.cloneNode(true) $div.setAttribute('lang', 'de') const component = new CharacterCount($div) @@ -73,7 +88,7 @@ describe('CharacterCount', () => { const $parent = document.createElement('div') $parent.setAttribute('lang', 'de') - const $div = document.createElement('div') + const $div = $container.cloneNode(true) $parent.appendChild($div) const component = new CharacterCount($div) @@ -84,7 +99,7 @@ describe('CharacterCount', () => { describe('Data attribute configuration', () => { it('overrides the default translation keys', () => { - const $div = document.createElement('div') + const $div = $container.cloneNode(true) $div.setAttribute('data-i18n.characters-under-limit.one', 'Custom text. Count: %{count}') const component = new CharacterCount($div) @@ -96,7 +111,7 @@ describe('CharacterCount', () => { describe('precedence over JavaScript configuration', () => { it('overrides translation keys', () => { - const $div = document.createElement('div') + const $div = $container.cloneNode(true) $div.setAttribute('data-i18n.characters-under-limit.one', 'Custom text. Count: %{count}') const component = new CharacterCount($div, { diff --git a/src/govuk/components/checkboxes/checkboxes.mjs b/src/govuk/components/checkboxes/checkboxes.mjs index 91b92bfd97..cf7a944e44 100644 --- a/src/govuk/components/checkboxes/checkboxes.mjs +++ b/src/govuk/components/checkboxes/checkboxes.mjs @@ -9,11 +9,22 @@ import '../../vendor/polyfills/Function/prototype/bind.mjs' * Checkboxes component * * @class - * @param {HTMLElement} $module - HTML element to use for checkboxes + * @param {Element} $module - HTML element to use for checkboxes */ function Checkboxes ($module) { + if (!$module) { + // Return instance for method chaining + // using `new Checkboxes($module).init()` + return this + } + + var $inputs = $module.querySelectorAll('input[type="checkbox"]') + if (!$inputs.length) { + return this + } + this.$module = $module - this.$inputs = $module.querySelectorAll('input[type="checkbox"]') + this.$inputs = $inputs } /** @@ -29,8 +40,15 @@ function Checkboxes ($module) { * We also need to restore the state of any conditional reveals on the page (for * example if the user has navigated back), and set up event handlers to keep * the reveal in sync with the checkbox state. + * + * @returns {Checkboxes} Checkboxes component */ Checkboxes.prototype.init = function () { + // Check that required elements are present + if (!this.$module || !this.$inputs) { + return this + } + var $module = this.$module var $inputs = this.$inputs @@ -74,6 +92,10 @@ Checkboxes.prototype.init = function () { // Handle events $module.addEventListener('click', this.handleClick.bind(this)) + + // Return instance for assignment + // `var myCheckboxes = new Checkboxes($module).init()` + return this } /** @@ -92,8 +114,12 @@ Checkboxes.prototype.syncAllConditionalReveals = function () { * @param {HTMLInputElement} $input - Checkbox input */ Checkboxes.prototype.syncConditionalRevealWithInputState = function ($input) { - var $target = document.getElementById($input.getAttribute('aria-controls')) + var targetId = $input.getAttribute('aria-controls') + if (!targetId) { + return + } + var $target = document.getElementById(targetId) if ($target && $target.classList.contains('govuk-checkboxes__conditional')) { var inputIsChecked = $input.checked @@ -108,7 +134,7 @@ Checkboxes.prototype.syncConditionalRevealWithInputState = function ($input) { * Find any other checkbox inputs with the same name value, and uncheck them. * This is useful for when a “None of these" checkbox is checked. * - * @param {HTMLElement} $input - Checkbox input + * @param {HTMLInputElement} $input - Checkbox input */ Checkboxes.prototype.unCheckAllInputsExcept = function ($input) { var allInputsWithSameName = document.querySelectorAll('input[type="checkbox"][name="' + $input.name + '"]') diff --git a/src/govuk/components/details/details.mjs b/src/govuk/components/details/details.mjs index e1f483a19e..a02e5f166f 100644 --- a/src/govuk/components/details/details.mjs +++ b/src/govuk/components/details/details.mjs @@ -17,28 +17,39 @@ var KEY_SPACE = 32 * Details component * * @class - * @param {HTMLElement} $module - HTML element to use for details + * @param {Element} $module - HTML element to use for details */ function Details ($module) { + if (!$module) { + // Return instance for method chaining + // using `new Details($module).init()` + return this + } + this.$module = $module } /** * Initialise component + * + * @returns {Details} Details component */ Details.prototype.init = function () { + // Check that required elements are present if (!this.$module) { - return + return this } // If there is native details support, we want to avoid running code to polyfill native behaviour. var hasNativeDetails = typeof this.$module.open === 'boolean' - if (hasNativeDetails) { - return + if (!hasNativeDetails) { + this.polyfillDetails() } - this.polyfillDetails() + // Return instance for assignment + // `var myDetails = new Details($module).init()` + return this } /** diff --git a/src/govuk/components/error-summary/error-summary.mjs b/src/govuk/components/error-summary/error-summary.mjs index 60d6c0b6e1..125a26041a 100644 --- a/src/govuk/components/error-summary/error-summary.mjs +++ b/src/govuk/components/error-summary/error-summary.mjs @@ -12,20 +12,13 @@ import '../../vendor/polyfills/Function/prototype/bind.mjs' * Takes focus on initialisation for accessible announcement, unless disabled in configuration. * * @class - * @param {HTMLElement} $module - HTML element to use for error summary + * @param {Element} $module - HTML element to use for error summary * @param {ErrorSummaryConfig} [config] - Error summary config */ function ErrorSummary ($module, config) { - // Some consuming code may not be passing a module, - // for example if they initialise the component - // on their own by directly passing the result - // of `document.querySelector`. - // To avoid breaking further JavaScript initialisation - // we need to safeguard against this so things keep - // working the same now we read the elements data attributes if (!$module) { - // Little safety in case code gets ported as-is - // into and ES6 class constructor, where the return value matters + // Return instance for method chaining + // using `new ErrorSummary($module).init()` return this } @@ -34,6 +27,7 @@ function ErrorSummary ($module, config) { var defaultConfig = { disableAutoFocus: false } + this.config = mergeConfigs( defaultConfig, config || {}, @@ -43,15 +37,23 @@ function ErrorSummary ($module, config) { /** * Initialise component + * + * @returns {ErrorSummary} Error summary component */ ErrorSummary.prototype.init = function () { - var $module = this.$module - if (!$module) { - return + // Check that required elements are present + if (!this.$module) { + return this } + var $module = this.$module + this.setFocus() $module.addEventListener('click', this.handleClick.bind(this)) + + // Return instance for assignment + // `var myErrorSummary = new ErrorSummary($module).init()` + return this } /** @@ -169,8 +171,8 @@ ErrorSummary.prototype.getFragmentFromUrl = function (url) { * - The first `