Skip to content

Commit 6c61aa5

Browse files
committed
Replace calls to appendChild with new option in createElement
Allows regrouping code for creating components a little more, as well as getting rid of a couple of loops over child nodes
1 parent 4d2686f commit 6c61aa5

File tree

2 files changed

+56
-48
lines changed

2 files changed

+56
-48
lines changed

packages/govuk-frontend/src/govuk/common/create-element.mjs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,13 +5,20 @@
55
* @template {keyof HTMLElementTagNameMap} TagName
66
* @param {TagName} tagName - Type of element to create
77
* @param {{[key: string]: string}} [attributes] - Attributes to set on the element
8+
* @param {Node[]} [children] - The list of child nodes for the element
89
* @returns {HTMLElementTagNameMap[TagName]} Created element
910
*/
10-
export function createElement(tagName, attributes = {}) {
11+
export function createElement(tagName, attributes = {}, children) {
1112
const el = document.createElement(tagName)
1213
Object.entries(attributes).forEach(([name, value]) => {
1314
el.setAttribute(name, value)
1415
})
1516

17+
if (children) {
18+
for (const child of children) {
19+
el.appendChild(child)
20+
}
21+
}
22+
1623
return el
1724
}

packages/govuk-frontend/src/govuk/components/accordion/accordion.mjs

Lines changed: 48 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -245,30 +245,32 @@ export class Accordion extends ConfigurableComponent {
245245
*/
246246
createShowHideToggle() {
247247
// Create an inner container to limit the width of the focus state
248-
const $showHideToggleFocus = createElement('span', {
249-
class: 'govuk-accordion__section-toggle-focus'
250-
})
251-
252-
$showHideToggleFocus.appendChild(
253-
createElement('span', {
254-
class: iconClass
255-
})
256-
)
257-
$showHideToggleFocus.appendChild(
258-
createElement('span', {
259-
class: sectionToggleTextClass
260-
})
248+
const $showHideToggleFocus = createElement(
249+
'span',
250+
{
251+
class: 'govuk-accordion__section-toggle-focus'
252+
},
253+
[
254+
createElement('span', {
255+
class: iconClass
256+
}),
257+
createElement('span', {
258+
class: sectionToggleTextClass
259+
})
260+
]
261261
)
262262

263-
const $showHideToggle = createElement('span', {
264-
class: 'govuk-accordion__section-toggle',
265-
// Tell Google not to index the 'show' text as part of the heading. Must be
266-
// set on the element before it's added to the DOM.
267-
// See https://developers.google.com/search/docs/advanced/robots/robots_meta_tag#data-nosnippet-attr
268-
'data-nosnippet': ''
269-
})
270-
271-
$showHideToggle.appendChild($showHideToggleFocus)
263+
const $showHideToggle = createElement(
264+
'span',
265+
{
266+
class: 'govuk-accordion__section-toggle',
267+
// Tell Google not to index the 'show' text as part of the heading. Must be
268+
// set on the element before it's added to the DOM.
269+
// See https://developers.google.com/search/docs/advanced/robots/robots_meta_tag#data-nosnippet-attr
270+
'data-nosnippet': ''
271+
},
272+
[$showHideToggleFocus]
273+
)
272274

273275
return $showHideToggle
274276
}
@@ -282,23 +284,25 @@ export class Accordion extends ConfigurableComponent {
282284
createHeadingText($span) {
283285
// Create an inner heading text container to limit the width of the focus
284286
// state
285-
const $headingTextFocus = createElement('span', {
286-
class: 'govuk-accordion__section-heading-text-focus'
287-
})
288-
289-
// span could contain HTML elements which need moving to the new span
290-
// (see https://www.w3.org/TR/2011/WD-html5-20110525/content-models.html#phrasing-content)
291-
Array.from($span.childNodes).forEach(($child) =>
292-
$headingTextFocus.appendChild($child)
287+
const $headingTextFocus = createElement(
288+
'span',
289+
{
290+
class: 'govuk-accordion__section-heading-text-focus'
291+
},
292+
// span could contain HTML elements which need moving to the new span
293+
// (see https://www.w3.org/TR/2011/WD-html5-20110525/content-models.html#phrasing-content)
294+
Array.from($span.childNodes)
293295
)
294296

295297
// Create container for heading text so it can be styled
296-
const $headingText = createElement('span', {
297-
class: sectionHeadingTextClass,
298-
id: $span.id
299-
})
300-
301-
$headingText.appendChild($headingTextFocus)
298+
const $headingText = createElement(
299+
'span',
300+
{
301+
class: sectionHeadingTextClass,
302+
id: $span.id
303+
},
304+
[$headingTextFocus]
305+
)
302306

303307
return $headingText
304308
}
@@ -316,24 +320,21 @@ export class Accordion extends ConfigurableComponent {
316320
createSummarySpan($summary) {
317321
// Create an inner summary container to limit the width of the summary
318322
// focus state
319-
const $summarySpanFocus = createElement('span', {
320-
class: 'govuk-accordion__section-summary-focus'
321-
})
322-
323-
const $summarySpan = createElement('span')
323+
const $summarySpanFocus = createElement(
324+
'span',
325+
{
326+
class: 'govuk-accordion__section-summary-focus'
327+
},
328+
Array.from($summary.childNodes)
329+
)
324330

325-
$summarySpan.appendChild($summarySpanFocus)
331+
const $summarySpan = createElement('span', {}, [$summarySpanFocus])
326332

327333
// Get original attributes, and pass them to the replacement
328334
for (const attr of Array.from($summary.attributes)) {
329335
$summarySpan.setAttribute(attr.name, attr.value)
330336
}
331337

332-
// Copy original contents of summary to the new summary span
333-
Array.from($summary.childNodes).forEach(($child) =>
334-
$summarySpanFocus.appendChild($child)
335-
)
336-
337338
return $summarySpan
338339
}
339340

0 commit comments

Comments
 (0)