🛠️ Popover: Library

Source

Let’s start by copying everything from main.js into popover.js. As before, we will comment out everything and start anew. We will only uncomment the parts we need.

// popover.js
// Copy code here

We will link popover.js to main.js

// main.js
import `./popover.js`

Make sure to set main.js as an ES6 modules file.

<script src="/js/main.js" type="module"></script>

We’re ready to begin restructuring the Popover with OOP.

Restructuring with Object Oriented Programming

Object Oriented Programming is about creating instances from a blueprint. We begin our work by creating the blueprint. In this case, we’ll name the blueprint Popover.

I’m going to use Factory functions in this lesson. You are welcome to follow along with any OOP flavour.

export default function Popover () {
  // ...
}

We’ll jump straight into the execution part of the code and work backwards. This is the easiest way of figuring out how to refactor. Here’s what we begin with:

// Positions popover
popoverTriggers.forEach(popoverTrigger => {
  const popover = getPopover(popoverTrigger) || createPopover(popoverTrigger)
  const popoverPosition = calculatePopoverPosition(popoverTrigger, popover)

  popover.style.top = `${popoverPosition.top}px`
  popover.style.left = `${popoverPosition.left}px`
  hidePopover(popover)
})

Since we use forEach to loop through popoverTriggers, we know we need to pass each popoverTrigger into the Popover blueprint.

export default function Popover(popoverTrigger) {
  // ...
}

We can use Popover like this:

// main.js
import Popover from './popover.js'

const popoverTriggers = document.querySelectorAll('.popover-trigger')
popoverTriggers.forEach(trigger => {
  Popover(trigger)
})

Let’s focus on Popover now.

I’ll start changing the name of the popoverTrigger variable. In this case, I chose triggerElement because of two reasons:

The “popover” is redundant since we’re obviously dealing with popovers. trigger alone would suffice.
But trigger can be an action. We aren’t referring to an action in this case. We’re referring to a HTML element, so I added Element as a suffix to make the variable explicit.

export default function Popover (triggerElement) {
  // ...
}

For each triggerElement, we searched the DOM for the corresponding popover. If the popover is not found, we create one with JavaScript.

// Old code
popoverTriggers.forEach(popoverTrigger => {
  const popover = getPopover(popoverTrigger) || createPopover(popoverTrigger)
  // ...
})

We can copy these two lines into the Popover blueprint.

export default function Popover (triggerElement) {
  const popover = getPopover(popoverTrigger) || createPopover(popoverTrigger)
}

We need to change popoverTrigger into triggerElement.

export default function Popover (triggerElement) {
  const popover = getPopover(triggerElement) || createPopover(triggerElement)
}

I also propose changing popover to popoverElement since we’re referring to the HTML Element.

export default function Popover (triggerElement) {
  const popoverElement = getPopover(triggerElement) || createPopover(triggerElement)
}

Next, we need to decide where to place getPopover and createPopover.

Deciding where to place `getPopover` and `createPopover`

There are three solutions:

Place them outside Popover
Placing them both in Popover as methods
Placing them both in Popover as functions

All three choices work. In this case, I’d recommend choosing #3: Placing them both in Popover as functions. We’ll examine all three solutions and I’ll explain why #3 feels like the best approach.

Solution 1: Placing them outside `Popover`

Here’s the code for getPopover and createPopover.

function getPopover (popoverTrigger) {
  return document.querySelector(`#${popoverTrigger.dataset.target}`)
}

function createPopover (popoverTrigger) {
  const popover = document.createElement('div')
  popover.classList.add('popover')
  popover.dataset.position = popoverTrigger.dataset.popoverPosition

  // Dynamic id
  const id = generateUniqueString(5)
  popover.id = id
  popoverTrigger.dataset.target = id

  const p = document.createElement('p')
  p.textContent = popoverTrigger.dataset.content

  popover.appendChild(p)
  document.body.appendChild(popover)
  return popover
}

They already work if you place them outside Popover. Here’s how you’d use them:

export default function Popover (triggerElement) {
  const popoverElement = getPopover(triggerElement) || createPopover(triggerElement)
}

This works, but I find it hard for my brain to wrap around the code.

When I make a blueprint, I like to put all functions and methods related to that blueprint inside the blueprint itself. Functions outside the blueprint serve a different purpose.

