0

I have an ad I'm working on that displays user inputs from another page but since these inputs can be large numbers, I need to apply JS to format them to be $1K instead of $1000, $1M instead of $1,000,000 and so on. I've tried a few scripts that work in the console but aren't connecting with the HTML that has a value (that I input just to test the JS). I need to make sure this will work on whatever's inside the span tag since on the actual ad there will be a dynamic value that is pulling in whatever the user's input was. I'm pretty sure the issue is in regards to how I'm calling the value inside the first span with the #old id/ .current class, but I've tried several iterations of how to target that value and am getting further from the fix.

I have two JS scripts that are commented out currently, Attempt 1 and Attempt 2 and I'm wondering if it is in relation to the variable that is being defined before the function in Attempt 2 or if there's something missing at the end of either to have the script applied to the value I'm targeting.

Here's the codepen I have so far: http://codepen.io/linzgroves/pen/KgGPGX

If anyone has suggestions on what to adjust here, that'd be super helpful. Thank you!

P.S. I have looked at (and tried) several of the similar questions that have been asked about JS number formatting (such as this) but my issue seems to be more a matter of me incorrectly targeting the value within the span tag and not with the JS itself. However, if there is a similar question related to targeting elements correctly, I'd be happy to pointed towards that as well!

Also, let me know if there's additional information I need to provide.

Thanks!

<div id="container">
<div id="inner">
<div id="message_container">

<div id="roi-headline">
<h2>Cool Header</h2>
<h1>An even cooler headline</h1> 
</div>

<div id="roi-values">
<div id="roi-value-1">
<div id="roi-value-1-graph" style="width:75%;">
    <em>from</em> <sup>$</sup>
    <span id="old" class="current">
        100000
    </span>
</div>
</div>

<div id="roi-value-2">
<div id="roi-value-2-graph" style="width:100%;">
    <em>to</em>
    <span><sup>$</sup>15<sup>K</sup></span>
</div>
</div>

</div>

<div id="button">Learn More</div>  
</div>
</div>
</div> 

Edit: Updated the following var to include .innerText

/* Attempt 1 */ 
var num = document.getElementById("old").innerText;
function nFormatter(num) {

 if (num >= 1000000000) {
    return (num / 1000000000).toFixed(1).replace(/\.0$/, '') + 'G';
 }
 if (num >= 1000000) {
    return (num / 1000000).toFixed(1).replace(/\.0$/, '') + 'M';
 }
 if (num >= 1000) {
    return (num / 1000).toFixed(1).replace(/\.0$/, '') + 'K';
 }
 return num;
};

/* Attempt 2 */
var value = "span.current";
function prettifyNumber(value) {    
var thousand = 1000;
var million = 1000000;
var billion = 1000000000;
var trillion = 1000000000000;
if (value < thousand) {
    return String(value);   
}

if (value >= thousand && value <= 1000000) {
     return  Math.round(value/thousand) + 'k';   
}

if (value >= million && value <= billion) {
    return Math.round(value/million) + 'M';   
}

if (value >= billion && value <= trillion) {
    return Math.round(value/billion) + 'B';   
}

else {
    return Math.round(value/trillion) + 'T';   
}

};
Community
  • 1
  • 1
L. G.
  • 33
  • 7
  • `document.getElementById("old")` returns a DOM element, not a number.. If you pass `document.getElementById("old").innerText` into your `nFormatter()` function, you get the desired output – mhodges Oct 17 '16 at 21:10
  • @mhodges Thank you for the response! I had tried that previously and just adjusted the code again to include that and am still not seeing it affect the 10000 within that span:/ Is it correct for the variable 'num' to be defined outside of the function? – L. G. Oct 17 '16 at 21:18
  • The only way your changes are going to be reflected in the DOM is if you set the innerText or innerHTML of the DOM element. Are you doing that anywhere? I don't see it in the code you've provided – mhodges Oct 17 '16 at 21:20

3 Answers3

0

Your functions work just fine - if you want the changes reflected in the DOM, you will have to set the innerText or innerHTML of your #old element.

You can accomplish this by changing your nFormatter() function like so:

var num = document.getElementById("old");
function nFormatter(num) {
  var oldNumber = num.innerText;
  var newNumber = oldNumber;
  if (oldNumber >= 1000000000) {
    newNumber = (oldNumber / 1000000000).toFixed(1).replace(/\.0$/, '') + 'G';
  }
  if (oldNumber >= 1000000) {
    newNumber = (oldNumber / 1000000).toFixed(1).replace(/\.0$/, '') + 'M';
  }
  if (oldNumber >= 1000) {
    newNumber = (oldNumber / 1000).toFixed(1).replace(/\.0$/, '') + 'K';
  }
  num.innerText = newNumber;
}

OR you can pass it just the number and use the returned result to set the innerText or innerHTML

var num = document.getElementById("old");
function nFormatter(num) {
  if (num >= 1000000000) {
    return (num / 1000000000).toFixed(1).replace(/\.0$/, '') + 'G';
  }
  if (num >= 1000000) {
    return (num / 1000000).toFixed(1).replace(/\.0$/, '') + 'M';
  }
  if (num >= 1000) {
    return (num / 1000).toFixed(1).replace(/\.0$/, '') + 'K';
  }
  return num;
}
num.innerText = nFormatter(num.innerText);
mhodges
  • 10,938
  • 2
  • 28
  • 46
  • Ah, thank you! I just wasn't understanding how to set the innerText but I see what you mean now and will just use the returned result to set the innerText. – L. G. Oct 17 '16 at 21:58
0

Here is an updated codepen. You were doing <= the upper bound and >= the lower bound on each if statement. This would cause multiple if's to be true. Also, have to make sure you call your function after you create it and I would advise not using the same name for a global variable and a parameter to a function. It can cause some confusing behavior.

var value1 = document.getElementById("old");
function prettifyNumber(value) {    
var thousand = 1000;
var million = 1000000;
var billion = 1000000000;
var trillion = 1000000000000;

value = parseInt(value);

var retVal = "";
if (value < thousand) {
    retVal = String(value);   
}

if (value >= thousand && value < million) {
     retVal =  Math.round(value/thousand) + 'k';   
}

if (value >= million && value < billion) {
    retVal = Math.round(value/million) + 'M';   
}

if (value >= billion && value < trillion) {
    retVal = Math.round(value/billion) + 'B';   
}

if(value > trillion) {
    retVal = Math.round(value/trillion) + 'T';   
}

value1.innerHTML = retVal;

};

prettifyNumber(value1.innerText);
big_water
  • 3,024
  • 2
  • 26
  • 44
  • Thank you! I definitely see now why those issues were keeping it from working. And thanks for clarifying calling the function at the end - I had tried a few things and wasn't targeting it quite right. – L. G. Oct 17 '16 at 21:54
0

In both attempts you are missing a basic step, that is: you are not invoking the function.

The fact that you are calling the function argument with the same name of the variable is not enough. On the contrary, the function argument hides the external variable, that therefore won't be accessible by name inside the function. Anyway, this is just an additional consideration, not the real problem.

Consider attempt 1. After the function definition, you should add the actual invocation, something like

document.getElementById("old").innerText = nFormatter(num);

Attempt 2 has basically the same issue (i.e. no function call), but also you must obtain the desired element (with document.querySelectorAll for instance) before handing in to the prettifyNumber function.

Summarizing, the first attempt is the most correct one among the two, keeping in mind that both functions work fine.

FstTesla
  • 844
  • 1
  • 7
  • 10