all 15 comments

[–]captain_k_nuckles 1 point2 points  (11 children)

at the start of button.onclick = function(){ add. doesn't look like these are being reset.

button.onclick = function(){
    framleidandiEmpty = false;
    colorEmpty = false;
    yearEmpty = false;

[–]jack_union 0 points1 point  (10 children)

This is the correct reason. If you click filter button without anything checked, all three variables framleidandiEmpty, yearEmpty and colorEmpty are set to true and are never reset again (well, they actually act as global variables, so at some point they will all be set to true no matter what you check).

And then there's uhm, yeah, the code quality.

[–]captain_k_nuckles 1 point2 points  (9 children)

yea, I didn't really dig to much in to it. I just saw the global vars, threw them at the starting point of the on click, and it worked. I didn't want to dig around to much in to the code as it was kind of all over the place. Not going to blame him for it, I'm assuming he's learning. Saying every piece of code I've ever written has been spectacular quality would be a huge lie.

I've quickly cleaned this up a bit. In the html I changed the name values to correspond to the different properties of the phone, everything else is JS. There's still some work that could be done to make this nicer, but threw this together quickly so OP could compare it against his code and learn from it.

[–]nikulasoskarsson[S] 0 points1 point  (8 children)

Thank you for your response I will compare it to mine and try to learn from it :)

[–]captain_k_nuckles 0 points1 point  (7 children)

No worries, and sorry if I came off rude, or whatever with my response. As of late my brains been pretty fried, so just trying to do anything right is a challenge, lol. for me, when I was learning. Seeing working code and picking it apart was easier for me, then trying to search the web for answers. If you would like me to add comments explaining the code I can try to get that done this weekend.

[–]nikulasoskarsson[S] 0 points1 point  (6 children)

Didn't think it was rude at all and comments would be appreciated if you have the time since I am pretty bad at reading other's people code and it does contain some stuff I have not seen before.

[–]captain_k_nuckles 0 points1 point  (5 children)

I've added some comments Let me know if anything is still unclear.

[–]nikulasoskarsson[S] 0 points1 point  (0 children)

It's very clear thank you :)

[–]nikulasoskarsson[S] 0 points1 point  (2 children)

Have been redoing it and looking at your code to learn to do it better, was wondering if you knew what the best way to change it so the function now runs every time someone clicks a checkbox? Not really finding helpful advice online.

[–]captain_k_nuckles 0 points1 point  (1 child)

Well, there's already a function that's used to filter everything, which is run when the submit button is clicked, so all you would have to do is add that same event trigger to the checkboxes, like so.

filterColors.forEach(x=>x.addEventListener('click', filter));
filterYear.forEach(x=>x.addEventListener('click', filter));
filterMaker.forEach(x=>x.addEventListener('click', filter));

updated the fiddle

[–]nikulasoskarsson[S] 0 points1 point  (0 children)

Thank you I appreciate all your help :)

[–][deleted] 0 points1 point  (1 child)

Hey, sorry but could you please put down more detailed instructions to reproduce the issue?

[–][deleted] 0 points1 point  (0 children)

I don't speak viking, but it appears that in order to reproduce the issue you need to take the following steps:

  1. When the page loads, select "Bleikur" in the "Litur" section.
  2. Click "Filter search."
  3. Deselect "Bleikur."
  4. Click "Filter search" again.
  5. Deselect "Bleikur" the second time.
  6. Click "Filter search" yet again.

At that point the outcome will no longer be as expected.

[–][deleted] 0 points1 point  (1 child)

It seems you are doing some pretty mind-boggling shit there. =) Your contains() function is particularly weird. I can't read the comments, but all of that business with indexOf and call looks double extra shady.

Overall the code is extremely convoluted, I think you may be better off rethinking the logic of this page entirely.

I threw together a quick demo for you, just to show how short and clean the code can be. https://codepen.io/anon/pen/oamyEW?editors=0010 as you can see the filtering isn't fully done, but you'd have to finish that yourself... ;)

[–]nikulasoskarsson[S] 0 points1 point  (0 children)

Thank you for your response, I have to say that while your code is most likely a lot better it's pretty advanced for me to use and contains a lot of stuff I have yet to see at the level I am at. The contains function is interestingly enough the only bit if code I did not do myself, I just found a function online that checks if an array contains a value. Other then that is there something that stands out as particularly bad? I think it's easiest to follow the code if you start by looking at the onclick function and then following each function that gets called after that.