JavaScript code to add buttons and slowly remove them

Question

My javascript code is working, however i am seeking suggestions on how I can go about improving my existing code for reusability and testability. The code is pretty straight forward, I have three buttons "add", "slow remove" and "clear". The add button adds 40 buttons to the DOM dynamically with id + index. The slow remove button, uses setTimeout and CSS transition opacity to slowly fade out the 40 buttons, and the clear button clears the 40 buttons immediately.

Buttons JS:

  (function () {

    /* button_add when clicked dynamically generates 40 buttons with indexes
       when clicked an alert is display with button's id */
    const CLS = ["thin", "button"];
    const BUTTON_NUM = 40;
    let buttons = [];
    let add_btns = document.querySelector('.button_add');
     add_btns.addEventListener("click", function () {
         let i = 0;
        while (i <= BUTTON_NUM) {
            let btn = document.createElement("BUTTON");
            buttons.push(container.appendChild(btn));
            i++;
            for (let button in buttons) {
                btn.setAttribute('id', "button " + button);
                btn.classList.add(...CLS);
                btn.innerHTML = "Button # " + button;
            }
            btn.addEventListener("click", function () {
                let id = this.id;
                alert("my id is: " + id);
            });
        }
     });

    /* button_slow_remove when clicked 40 buttons fade out slowly and buttons array is
       emptied */
    let slow_remove_btns = document.querySelector('.button_slow_remove');
    slow_remove_btns.addEventListener("click", function () {
            buttons.forEach(function (el) {
                el.classList.add('slow-fade');
            });
            buttons.length = 0;
    });

    /* button_clear when clicked 40 buttons are cleared immediately and buttons array is
           emptied */
    let clear_btns = document.querySelector('.button_clear');
    clear_btns.addEventListener("click", function () {
        buttons.forEach(function (el) {
            el.classList.add('clear');
        });
        buttons.length = 0;
    });
}) ();

Buttons.css:

html section button, body section button {
    align-self: center;
    background: transparent;
    padding: 1rem 1rem;
    margin: 0 1rem;
    transition: all .5s ease;
    color: #41403E;
    font-size: 2rem;
    letter-spacing: 1px;
    outline: none;
    box-shadow: 20px 38px 34px -26px rgba(0, 0, 0, 0.2);
    border-radius: 255px 15px 225px 15px/15px 225px 15px 255px;
}

div#container button {
    display: inline-flex;
    align-self: center;
    background: transparent;
    padding: 1rem 1rem;
    margin:  .2rem 1rem;
    transition: all .5s ease;
    color: #41403E;
    font-size: .3rem;
    letter-spacing: 1px;
    outline: none;
    box-shadow: 20px 38px 34px -26px rgba(0, 0, 0, 0.2);
    border-radius: 255px 15px 225px 15px/15px 225px 15px 255px;
}


html section button:hover, div#container button:hover, body section button:hover {
    box-shadow: 2px 8px 4px -6px rgba(0, 0, 0, 0.3);
}

section button.dotted, body section button.dotted.thick {
    border: dotted 5px #41403E;
}

div#container button.thin, body section button.lined.thin {
    border: solid 2px #41403E;
}

.button--main, button.button{
    cursor: pointer;
}

.clear {
    display: none!important;
}

.slow-fade {
    opacity: 0;
    -webkit-transition: opacity 7s linear;
    -moz-transition: opacity 7s linear;
    transition: opacity 7s linear;
}

@media (max-width: 620px) {
    body h1 {
        margin-top: 2rem;
    }
    body section, div#container {
        display: flex;
        flex-direction: column;
        justify-content: center;
        margin-bottom: 1rem;
    }
    body section button {
        align-self: center;
        margin-bottom: 2rem;
    }
}

Answer 1

---

Naming.

Use a consistent and idiomatic naming style. Most important is consistency within the code base.

JS

For JS use cammelCase for all names appart from object that are instanciated with the new token, in that case use PascalCase. For constants you can use UPPER_CASE_SNAKE but as objects and arrays are often declared as constants you can end up with a lot of UPPER_CASE_SNAKE in the code, which does not read well.

