Having issues with the functionality of my toggle effect for the show your local weather api

Having issues with the functionality of my toggle effect for the show your local weather api
0

#1

I have trying for some time now to figure out what is the issue with my onclick function and updating the temp. I think I need some impartial feedback so I can hopefully proceed along. If anyone can point where I am going wrong with my logic or a better way to achieve the desired effect(toggle btw Fahrenheit and Celsius and would definitely appreciate the feedback.


#2

Lines 39 - 50. Your second if is not executed because either cel or fah is true. Just remove it and you’re good to go.

if (cel) {
  cel = !cel;
  fah = !fah;
  $("#fUnit").off("click");
  $("#fUnit").removeClass("toggleUnit");
  $("#cUnit").addClass("toggleUnit");
  $("#cUnit").click(function() {
     toggleTemp(cel, fah);
  });
}

Also, you don’t have to remove the event and add it again. The one you set on line 120 is enough.


#3

If you look at the console, you will see a couple of errors. The one that is specifically causing the temp to not change is located in this line of your setAttribute function.

city = wData.name;

You have:

  $("#fUnit").click(function() {
    toggleTemp(cel, fah);
  });

which when the element with id=“fUnit” is clicked will execute toggleTemp. Everything is good so far. The problem is at the very end of your toggleTemp function, you are calling setAttribute. When it is called, wData has a value of undefined, because wData is not in scope anymore. wData only had a value when it was called from within the anonymous function of the .getJSON call.


#4

@TheGallery Thanks, I was able to desired functionality of switching between giving the user the option of choosing only either celsius or fahrenheit…go to weather.com for further clarification of what I mean…it displays the effect I am aiming for. Unfortunately, I am still unable to reflect the change in temperature to correlate with do selected option. Additionally, you mentioned that there was no need to add and removed the click option. If so, how would I then be able to achieve providing then user with only one option to click when there are two options presented. Any further suggestions would be greatly appreciated!


#5

@rmdawson71 I looked into the error you mentioned and I actually noticed it early on when doing a console.log of my cel & fah variables. Unfortunately, I chose to ignore because for the life of me I can’t find what is causing the issue. I mean it appears I am calling the object correctly and how else am I to retrieve the name of the city? Obiviously, it was wrong for me to do so because that(purposeful) oversight is now creating additional problems. Any suggestions on how I can fix this? Also, while we’re on the topic of errors, I am also getting an error due to the google maps funtion which doesn’t make any sense to me either … any suggestions for a resolution on this error as well?


#6

Additionally, you mentioned that there was no need to add and removed the click option. If so, how would I then be able to achieve providing then user with only one option to click when there are two options presented.

My bad, I thought you bound the event to the container element.

The reason your temp is not changing properly is the error that @rmdawson71 mentioned.


#7

You really only need to call setAttribute one time (inside the anonymous “success” function of the getJSON).

To resolve the toggleTemp issue, you could declare a variable called currentUnit withtin the scope of

$(function()` 

You would initialize currentUnit with “C” to represent the unit which is returned from the getJSON call.

You already have the following spans cUnit and fUnit you will need two separate on click event handlers which will call toggleTemp. You already have one below, so create the other one for cUnit.

  $("#fUnit").click(function() {
    toggleTemp(cel, fah);
  });

For toggleTemp, you will need to rethink how to use the new variable currentUnit to your advantage. You definitely will not be recalling toggleTemp inside the function like you are currently doing. You will also need to think if it is possible to get the Celsius and Fahrenheit values without them being arguments to the toggleTemp function. One more hint is you will need to remember to toggle the value of currentUnit each time toggleTemp is called. Because two different element clicks call the same toggleUnit function, your function needs to determine if anything needs to toggle at all. For example, if the your page is displaying 23 C, then clicking the C should do nothing.

I have given you some things to ponder. See what you can figure out on your own. If you get stuck, let us know and someone will try to help you out.


#8

@rmdawson71 Okay, I will attempt your suggestions. Before I do, does the fact that the cUnit & fUnit are symbols representing celsius & fahrenheit respectively have any bearing on how I would then utilize currentUnit?


#9

It shouldn’t, because you will change currentUnit based on which of the elements is clicked.


#10

So I was able to come up with this:

$("#fUnit").click(function() {
toggleTemp(currentUnit);
});
$("cfUnit").click(function() {
toggleTemp(currentUnit);
});

function toggleTemp(currentUnit) {
if (currentUnit=="C") {
    currentUnit ="F";
   fTemp = Math.round(parseInt($("#temp").text()) * 9 / 5 + 32);
   $("#temp").text(fTemp);
    $("#fUnit").removeClass("toggleUnit");
    $("#cUnit").addClass("toggleUnit");
 } else {
  currentUnit ="C";
   cTemp = Math.round(parseInt($("#temp").text()) * 9 / 5 + 32);
   $("#temp").text(temp);
   console.log(temp);
  $("#cUnit").removeClass("toggleUnit");
  $("#fUnit").addClass("toggleUnit");
  }
  }

This would eliminate the display temp function and call to setAttribute . However, I am unsure of the logic and unfortunately doing this has brought so many errors up in console.log that I can’t text it out.


#11

What if you created another element which is hidden (using css display:none) and in setAttributes, you would use the .text() and put the original Celsius value?. That way, you would always be able to display the Celsius value when needed and then only make a conversion to Fahrenheit when the fUnit element is clicked. The temp element would be populated based on what is in this hidden element and what currentUnit is.


#13

I was able to locate the problem… I had currentUnit set to F for both the if and else statements. I feel like a complete noob but at least it’s resolved and I can now move on…Yay!