Solution 2: Placing them inside `Popover` as methods

When placing functions inside blueprints, I normally use methods because it’s easier to find them at a glance.

To save getPopover and createPopover as methods, we need to create a common object to store all methods. I’ll use the popover variable in this case.

export default function Popover (triggerElement) {
  // ...

  const popover = {
	  getPopover (popoverTrigger) {
      // ...
	  },

	  createPopover (popoverTrigger) {
      // ...
	  }
  }
}

We should rename popoverTrigger to triggerElement since we used triggerElement in Popover.

export default function Popover (triggerElement) {
  // ...

  const popover = {
	  getPopover (triggerElement) {
	    return document.querySelector(`#${triggerElement.dataset.target}`)
	  },

	  createPopover (triggerElement) {
	    const popover = document.createElement('div')
	    popover.classList.add('popover')
	    popover.dataset.position = triggerElement.dataset.popoverPosition

	    // Dynamic id
	    const id = generateUniqueString(5)
	    popover.id = id
	    triggerElement.dataset.target = id

	    const p = document.createElement('p')
	    p.textContent = triggerElement.dataset.content

	    popover.appendChild(p)
	    document.body.appendChild(popover)
	    return popover
	  }
  }
}

We can omit the triggerElement parameter since triggerElement is already in the lexical scope.

export default function Popover (triggerElement) {
  // ...

  const popover = {
	  getPopover () {
      // ...
	  },

	  createPopover () {
      // ...
	  }
  }
}

We’ll call getPopover and createPopover likes his:

export default function Popover (triggerElement) {
  const popoverElement = popover.getPopover() || popover.createPopover()

  const popover = {/*...*/}
}

It’s not a great experience. We had to write the “popover” word twice in each method call.

export default function Popover (triggerElement) {
  // Repeating popover twice makes this code tedious to read
  const popoverElement = popover.getPopover() || popover.createPopover()

  const popover = {/*...*/}
}

We can shorten the name of these two methods by removing the “popover” keyword, but the methods don’t convert the same amount of meaning anymore.

export default function Popover (triggerElement) {
  // ... Doesn't convert the same amount of meaning
  const popoverElement = popover.get() || popover.create()

  const popover = {/*...*/}
}

What is popover.get getting? Are we getting the triggerElement? Are we getting popoverElement? Or are we getting something else?

It’s ambiguous.

Likewise popover.create is ambiguous too.

I try not to use ambiguous function/method names because they create unnecessary questions, and in turn, uses unnecessary brainpower.

The cleanest solution I found is to keep the names getPopover and createPopover.

export default function Popover (triggerElement) {
  const popoverElement = getPopover() || createPopover()

  function getPopover () {
    // ...
  }

  function createPopover () {
    // ...
  }

  const popover = {/*...*/}
}

This way: I get to tell my brain getPopover and createPopover are part of Popover. I also get the clarity that I’m getting/creating a popover with these methods.

If I wanted to be super explicit, I can rename getPopover as getPopoverElement. But I think that adds too much friction.

Where to put `generateUniqueString`?

createPopover uses a getUniqueString function. Again, we can put getUniqueString in three places:

Outside Popover
Inside Popover as a method
Inside Popover as a function

getUniqueString feels more like a helper function. It is generic enough to use in many different components. I tend to keep these functions outside my blueprints, but you can do whatever you want.

Let’s continue inspecting the rest of the code that executes the Popover. Here’s what we’re left with:

popoverTriggers.forEach(popoverTrigger => {
  // ...
  const popoverPosition = calculatePopoverPosition(popoverTrigger, popover)

  popover.style.top = `${popoverPosition.top}px`
  popover.style.left = `${popoverPosition.left}px`
  hidePopover(popover)
})

We need to calculate the popover’s position with calculatePopoverPosition. We can move this function into Popover as a method.

export default function Popover () {
  // Declare variables
  // Functions

  const popover = {
    calculatePopoverPosition (popoverTrigger, popover) {
      // ...
    }
  }
}

We can change popover to popoverElement and popoverTrigger to triggerElement. This conforms with what we have above. (Remember to change the variables inside the function too. I’ll let you do this yourself).

export default function Popover (triggerElement) {
  // Declare variables
  // Functions

  const popover = {
    calculatePopoverPosition (triggerElement, popoverElement) {
      // ...
    }
  }
}

