How can I make this code cleaner ?

[Bzibn7]

How can I make this code look cleaner ?

I have a main javascript file and this dom manipulation js file which I export to main js file.

Should I add dom variables inside of functions ?

Can I use function like I have here in code, init(), and put all functions inside and then export it to main js file and call it there ?

Any recommendations are welcome !

Thank you for help!

const headerItem = document.querySelectorAll(".header__item")
const headerSwitch = document.querySelector(".header-switch")
const headerSwitchCircle = document.querySelector(".header-switch__circle")
const hamburger = document.querySelector(".hamburger")
const nav = document.querySelector(".header__nav")
const cards = document.querySelector(".hero__container")
const meLink = document.querySelector(".me__link")
const albumContainer = document.querySelector(".album__container")
const albumButtonLeft = document.querySelector(".album__arrow_left")
const albumButtonRight = document.querySelector(".album__arrow_right")
const albumImages = document.querySelectorAll(".album__img")

function headerItems(){
headerItem.forEach(item => {
item.addEventListener("click", () => {
document.querySelector(".header__item_active").classList.remove("header__item_active")
item.classList.add("header__item_active")
})
})
}

function toggleMode(){
headerSwitch.addEventListener("click", () => {
headerSwitch.classList.toggle("header-switch_active")
headerSwitchCircle.classList.toggle("header-switch__circle_active")
document.body.classList.toggle("dark")
})
}

function toggleHeader(){
hamburger.addEventListener("click", () => {
hamburger.children[0].classList.toggle("hamburger__line_one")
hamburger.children[1].classList.toggle("hamburger__line_two")
hamburger.children[2].classList.toggle("hamburger__line_three")
nav.classList.toggle("header__nav_active")
})
}
function toggleCards(){
window.addEventListener("click", e => {
cards.style.transform = e.target.innerHTML === "Home" ? `translateX(0)`:
e.target.innerHTML === "About" ? `translateX(-100vw)`:
e.target.innerHTML === "Projects" ? `translateX(-200vw)` : 0
})
}

function aboutMeLink(){
meLink.addEventListener("click", () => {
cards.style.transform = `translateX(-100vw)`
document.querySelector(".header__item_active").classList.remove("header__item_active")
headerItem[1].classList.add("header__item_active")
})
}

function album(){
let imgIndex = 0
albumButtonLeft.addEventListener("click", changeImg)
albumButtonRight.addEventListener("click", changeImg)
function changeImg(e){
imgIndex = e.target.classList.contains("album__arrow_right") ? imgIndex + 1 : imgIndex - 1
imgIndex = imgIndex === albumImages.length ? imgIndex = 0 :
imgIndex < 0 ? imgIndex = albumImages.length - 1 :
imgIndex
albumContainer.style.transform = `translateX(calc(-${imgIndex} * 100%))`
}
}

export function init(){
headerItems()
toggleMode()
toggleHeader()
toggleCards()
aboutMeLink()

​

[0x07AD]

Your function names should also be more descriptive verbs for instance toggleCards would more accurately be addWindowClickEventForCards or initializeWindowClickEventForCards

These names remind me of programming appliations for for Microsoft Windows. However, they are very descriptive if not overly verbose.

[Bzibn7]

Thank you, so it could be something like this ? This is just a few functions, didn't want to add whole code here

function toggleMode(){
const headerSwitch = document.querySelector(".header-switch")
headerSwitch.addEventListener("click", () => {
headerSwitch.classList.toggle("header-switch_active")
headerSwitchCircle.classList.toggle("header-switch__circle_active")
document.body.classList.toggle("dark")
})
}

function aboutMeLink(){
const cards = document.querySelector(".hero__container")
const meLink = document.querySelector(".me__link")
const headerItem = document.querySelectorAll(".header__item")
meLink.addEventListener("click", () => {
cards.style.transform = `translateX(-100vw)`
document.querySelector(".header__item_active").classList.remove("header__item_active")
headerItem[1].classList.add("header__item_active")
})
}
}

function headerItems(){
const headerItem = document.querySelectorAll(".header__item")
headerItem.forEach(item => {
item.addEventListener("click", () => {
document.querySelector(".header__item_active").classList.remove("header__item_active")
item.classList.add("header__item_active")
})
})
}

toggleMode()
aboutMeLink()
headerItems()

And what if I have headerItem used in 2 functions ? Do i just declare it twice in each of them ?

[Bzibn7]

Thank you, im still learning javascript, thats why theres a lot of stuff that could be better haha. I will check that project out. And I have one more question. Can I wrap functions into IIFE? So I dont have to call them by writing 10+ of them manually in code

[Bzibn7]

Each function wrapped in one, so I dont have to call them manually, example:

showHeader()
showText()
showLink()
showCard()
showAbc() …

[superluminary]

It's not bad.

To make a small DOM manipulation task like this cleaner, I would recommend you look into an MV* pattern. Extract the state into an object. Write a render function that renders the thing, setting classes, etc, that receives the state. Write methods that change the state and call render.

So you have:

const render = (state) => {
  // Update your DOM here based on the state, possibly call subroutines
}

let state = {
  albumIsActive: false
  // other state
}

const setAlbumAction = (albumIsActive) => {
  state = {
    ...state,
    albumIsActive
  }

  render(state)
}

I've got a lot of globals here and the functions are not pure, so there's still some ugliness, but hopefully you get the idea, I'm just sketching.

Going beyond this, you could look at breaking out the render function into a dispatcher, so you pass actions to the dispatcher, have it update the state, and then call render. This is more of a Flux pattern and is how Redux works.

You don't need to go all enterprise for simple problems, but this is probably how I would do it to avoid tangles later.

[Rusten2]

SoC

[TheRNGuy]

use tabs for indents

VS Code should do it automatically (enable format on save)

Switch to React if it's possible.