0

I want to create several html elements using javascript. I have a working version but the code repeats itself, I would like to collapse it into a for loop.

Here the code to reduce:

function editerPage() {
    var boutonTitre = document.createElement('button');
    boutonTitre.id = 'titre';
    boutonTitre.innerHTML = 'Titre de l\'onglet de la page';
    boutonTitre.setAttribute('onclick', 'titre()');
    var boutonH1 = document.createElement('button');
    boutonH1.id = 'h1';
    boutonH1.innerHTML = 'Titre h1 de la page';
    boutonH1.setAttribute('onclick', 'h1()');
    var boutonP = document.createElement('button');
    boutonP.innerHTML = 'Paragraphe de la page';
    boutonP.id = 'p';
    boutonP.setAttribute('onclick', 'p()');
    var inputCouleurH1 = document.createElement('input');
    inputCouleurH1.innerHTML = 'Changer la couleur du titre h1';
    inputCouleurH1.id = 'input';
    inputCouleurH1.setAttribute('onclick', 'couleurH1()');
    inputCouleurH1.setAttribute('type', 'color');
    document.body.appendChild(boutonTitre);
    document.body.appendChild(boutonH1);
    document.body.appendChild(boutonP);
    document.body.appendChild(inputCouleurH1);
}

And Here i have try to reduce it but its doesent work:

var bouton = ['title', 'h1', 'p', 'input']

function editerPage() {
    for ( i = 0; i >= bouton.length; i++ ) {
        bouton[i] = document.createElement('button');
        bouton[i].innerHTML = 'Création du ' + bouton[i];
        bouton[i].id = '"' + bouton[i] + '"';
        bouton[i].setAttribute('onclick', '"' + bouton[i] + '"');
        document.body.appendChild(bouton[i]);
    }
}

Someone can help me please ?

4
  • There is a difference between setAttribute("onclick", "p()") (the former code) and setAttribute("onclick", "p") (the latter code). Also note that you are replacing each array element with the created element, so you cannot run editerPage() twice. Commented Aug 7, 2020 at 20:30
  • Exactly ! I need to add + "()" Commented Aug 7, 2020 at 20:35
  • U think i can launch editerPage() one more time if i launch an other function for remove all the element before the second launch ? Commented Aug 7, 2020 at 20:36
  • No, because you've replace bouton with elements by using bouton[i] = document.createElement('button');. Just use a local variable like var button = document.createElement('button');, and replace bouton[i] to the left of an equals sign or dot . with button... and make it document.body.appendChild(button); then you can run it as many times as you'd like, if you remove the part setting the ID, since those have to be unique. Commented Aug 7, 2020 at 20:41

4 Answers 4

3

The for loop seems to be incorrect. You might want to do: for(var i = 0, i < bouton.length; i++) { }

Also, you are assigning the created element back to the array and then accessing it to fill its text. Try using a local variable:

var bouton = ['title', 'h1', 'p', 'input']

function editerPage() {
    for (var i = 0; i < bouton.length; i++ ) {
        var btn = document.createElement('button');
        btn.innerHTML = 'Création du ' + bouton[i];
        btn.id = '"' + bouton[i] + '"';
        btn.setAttribute('onclick', '"' + bouton[i] + '"');
        document.body.appendChild(btn);
    }
}

editerPage()
Sign up to request clarification or add additional context in comments.

8 Comments

Should be < and you are missing a closing ).
Should also use var i = 0; so that you're not making i global.
I believe if i is> = button.length it makes an infinite loop right?
Wait, the last object is an input and you forgot this: element.setAttribute('type', 'color');
Thanks Giovanni, a valid remark. I just wanted to make Francks current snippet work.
|
1

Ciao, you could factorize code like this:

function editerPage() {
        let attributes = {
           type: ['button', 'button', 'button', 'input'],
           id: ['titre', 'h1', 'p', 'input'],
           html: ['Titre de l\'onglet de la page', 'Titre h1 de la page', 'Paragraphe de la page', 'Changer la couleur du titre h1'],
           click: ['titre()', 'h1()', 'p()', 'couleurH1()']
        };
        for (let i = 0; i < 4; i++ ){
          var element = document.createElement(attributes.type[i]);
          element.id = attributes.id[i];
          element.innerHTML = attributes.html[i];
          element.setAttribute('onclick', attributes.click[i]);
          if (attributes.type[i] === 'input') element.setAttribute('type', 'color');
          document.body.appendChild(element);
        }
    }

2 Comments

Sure, and maybe can i create an object for merge all the variable ?
Yes it's possible. You maybe want something like attributes = {type:[...], id:[...], html:[...], click:[...]}. I modify my answer.
0

You have made some errors:

  • You overwrite bouton[i] with the created button-element and set this as innerHTML. Use a new variable for this.
  • In the for-loop you used i >= bouton.length, so the loop will never be executed it had to be <.
  • btn.id is just bouton[i] withoutb quotationmarks because the variable stores the string already.
  • Better append the buttons to a wrapper-div and not to the body.
  • The click-event has to be done by adding an eventListener with the "click" as event and a function that will be done. I add here just a log at the console because I don't have anything from you for this.

I extended your button-array. Every entry is now an object which contains all information for creating this tag. You could easily add new properties. If it's not allways necessary than test if the property is set before handle it at the new tag like I do it with type='color' which is only used for the input.

var bouton = [
  {id: 'title', tag: "button"},
  {id:'h1', tag: 'button'},
  {id: 'p', tag: 'button'},
  {id: 'input', tag: 'input', type: 'color', _click: 'couleurH1'}
];

editerPage();

function editerPage() {
    for ( let i = 0; i < bouton.length; i++ ) {
        let btn = document.createElement(bouton[i].tag);
        btn.innerHTML = 'Création du ' + bouton[i].id;
        btn.id = bouton[i].id;
    
        btn.addEventListener('click', function() {
            console.log(btn.id);
        });

        if (btn.type)
            btn.setAttribute('type', bouton[i].type);

        document.getElementById("wrapper").appendChild(btn);
    }
}
<div id="wrapper"></div>

2 Comments

Ciao, one of the elements is an input and it has inputCouleurH1.setAttribute('type', 'color');
@Giovanni Esposito You are right. I extended the button-array so every entry is now a object. With this it`s no problem to handle the coulor-input and much more.
0

A little late to the party but there is a cleaner and more efficient way to archive what you are trying:

const el = document.querySelector('#app');

el.addEventListener('click', () => {
  console.log(event.target);
});

el.innerHTML = `
<h1>Page Title</h1>
<p>Lorem, ipsum dolor.</p>
<input value="Lorem ipsum dolor sit." >
<button id="titre">Click Me</button>
`;

This way we touch the DOM once. Delegate elements creation to DOM, which is faster. We use event delegation, in other words we add single listener, again faster and more efficient. Remember, you can use target object to modify the clicked element:

el.addEventListener('click', () => {
  // Modify the element that received the click
  // You can use conditionals to check if the element
  // is the one you want to modify
  event.target.innerText = 'Hello';
});

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.