We don’t need to pass triggerElement and popoverElement into calculatePopoverPosition they can be found in the lexical scope.

export default function Popover (triggerElement) {
  // Declare variables
  // Functions

  const popover = {
    calculatePopoverPosition () {
      // ...
    }
  }
}

To call calculatePopoverPosition, we need to write popover.calculatePopoverPosition.

Let’s eliminate the extra “Popover” keyword. We’ll use change calculatePopoverPosition to calculatePosition.

export default function Popover (triggerElement) {
  // Declare variables
  // Functions

  const popover = {
    calculatePosition () {
      // ...
    }
  }
}

We can then use calculatePosition like this:

export default function Popover (triggerElement) {
  // Variables
  // Functions

  const popover = {/*...*/}

  // Execution
  const popoverPosition = popover.calculatePosition()
}

Next, we want to position the popover with the positions we calculated. Here’s the previous code:

popoverTriggers.forEach(popoverTrigger => {
  // ...
  popover.style.top = `${popoverPosition.top}px`
  popover.style.left = `${popoverPosition.left}px`
  // ...
})

We can use these two lines directly:

export default function Popover (triggerElement) {
  // ...
  const popoverPosition = popover.calculatePosition()
  popoverElement.style.top = `${popoverPosition.top}px`
  popoverElement.style.left = `${popoverPosition.left}px`
}

Finally, we hide the popover with hidePopover.

Here’s the previous code:

popoverTriggers.forEach(popoverTrigger => {
  // ...
  hidePopover(popover)
})

We can add hidePopover to Popover as a method.

export default function Popover (triggerElement) {
  // Variables
  // Functions

  const popover = {
    hidePopover (popover) {
      // ...
    }
  }

  // Execution
}

As before:

We’ll change popover to popoverElement.
We don’t need to pass popoverElement since its already in the lexical scope.
We’ll remove the extra “Popover” keyword from the method’s name.

Here’s the optimised version:

export default function Popover (triggerElement) {
  // Variables
  // Functions

  const popover = {
    hide () {
      // ...
    }
  }

  // Execution
}

We can use hide like this:

export default function Popover (triggerElement) {
  // Variables
  // Functions

  const popover = {
    hide () {
      // ...
    }
  }

  // Execution
  // ...
  popover.hide()
}

An aside on `hide`

popover.hide isn’t very explicit – it suffers from the same problem as popover.get and popover.create.

If we want to be super explicit, we can do something like popoverElement.hide. To do this, we need to add a method to popoverElement.

export default function Popover () {
  popoverElement.hide = function () {
    // ...
  }
}

Be careful of doing this because you’ll create a non-standard HTML element. When you work in teams, you may mislead people into thinking HTML Elements have the hide method. (Which is not good).

If you insist on standardisation and consistency, you can create a function called hidePopover inside Popover instead.

export default function Popover () {
  // ...

  function hidePopover() {
    // ...
  }

  // ...
}

I don’t like this because the extra function tells me there’s another exception I need to watch out for – which uses brain energy – and it’s something I’m reluctant to do.

So popover.hide is a happy-enough medium for me right now, and I don’t bother to refactor more.

(Protip: Good enough solutions are good enough. Move on when you think it’s good enough! If you come back to it and it irritates you, you still have the chance to refactor).

🛠️ Popover: Library

🛠️ Popover: Library

Restructuring with Object Oriented Programming

Deciding where to place `getPopover` and `createPopover`

Solution 1: Placing them outside `Popover`

Solution 2: Placing them inside `Popover` as methods

Solution 3: Placing them in Popover as functions

Where to put `generateUniqueString`?

Initializing the Popover

Hiding the Popover

An aside on `hide`

🛠️ Popover: Library

🛠️ Popover: Library

Restructuring with Object Oriented Programming

Deciding where to place getPopover and createPopover

Solution 1: Placing them outside Popover

Solution 2: Placing them inside Popover as methods

Solution 3: Placing them in Popover as functions

Where to put generateUniqueString?

Initializing the Popover

Hiding the Popover

An aside on hide

Deciding where to place `getPopover` and `createPopover`

Solution 1: Placing them outside `Popover`

Solution 2: Placing them inside `Popover` as methods

Where to put `generateUniqueString`?

An aside on `hide`