@mickyginger
Posted
Hi Sky,
I think I may be able to help with your JavaScript 🤗
Firstly, you are calling the updateValue
method onchange
and passing this.value
, however this
is not defined until the method is called so it's evaluating to undefined
inside the method.
Secondly, in updateCheck
you are setting the content of #per-month
to be the result of setting #dollar
to be "$" + val + ".00"
(which is just the string "$15"), which means that the #dollar
span is being removed.
How about something like this:
const check = document.getElementById('check');
const dollar = document.getElementById('dollar');
const interval = document.getElementById('interval');
function updateCheck() {
const isYearly = check.checked
const amount = isYearly ? '$20.00' : '$15.00';
const interval = isYearly ? '/yearly' : '/monthly';
dollar.textContent = amount;
interval.textContent = interval;
}
With an added span in your HTML
<p id="per-month">
<span id="dollar">$16.00</span>
<span id="interval">/month</span>
</p>
This feels a little easier to reason about. I have stored the DOM elements in variables outside the function so that we don't need to retrieve the elements from the DOM each time we click on the checkbox (this is actually quite an expensive operation for JS).
I'm using a ternary operator to decide which values to display based on whether the checkbox is checked, which makes the code a little more terse.
You could (or maybe should?) also assign the event listener in the JavaScript code which would then separate the responsibilities of the HTML and JS files. Something like:
check.addEventListener('change', updateCheck);
I would also probably advocate changing the name of updateCheck
to updateAmount
, since the function changes the amount displayed to the user rather than modifying the checkbox in any way. This may seem a bit picky, but it helps to make it easier for another developer to quickly understand what your code is doing.
Hopefully that all makes sense! 😀
Marked as helpful
@Skyz03
Posted
@mickyginger thank you so much.