Merge pull request #5043 from alphagov/accordion-tidy

Refactor the accordion JavaScript
alphagov · Jun 11, 2024 · 4c5806e · 4c5806e
2 parents be1c473 + bed231c
commit 4c5806e
Show file tree

Hide file tree

Showing 2 changed files with 33 additions and 32 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -4,6 +4,12 @@ For advice on how to use these release notes see [our guidance on staying up to
 
 ## Unreleased
 
+### Fixes
+
+We've made fixes to GOV.UK Frontend in the following pull requests:
+
+- [#5043: Refactor the accordion JavaScript](https://github.com/alphagov/govuk-frontend/pull/5043)
+
 ## 5.4.0 (Feature release)
 
 To install this version with npm, run `npm install govuk-frontend@5.4.0`. You can also find more information about [how to stay up to date](https://frontend.design-system.service.gov.uk/staying-up-to-date/#updating-to-the-latest-version) in our documentation.

diff --git a/packages/govuk-frontend/src/govuk/components/accordion/accordion.mjs b/packages/govuk-frontend/src/govuk/components/accordion/accordion.mjs
@@ -151,9 +151,7 @@ export class Accordion extends GOVUKFrontendComponent {
     this.initControls()
     this.initSectionHeaders()
 
-    // See if "Show all sections" button text should be updated
-    const areAllSectionsOpen = this.checkIfAllSectionsOpen()
-    this.updateShowAllButton(areAllSectionsOpen)
+    this.updateShowAllButton(this.areAllSectionsOpen())
   }
 
   /**
@@ -263,8 +261,8 @@ export class Accordion extends GOVUKFrontendComponent {
     // Copy all attributes from $span to $button (except `id`, which gets added
     // to the `$headingText` element)
     for (const attr of Array.from($span.attributes)) {
-      if (attr.nodeName !== 'id') {
-        $button.setAttribute(attr.nodeName, `${attr.nodeValue}`)
+      if (attr.name !== 'id') {
+        $button.setAttribute(attr.name, attr.value)
       }
     }
 
@@ -282,7 +280,9 @@ export class Accordion extends GOVUKFrontendComponent {
     $headingText.appendChild($headingTextFocus)
     // span could contain HTML elements
     // (see https://www.w3.org/TR/2011/WD-html5-20110525/content-models.html#phrasing-content)
-    $headingTextFocus.innerHTML = $span.innerHTML
+    Array.from($span.childNodes).forEach(($child) =>
+      $headingTextFocus.appendChild($child)
+    )
 
     // Create container for show / hide icons and text.
     const $showHideToggle = document.createElement('span')
@@ -312,7 +312,7 @@ export class Accordion extends GOVUKFrontendComponent {
     $button.appendChild(this.getButtonPunctuationEl())
 
     // If summary content exists add to DOM in correct order
-    if ($summary?.parentNode) {
+    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 phrasing
@@ -326,14 +326,16 @@ export class Accordion extends GOVUKFrontendComponent {
 
       // Get original attributes, and pass them to the replacement
       for (const attr of Array.from($summary.attributes)) {
-        $summarySpan.setAttribute(attr.nodeName, `${attr.nodeValue}`)
+        $summarySpan.setAttribute(attr.name, attr.value)
       }
 
       // Copy original contents of summary to the new summary span
-      $summarySpanFocus.innerHTML = $summary.innerHTML
+      Array.from($summary.childNodes).forEach(($child) =>
+        $summarySpanFocus.appendChild($child)
+      )
 
       // Replace the original summary `div` with the new summary `span`
-      $summary.parentNode.replaceChild($summarySpan, $summary)
+      $summary.remove()
 
       $button.appendChild($summarySpan)
       $button.appendChild(this.getButtonPunctuationEl())
@@ -373,11 +375,11 @@ export class Accordion extends GOVUKFrontendComponent {
    * @param {Element} $section - Section element
    */
   onSectionToggle($section) {
-    const expanded = this.isExpanded($section)
-    this.setExpanded(!expanded, $section)
+    const nowExpanded = !this.isExpanded($section)
+    this.setExpanded(nowExpanded, $section)
 
     // Store the state in sessionStorage when a change is triggered
-    this.storeState($section)
+    this.storeState($section, nowExpanded)
   }
 
   /**
@@ -386,11 +388,11 @@ export class Accordion extends GOVUKFrontendComponent {
    * @private
    */
   onShowOrHideAllToggle() {
-    const nowExpanded = !this.checkIfAllSectionsOpen()
+    const nowExpanded = !this.areAllSectionsOpen()
 
     this.$sections.forEach(($section) => {
       this.setExpanded(nowExpanded, $section)
-      this.storeState($section)
+      this.storeState($section, nowExpanded)
     })
 
     this.updateShowAllButton(nowExpanded)
@@ -469,8 +471,7 @@ export class Accordion extends GOVUKFrontendComponent {
     }
 
     // See if "Show all sections" button text should be updated
-    const areAllSectionsOpen = this.checkIfAllSectionsOpen()
-    this.updateShowAllButton(areAllSectionsOpen)
+    this.updateShowAllButton(this.areAllSectionsOpen())
   }
 
   /**
@@ -490,14 +491,10 @@ export class Accordion extends GOVUKFrontendComponent {
    * @private
    * @returns {boolean} True if all sections are open
    */
-  checkIfAllSectionsOpen() {
-    const sectionsCount = this.$sections.length
-    const expandedSectionCount = this.$module.querySelectorAll(
-      `.${this.sectionExpandedClass}`
-    ).length
-    const areAllSectionsOpen = sectionsCount === expandedSectionCount
-
-    return areAllSectionsOpen
+  areAllSectionsOpen() {
+    return Array.from(this.$sections).every(($section) =>
+      this.isExpanded($section)
+    )
   }
 
   /**
@@ -523,8 +520,9 @@ export class Accordion extends GOVUKFrontendComponent {
    *
    * @private
    * @param {Element} $section - Section element
+   * @param {boolean} isExpanded - Whether the section is expanded
    */
-  storeState($section) {
+  storeState($section, isExpanded) {
     if (this.browserSupportsSessionStorage && this.config.rememberExpanded) {
       // We need a unique way of identifying each content in the Accordion.
       // Since an `#id` should be unique and an `id` is required for `aria-`
@@ -533,12 +531,9 @@ export class Accordion extends GOVUKFrontendComponent {
 
       if ($button) {
         const contentId = $button.getAttribute('aria-controls')
-        const contentState = $button.getAttribute('aria-expanded')
 
-        // Only set the state when both `contentId` and `contentState` are taken
-        // from the DOM.
-        if (contentId && contentState) {
-          window.sessionStorage.setItem(contentId, contentState)
+        if (contentId) {
+          window.sessionStorage.setItem(contentId, isExpanded.toString())
         }
       }
     }
@@ -584,7 +579,7 @@ export class Accordion extends GOVUKFrontendComponent {
       'govuk-visually-hidden',
       this.sectionHeadingDividerClass
     )
-    $punctuationEl.innerHTML = ', '
+    $punctuationEl.textContent = ', '
     return $punctuationEl
   }