0

I have a javascript function with two parameters : results which is an object array and i which is the index.

The function displays item from that array. I also want to to build links to show other entries in the array.

My code is:

function renderNews(results, i) {
    $('.articleTitle').text(results[i].Title);
    $('.articleBody').text(results[i].newsBody);

    // Build links
    var linkHtml = '';

    for (var i = 0; i < results.length; i++) {

   linkHtml += '<a href="#" onclick="renderNews(results, ' + i + ')">' + (i + 1) + '</a> ';
    }

    $('.articleActions').html(linkHtml);
}

As you can see I am setting my onclick for the function to call itself to redraw the results. I get a "function not defined error".

I'm still very much learning as I go. Is it bad idea for a function to call itself? I wonder if anyone can advise on the right way of doing this.

2
  • The on click function is not defined because the function wasnt called yet. Therefore nothing within the function has been executed Commented Jan 28, 2016 at 15:36
  • 1
    The bad idea is to use onclick instead of listeners to click events. Commented Jan 28, 2016 at 15:37

1 Answer 1

1

If I understand, renderNews will be called when the page gets loaded, right? Actually, your links would be put inside a component with articleActions class. By your idea, clicking any link would call this function again, and all links would be replaced by a new links. This sounds strange. Also, I can't tell what do you expect when passing that results to the onclick event. Actually, if your idea was to always reuse the same results array, passing it undefinitely to the same function over and over again, you could make things much simpler:

function renderNews(results) {

    if (results.length == 0)
      return;

    // Build links
    var linkHtml = '';

    for (var i = 0; i < results.length; i++)
       linkHtml += '<a href="#" data-articletitle="' + results[i].Title + '" data-articlebody="' + results[i].newsBody +'" class="article-link">' + (i + 1) + '</a> ';

    $('.articleActions').html(linkHtml);

    $('.articleTitle').text(results[0].Title);
    $('.articleBody').text(results[0].newsBody);

}

$('.article-link').click(function(){
    $('.articleTitle').text($(this).data('articletitle'));
    $('.articleBody').text($(this).data('articlebody'));
});

As far as I understand, whenever you want to update the current articles, you call renderNews which will build/rebuild a lot of links for each article in the array holding their data (title and body), and will load the first item. So renderNews is going to be called once the page loads (I don't know how you intend to do this).

There is a click event for any component with article-link class, in this case all your links (anchors). When one is clicked, it updates the screen (article's title and body) with its data.

You could improve the code to keep track of the selected item, and once renderNews is called, you load that article instead of the first one. Or you could keep passing the article's index as parameter, like your example.

Since I don't know how do you call renderNews function, it's hard to make a better code, but this might clear something to you.

Simple JSFiddle demo

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

4 Comments

Should the click event use data-articleTitle rather than articTitle?
I have tried both, but neither updates the text in each element. I can see in the DOM that both of the data- elements have the correct value. I can also see that the event fires correctly but the text does not change. Any ideas please?
@Brendan when you call $(this).data('foo') jQuery looks for a data-foo attribute, you don't need to pass data- to the parameter, only what comes next, in our case, articletitle. My bad that I defined these attributes with upper case (articleTitle and articleBody) I fixed it in my example, and added a JSFiddle so you can see how it works.
@Alisson - thank you for taking the time to explain this to me. I have learned a lot from your example and my code now works. Best wishes, Brendan.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.