Basic JS DOM output help

[rexehuk]

Hey guys,

Been out of the game for a while, been meaning to brush up for a long time.

Got a data coming back via AJAX from php, and im outputting an associative array. However when outputting to the html page the loop is over-writing the previous insert so you only ever get 1 element on the page.

reviews.forEach(function(review) {
document.getElementById("reviews").innerHTML= '<div class="col-sm-4 portfolio-item" id="review"> <div class="review" data-stars="1,2,3,4,5"> <p class="reviewContent">' + review[0] + '</p> <p class="reviewName">' + review[1] + '</p> <div class="rating-score"> <div class="rating-5">' + review[2] + '</div> </div> </div> </div>';
)

Where am I going wrong?

To maybe help, some of the HTML layout for you

<section class="success">
<div class="container">
<div class="col-lg-12 text-center">
<h2>What our customers say</h2>
</br>
<div class="row" id="reviews">


</div>
</div>


</div>
</section>

 

[Nymins]

Take out 'innerHTML' and replace it with 'append'. Or if you want to keep using 'innerHTML', you can change the equals to "+=" instead of just "=".

 

[rexehuk]

Take out 'innerHTML' and replace it with 'append'. Or if you want to keep using 'innerHTML', you can change the equals to "+=" instead of just "=".

Append didn't work, but the += works a treat.

Many thanks chap

 

[Nymins]

appendChild may work, forgot append is a jQuery function.

 

[kaku]

It's probably not a good idea to assign id="review" to each review you're adding, because you'll end up with multiple elements with the same ID which is invalid.

Also, since you're always inserting reviews into the same container, you can store the reference to the container element in a variable before looping (currently it's being searched for every loop iteration). In this particular instance you probably won't notice much of a difference, but it's a good practice to get into and can improve performance in other situations.

 

[rexehuk]

It's probably not a good idea to assign id="review" to each review you're adding, because you'll end up with multiple elements with the same ID which is invalid.

Also, since you're always inserting reviews into the same container, you can store the reference to the container element in a variable before looping (currently it's being searched for every loop iteration). In this particular instance you probably won't notice much of a difference, but it's a good practice to get into and can improve performance in other situations.

Cheers for the advice, outside of that I do have a counter implemented to do review_1 etc, but took it out to simply the forum post.

Cheers for the search variable, will look into doing that.

 

[Tom B]

This is a shameless plug, but I wrote a library for templating in JS that will make this job a lot simpler:

https://github.com/Level-2/Tranjsform

Here's the code:

var xml = '
<section class="success">
<div class="container">
<div class="col-lg-12 text-center">
<h2>What our customers say</h2>
</br>
<div class="row" id="reviews">



<div class="col-sm-4 portfolio-item" id="review">
    <div class="review" data-stars="1,2,3,4,5">
        <p class="reviewContent">review content</p>
        <p class="reviewName">name</p>
        <div class="rating-score">
            <div class="rating-5">rating</div>
        </div>
    </div>

</div>


</div>
</div>


</div>
</section>


';

var tss = '
#reviews > div {repeat: data(); }
.reviewContent {content: iteration(0); }
.rating-5 {content: iteration(2); }
.reviewName {content: iteration(1); }';

var template = new Tranjsform.Builder(xml, tss);

var output = template.output(reivews);

Assuming reviews is an array of the reviews from the server, this will place the fully generated HTML string in the output variable. If you want a document use `template.output(reviews,true);

 

[rexehuk]

This is a shameless plug, but I wrote a library for templating in JS that will make this job a lot simpler:

https://github.com/Level-2/Tranjsform

Here's the code:

Assuming reviews is an array of the reviews from the server, this will place the fully generated HTML string in the output variable. If you want a document use `template.output(reviews,true);

Cheers buddy.

I can hopefully ditch JS completely now which will be awesome! Will just return Laravel collections to the browser and format it all via the template engine.

The joys of a framework! Oh I'm not going back