There seams to be no general agreement as to what style to use. I personally only use UPPER_CASE_SNAKE for constants that are basic types (Number, Bool, String, etc..) and for object properties that are not writable.

CSS

Currently your CSS naming is inconsistent. This will make it harder to write good code as you will be spending mental energy remembering what style you used for what name, eg you have the two classes named button_slow_remove and slow-fade you remember the name not the hyphen or underscore, later you come back to the project and you will forever be checking "Was that name underscore or hyphen"

For CSS you can use Block Element Modifier (BEM) naming convention. The link provided a good introduction with plenty of resources to get you started.

Note BEM is but one of many and there is nothing wrong with defining your own, as long as you are consistent.

Plurals

Names give hints to a variables content. We pluralize arrays, lists, sets, etc (sometimes even strings if we are using them as an array of characters), and use the singular for single items.

You are using let add_btns = document.querySelector('.button_add'); and someone, even you coming back later will automatically think buttons (an array / array like), its a buton, use the singular const addBtn = document.querySelector('.button_add');

Also it is ok to use common abbreviations, btn is just as good a name as button

Always BE CONSISTENT!

---

Code Style

• NEVER start a line with a ( unless it is part of a multi line statement or expression. You have (function () { If someone adds a line above this and is one of those crazzy people that say they don't need to terminate lines

Eg

var a = 0
var b = a  // << this line is exterminated

 (function() {  // << will parse as var b = a(function() { and throw an error

So play it safe and add the ; for those that don't.

var a = 0
var b = a  // << this line is un-terminated

;(function() {  // the ; terminates any multi line statement or expressions

// while at you can use the shorter form

;(()=>{
    ...code
})()

• Use arrow functions to reduce code noise. Code noise is any code that is not needed.

• Always declare variables as const, only use var or let if the variable needs to change. The following variables are all incorrectly defined as let yet don't change buttons, add_btns, btn, id, clear_btns, slow_remove_btns

• Take care with the indenting.

• Reduce code noise. A measure code quality can be defect density, this value is linked to the coder and their experience level. Your defect density will remain consistent (though time will reduce the value), it means more code, more bugs. The easiest way to reduce the number of bugs in your code is to reduce the code size. You have many unneeded lines of code, many due to style.

You have

btn.addEventListener("click", function () {
    let id = this.id;
    alert("my id is: " + id);
});

// less noise
btn.addEventListener("click", (e) => alert("My id is: " + e.target.id));

// or to keep `this`
btn.addEventListener("click", function() { alert("My id is: " + this.id) });

---

API use

• Don't use setAtribute if its not needed. The general rule is setAtribute is only needed for properties that are not part of the DOM's API and needs to be reflected in the markup.

• When setting text for an element use the textContent property, ONLY use innerHTML if you are adding HTML

• Use an element's id to select it rather than the class name. Using class name forces you to create CSS rules for elements with the same style.

• Rather than add the buttons directly to the DOM, which will force a re-flow and slow things down, use a documentFragment, or create an off page container to work with, then when complete add them to the page in one call / one re-flow.

---

Bugs

You have some bugs in the code.

1. Your code comments say 40 buttons yet the code generates 41 buttons.

The code at fault is

const BUTTON_NUM = 40;

// ... some code removed

     let i = 0;
    while (i <= BUTTON_NUM) { // from 0 to 40 are 41 steps
    
    // Should be
    while (i < BUTTON_NUM) {

2. You are adding id's too often. Inside the while loop you do a for in loop over the buttons you have already created. This means the first button is assigned the same values 41 times.

---

Testing

You don't write your code for testing, you test code. You have not presented any testing code so there is nothing to review.

---

Some other points

• Keep things simple. You are storing an array of buttons, you are adding events to each fade button. There is no need. Add a single event to the button container, use the event.target argument to get the element clicked on. Don't store elements if you don't need them

• Magic numbers and strings are bad. You code has many strings referring to class names all over the place. If you need to make changes, it requires you to make changes in many places, which is a pain and potencial defect.

  You started well with const CLS = ["thin", "button"] and const BUTTON_NUM = 40 but then started mixing in string as needed. Keep all magic values (constants that are subject to design change) in one place, that need only one change to the source.

---

From the Comments

Try to make it class – zb

NO Don't! JS does not have classes, only objects. class is a syntax used to create objects.

You don't need to create an object, you have correctly encapsulated your code with a singleton, it needs no exposed API (object with properties and functions), and works as a static single instance (It does not need many copies).

OO coding is a complex subject, and most low level coders do not have a good grasp of when and how to use it effectively. Making it a class does not mean it makes it better code.

Rewrite

From the ground up and using an object btns to hold button related consts, and functionality.

To improve portability, or really just make it more flexible I have create functions to cover the common tasks you code was doing. Eg query, addEvent, clickEvent...

I have also deliberately referenced element buttonContainer to show that it is a legitimate way to access elements. But do note that that name is global and not isolated to the singleton.

The layout and CSS have been improvised as you only provided CSS and that had unrelated content.

The code adds 3 events for each of the main ui buttons. The add button adds the buttons to a fragment and then to the page (clearing the previous content beforehand)

Then a final event is added to the button container to manage the alert showing button id.

It is designed to simplify the adding of Remove FX. You need only change 3 lines of JS and that is only data to add a new fade FX. (of course you need to add the HTML and CSS)

;(() => {
    const btns = {
        count : 13,
        className : "buttons buttons--style-default buttons--action-fader",
        text : "Button #", // the # is replaced with the ID
        id : "button#",    // the # is replaced with the ID
        actions : {add : "#addBtn", slowFade : "#slowFadeBtn",clear : "#clearBtn" },
        anim : { slowFade : "buttons--anim-fade", clear : "buttons--anim-clear" },
        animNames : ["slowFade", "clear"],        
        reset() {           
           btnContainer.innerHTML = "";
           for (const name of btns.animNames) { btnContainer.classList.remove(btns.anim[name]) }
        },
        remove(method) { btnContainer.classList.add(method) },
    };
 
    const query = query => document.querySelector(query);
    const addEvent = (el, func, type = "click") => el.addEventListener(type, func);  
    const clickEvent = event => alert("My id is : " + event.target.id);
    const actionEvent = name => addEvent(query(btns.actions[name]), () => btns.remove(btns.anim[name]));
    const frag = document.createDocumentFragment();
            
    addEvent(query(btns.actions.add), () => {
        btns.reset(); 
        for (let i = 0; i < btns.count; i ++) {
            frag.appendChild( Object.assign( document.createElement("button"), {
                    id : btns.text.replace("#", i),
                    className : btns.className,
                    textContent : btns.text.replace("#", i),
                })
            );
        }
        btnContainer.appendChild(frag);  
    });
    addEvent(btnContainer, clickEvent);
    btns.animNames.forEach(actionEvent);
}) ();

.buttons {
    background: transparent;
    align-self: center;
}
.buttons--style-default {
    padding: 0.5rem 0.5rem;
    transition: all .5s ease;
    outline: none;
    color: #41403E;
    cursor: pointer;
    font-size: 1rem;
    box-shadow: 20px 38px 34px -26px rgba(0, 0, 0, 0.2);
    border-radius: 255px 15px 225px 15px/15px 225px 15px 255px;    
}
.buttons--action-ui {
    align-self: center;
    margin: 0 0.2rem;
}
.buttons--action-fader {
    display: inline-flex;
    margin:  .2rem 0.2rem;
    border: solid 2px #41403E;
}
.buttons--style-default:hover {
    box-shadow: 2px 8px 4px -6px rgba(0, 0, 0, 0.3);
}
/* Animations or animation like rules for buttons */
.buttons--anim-clear {
    display: none!important;
}
.buttons--anim-fade {
    opacity: 0;
    transition: opacity 4s linear;
}

<div class="buttons">
    <button id="addBtn" class="buttons buttons--style-default buttons--action-ui" >Add buttons</button>
    <button id="clearBtn" class="buttons buttons--style-default buttons--action-ui" >Remove buttons</button>
    <button id="slowFadeBtn" class="buttons buttons--style-default buttons--action-ui" >Remove with fade</button>
    <div id="btnContainer" class = "buttons buttons-container">
    </div>
 </div>