🛠 Accordion: Refactor

4m:6s

Here’s the code we wrote for the accordion.

const accordionContainer = document.querySelector('.accordion-container')

accordionContainer.addEventListener('click', event => {
  const accordionHeader = event.target.closest('.accordion__header')
  if (!accordionHeader) return

  const accordion = accordionHeader.parentElement
  const accordionContent = accordionHeader.nextElementSibling
  const accordionInner = accordionContent.children[0]
  const height = accordion.classList.contains('is-open')
    ? 0
    : accordionInner.getBoundingClientRect().height

  accordion.classList.toggle('is-open')
  accordionContent.style.height = `${height}px`
})

We can use two best practices to refactor the accordion component. They are:

Write declarative code
Write functions with a single purpose

Let’s get started.

First two lines in the event listener

The first two lines of code in the event listener tell us whether we want the event listener to do anything. We should not move them away.

accordionContainer.addEventListener('click', event => {
  // These will stay in the event listener
  const accordionHeader = event.target.closest('.accordion__header')
  if (!accordionHeader) return
  // ...
})

Get Content Height

Three of the next four lines of code are used to get the .accordion__inner’s height.

accordionContainer.addEventListener('click', event => {
  // ...
  const accordion = accordionHeader.parentElement

  // These three lines are used to get content's height
  const accordionContent = accordionHeader.nextElementSibling
  const accordionInner = accordionContent.children[0]
  const height = accordion.classList.contains('is-open')
    ? 0
    : accordionInner.getBoundingClientRect().height
  // ...
})

This code is imperative. You have to go through it to understand what it does. We can make the code declarative by creating a function.

Let’s call this function getContentHeight. The name makes sense because we want to know what height should accordionContent be.

function getContentHeight() {
  // ...
}

First, you copy-paste everything into getContentHeight.

function getContentHeight() {
  const accordionContent = accordionHeader.nextElementSibling
  const accordionInner = accordionContent.children[0]
  const height = accordion.classList.contains('is-open')
    ? 0
    : accordionInner.getBoundingClientRect().height
}

When we call getContentHeight, we expect to get a height value. To get a height value, we need to return height from getContentHeight.

function getContentHeight() {
  const accordionContent = accordionHeader.nextElementSibling
  const accordionInner = accordionContent.children[0]
  return accordion.classList.contains('is-open')
    ? 0
    : accordionInner.getBoundingClientRect().height
}

Now, we can see getContentHeight requires two variables:

accordionHeader. This is used to get accordionContent and then accordionInner.
accordion. We used this to check whether the accordion is opened.

One way to write getContentHeight is to pass both variables in:

function getContentHeight(accordion, accordionHeader) {
  // ...
}

This is not the best way. Why? Look at the code again. We can get accordionInner from accordion directly. There’s no need to go through accordionContent and accordionHeader.

Here’s the refactored version:

function getContentHeight(accordion) {
  const accordionInner = accordion.querySelector('.accordion__inner')

  return accordion.classList.contains('is-open')
    ? 0
    : accordionInner.getBoundingClientRect().height
}

When I create dedicated functions like getContentHeight, I prefer to use if statements and early returns over ternary operators. For me, this makes code easier to read.

function getContentHeight(accordion) {
  const accordionInner = accordion.querySelector('.accordion__inner')

  if (accordion.classList.contains('is-open')) return 0
  return accordionInner.getBoundingClientRect().height
}

Finally, remember to document the function. (I’m going to stop documenting functions in these lessons. You can refer to the files for my actual comments).

/**
 * Finds the correct height of the accordion content
 * @param {HTMLElement} accordion The accordion
 * @returns {Number} Accordion content's height in px value
 */
function getContentHeight(accordion) {
  const accordionInner = accordion.querySelector('.accordion__inner')

  if (accordion.classList.contains('is-open')) return 0
  return accordionInner.getBoundingClientRect().height
}

Using getContentHeight:

accordionContainer.addEventListener('click', event => {
  // ...
  const accordion = accordionHeader.parentElement
  const height = getContentHeight(accordion)
  // ...
})

Last two lines

We used the last two lines of code to update the accordion.

accordionContainer.addEventListener('click', event => {
  // ...
  accordion.classList.toggle('is-open')
  accordionContent.style.height = `${height}px`
})

We can put them in a function that says updateAccordion.

function updateAccordion() {
  accordion.classList.toggle('is-open')
  accordionContent.style.height = `${height}px`
}

At first glance, updateAccordion needs three things:

The accordion to update.
The accordionContent to update.
The height of the content.

But we don’t need all three variables.

We can get accordionContent from accordion.

function updateAccordion(accordion) {
  const accordionContent = accordion.querySelector('.accordion__content')

  accordion.classList.toggle('is-open')
  accordionContent.style.height = `${height}px`
}

We can also get height from getContentHeight.

function updateAccordion(accordion) {
  const height = getContentHeight(accordion)
  const accordionContent = accordion.querySelector('.accordion__content')

  // Updates the accordion
  accordion.classList.toggle('is-open')
  accordionContent.style.height = `${height}px`
}

But we should not get height inside updateAccordion. If we get height inside updateAccordion, we make updateAccordion do TWO things instead of one.

Getting the height value (first thing)
Updating the height value (second thing)

If the function name says updateAccordion, we expect code inside update the accordion only. So we pass height into updateAccordion.

function updateAccordion(accordion, height) {
  const accordionContent = accordion.querySelector('.accordion__content')

  // Updates the accordion
  accordion.classList.toggle('is-open')
  accordionContent.style.height = `${height}px`
}

Using updateAccordion:

accordionContainer.addEventListener('click', event => {
  // ...
  updateAccordion(accordion, height)
})

Ordering functions and variables

In this case, none of our functions require variables from the global scope. We can declare them upfront before they’re used.

function getContentHeight(accordion) {
  /* ... */
}
function updateAccordion(accordion, height) {
  /* ... */
}

const accordionContainer = document.querySelector('.accordion-container')
accordionContainer.addEventListener('click', event => {
  // ...
})