Javascript click event stacking on the button inside API pagination

1

Hey guys,
I am writing pagination for my project and have a problem with click event stacking on each other. Here’s my project:

https://glitch.com/edit/#!/dash-of-passion?path=public%2Fjs%2Frecipes.js%3A1%3A0

If you go to “recipes” tab and choose a category (don’t use input, not implemented yet) you will get recipes through API and it will create a pagination on the bottom. Now when i click on the page i want to :

When changing the page it should show 10 different recipes according to the page we are at but it seems that the click event keeps stacking with every click, e.g =>
1st click = 10 results,
2nd click = 20 result,
3rd click = 40 results and so on…

I have tried:

  1. Using a named event handler in the click event but then i can’t use event.target for changing the page number
  2. Using “once:true” as an extra parameter in the click event, it did nothing.

Code for getting the recipes starts at line 114 in public/recipes.js.

Code for creating buttons, pages and stuff starts at line 153

Any help would be greatly appreciated !

2

Hello,

It’s doing what you expect, but you’re appending the results directly to the containing element instead of replacing the recipes, that’s why you end up with 10 results more each time.

On the other hand, on the file server where you retrieve the data, you should be passing the expected range of results as the from and to:

https://edamam-recipe-search.p.rapidapi.com/search?q=${category}&from=X&to=Y

This is to prevent the load of all the results and to be able to retrieve the results past the 100.

3

Ah ok so the problem here is the :

recipesArray.forEach(recipe => {
				result += `
				<...>
				`
		})

and then :

recipesCard.innerHTML = result;

Right ? How should i change it ?
As for the from and to i refrained from using it as free version of the API won’t allow me to access recipes further then 100th position, that’s why i made a limit in recipes.js :

const amountOfPages = 100/parseInt(amountChoice);

This is gonna create only as much buttons to access 100 recipes and no more :slight_smile:
So if user chooses to see 50 recipes per page, they will get only 2 buttons :stuck_out_tongue:

4

Sorry, I was wrong :sweat_smile:. Your code is a little messy, so I was confused :sweat:.

The problem lies within the function that handles the page click and how you determine which is the current page.

Look, I forked your project and added more console output to the front end: Glitch :･ﾟ✧. Review what each click does and try to find the problem :slight_smile:.

I understand that now :slight_smile:.

5

Sorry my brain is absolutely dead today (i work as a chef and had mental weekend at work) from exhaustion, can’t see what’s wrong there :frowning: Could you tell me please ?

6

The problem was that you were rebuilding the pages each time a page was changed, which included adding a new listener each time that caused the duplication of items.

Compare your code to this: https://glitch.com/edit/#!/wax-ludicrous-english?path=public%2Fjs%2Frecipes.js%3A170%3A7, from line 113 onward.

7

Ok that’s great ! I see the change but i can’t understand how it works. So you initialize pagesBuilt variable as a false and when you change the page it becomes true. Then you check if pagesBuilt is true and if not you change the page ? It definitely works i am getting 10 results per page change but don’t understand how ? Why pagesBuilt is not true after you changed it once ?

And how to get rid of previous recipes ?

EDIT: Also why you used map method for sorting out recipes instead forEach ? Just beacuse it looks neater ?

EDIT2: Just had an idea how to handle previous recipes, should i splice the array, push that splice in to new array and using forEach remove them there ?

Thank you so much :slight_smile:

8

Sorry for the delay :sweat_smile:.

Not exactly. The variable is used as a switch so the pages and listeners are setup once. You see, the first time the page is loaded, the pages do not exist (obviously). So you create the function to build the pagination and call it afterwards. The problem with this is that each time the createRecipe is called, it builds the pages again, hence you end up multiplying the listeners (that’s why the results were duplicating exponentially).

This: if (!pagesBuilt) means if the pages are not built. The ! that precedes the pagesBuilt variable negates the condition. This flag stays the same until you reload the page or reset its value (if needed).

I updated the code.

You were using a variable named result. Assuming you were trying to reference the result variable defined in the previous then, then that result was defined outside the current context, which makes it a global variable. If it’s a global variable, that means the contents were never erased, thus appending the generated HTML.

Yes, it’s not really necessary :slight_smile:.

Check the updated code :slight_smile:.

I hope it helps :grin:!

Regards!

9

Not exactly. The variable is used as a switch so the pages and listeners are setup once. You see, the first time the page is loaded, the pages do not exist (obviously). So you create the function to build the pagination and call it afterwards. The problem with this is that each time the createRecipe is called, it builds the pages again, hence you end up multiplying the listeners (that’s why the results were duplicating exponentially).

Ok now i get it why the listeners were duplicating :slight_smile:

This: if (!pagesBuilt) means if the pages are not built . The ! that precedes the pagesBuilt variable negates the condition. This flag stays the same until you reload the page or reset its value (if needed).

Sorry i might have written it wrong way, what you wrote above is what i meant :stuck_out_tongue:
What i don’t understand is you check if pagesBuilt are not built (that i understand) then if they are not build you call the changePage function (that i understand) but then pagesBuilt is set to true inside changePage function so with the next click event on the page change button it shouldn’t work because if(!pagesBuilt) changePage() but pagesBuilt is true now, so how come i am still able to change the page ?

Also just had a look at the code after change for .join(‘’). As i understand it loadedArray is a new array that joins together everything that goes in to recipesArray one by one, then only that is displayed right ? How’s join different to what i did ? I understand your explanation about result being a global variable, would it have different result (no pun intended) if that would be a local variable ?

And thank you so so much for all the explanation, it’s a massive help :slight_smile:

10

Because the listeners do the work. The changePage is just setting up the pages, not really handling the event. In fact, I would rename the function to setupPages or something similar, just to be consistent.

Analyzing the function, all it does is:

  1. Iterate the recipePagesBtns.
  2. Add a click listener to each button.

Yes, that’s right.

It’s not different in the result, but it’s easier to read and may be a little more performant than what you did (it may be insignificant though).

Yes, it has. In my code, it is a local variable (local to the createRecipe function), and it gets created each time the function is called. If you defined your result inside the createRecipe, then you wouldn’t need to care if it had content (it would always be empty).

It can be a global, but you need to take care in how you handle it. You were doing this:

// Inside your recipe iterator
result += `<div ...`;

But it was never cleared, hence you were always appending new content, not replacing it. A simple fix:

result = '';
// Then, inside your forEach:
result += `<div ...`;

With that said, globals are not a good practice. It can lead to unexpected results, specially if you’re working with other people. You experienced these unexpected results, in fact :stuck_out_tongue:.

Always try to define the variables you really need.

And finally, remember this phrase: Divide and conquer, which means (in this context) split the problem into smaller pieces and solve each one separately.

Let me know if you have more doubts :slight_smile:,

Regards!

11

Thanks man, you are amazing :slight_smile: I think i understand most of it but i am so tired and i am working so much recently that i don’t even have time to eat properly. Next week i am gonna have a good sit down, analyse everything and if i have any problems i will message you, is that ok ? Again thank you so much, :slight_smile:

1 Like
12

Of course, no problem :slight_smile:.

I hope you get some time to rest :slight_smile:!

1 Like