1

I'm trying to make a count up timer but the word days doesn't seem to want to show up on the screen

HTML

<div class="countup" id="countup1">
<div class="yrs"><span class="timeel years">00</span><span class="countupYrs">YRS</span></div> |
<div class="days"><span class="timeel days">00</span><span class="countupDays">DAYS</span></div> |
<div class="hrs"><span class="timeel hours">00</span><span class="countupHrs">HRS</span></div> |
<div class="mins"><span class="timeel minutes">00</span><span class="countupMins">MINS</span></div> |
<div class="secs"><span class="timeel seconds">00</span><span class="countupSecs">SECS</span></div>
                    </div>

JS

window.onload = function () {
    countUpFromTime("Dec 25, 2021 13:20:00", 'countup1');
};
function countUpFromTime(countFrom, id) {
    countFrom = new Date(countFrom).getTime();
    var now = new Date(),
        countFrom = new Date(countFrom),
        timeDifference = (now - countFrom);

    var secondsInADay = 60 * 60 * 1000 * 24,
        secondsInAHour = 60 * 60 * 1000;

    days = Math.floor(timeDifference / (secondsInADay) * 1);
    years = Math.floor(days / 365);
    if (years > 1) { days = days - (years * 365) }
    hours = Math.floor((timeDifference % (secondsInADay)) / (secondsInAHour) * 1);
    mins = Math.floor(((timeDifference % (secondsInADay)) % (secondsInAHour)) / (60 * 1000) * 1);
    secs = Math.floor((((timeDifference % (secondsInADay)) % (secondsInAHour)) % (60 * 1000)) / 1000 * 1);

    var idEl = document.getElementById(id);
    idEl.getElementsByClassName('years')[0].innerHTML = years;
    idEl.getElementsByClassName('days')[0].innerHTML = days;
    idEl.getElementsByClassName('hours')[0].innerHTML = hours;
    idEl.getElementsByClassName('minutes')[0].innerHTML = mins;
    idEl.getElementsByClassName('seconds')[0].innerHTML = secs;

    clearTimeout(countUpFromTime.interval);
    countUpFromTime.interval = setTimeout(function () { countUpFromTime(countFrom, id); }, 1000);
}

I've tried doing

<div class="days"><span class="timeel days">00</span><span id="countupDays" class="countupDays">DAYS</span></div>

JS

document.getElementById('countupDays').innerHTML = "DAYS";

to add the word days but that did not work because the timer would stop counting

Alex Naemi
  • 33
  • 5
  • None of your elements have an id of `element`, so your attempt won't find any elements to update. Which element are you attempting to update? – mykaf Dec 08 '22 at 18:14
  • @mykaf i used it as an example and realised people would get confused, its updated now – Alex Naemi Dec 08 '22 at 18:15
  • 1
    You have two elements with class="days": `
    00DAYS
    `; the first is the container of your targeted element, so its innerHTML is being updated instead of ".timeel.days"
    – mykaf Dec 08 '22 at 18:19
  • 1
    FYI: `idEl.getElementsByClassName('years')[0].innerHTML` is not good code for two reasons. 1) [`getElementsByClassName()` should be avoided](https://stackoverflow.com/questions/54952088/how-to-modify-style-to-html-elements-styled-externally-with-css-using-js/54952474#54952474) (especially if you aren't actually interested in getting a node list back) and 2) [`.innerHTML` should be avoided](https://developer.mozilla.org/en-US/docs/Web/API/Element/innerHTML) (when possible) for performance and security concerns (use `.textContent` instead). – Scott Marcus Dec 08 '22 at 18:26

2 Answers2

2

It's because you had a side effect having the outer element with class days

window.onload = function() {
  countUpFromTime("Dec 25, 2021 13:20:00", 'countup1');
};

function countUpFromTime(countFrom, id) {
  countFrom = new Date(countFrom).getTime();
  var now = new Date(),
    countFrom = new Date(countFrom),
    timeDifference = (now - countFrom);

  var secondsInADay = 60 * 60 * 1000 * 24,
    secondsInAHour = 60 * 60 * 1000;

  days = Math.floor(timeDifference / (secondsInADay) * 1);
  years = Math.floor(days / 365);
  if (years > 1) {
    days = days - (years * 365)
  }
  hours = Math.floor((timeDifference % (secondsInADay)) / (secondsInAHour) * 1);
  mins = Math.floor(((timeDifference % (secondsInADay)) % (secondsInAHour)) / (60 * 1000) * 1);
  secs = Math.floor((((timeDifference % (secondsInADay)) % (secondsInAHour)) % (60 * 1000)) / 1000 * 1);

  var idEl = document.getElementById(id);
  idEl.getElementsByClassName('years')[0].innerHTML = years;
  idEl.getElementsByClassName('days')[0].innerHTML = days;
  idEl.getElementsByClassName('hours')[0].innerHTML = hours;
  idEl.getElementsByClassName('minutes')[0].innerHTML = mins;
  idEl.getElementsByClassName('seconds')[0].innerHTML = secs;

  clearTimeout(countUpFromTime.interval);
  countUpFromTime.interval = setTimeout(function() {
    countUpFromTime(countFrom, id);
  }, 1000);
}
<div class="countup" id="countup1">
  <div class="yrs"><span class="timeel years">00</span><span class="countupYrs">YRS</span></div> |
  <div class="days-unique-name"><span class="timeel days">00</span><span class="countupDays">DAYS</span></div> |
  <div class="hrs"><span class="timeel hours">00</span><span class="countupHrs">HRS</span></div> |
  <div class="mins"><span class="timeel minutes">00</span><span class="countupMins">MINS</span></div> |
  <div class="secs"><span class="timeel seconds">00</span><span class="countupSecs">SECS</span></div>
</div>
IT goldman
  • 14,885
  • 2
  • 14
  • 28
  • While your answer is technically correct, a better approach would be to fix the CSS selection rather than messing with the HTML. ``document.querySelector(`#${id} > span.days`).innerHTML = days`` removes the ambiguity. – Tibrogargan Dec 08 '22 at 18:22
  • In this case it's better to fix the ambiguous html, even at the price of changing css as well. This is in order to maintain uniformity. And to prevent ambiguous html. – IT goldman Dec 08 '22 at 18:28
  • Correction: ``document.querySelector(`#${'countup1'} span.days`).innerHTML = days`` – Tibrogargan Dec 08 '22 at 18:29
  • Except the new HTML is patently not uniform. The `.days-unique-name` element is different to every other element of the counter component. (But yes, the HTML overall wasn't well designed in the first place - but it wasn't ambiguous. The selection method being used was) – Tibrogargan Dec 08 '22 at 18:30
0

The issue is really how you're selecting your elements. Because there are multiple elements with the class days, using getElementsByClass is returning multiple elements so idEl.getElementsByClassName('days')[0] is selecting the div, not the span. You could consider removing un-needed classes from your HTML but it's actually perfectly possible to select the correct element with the right selector.

Here is a version that does that (also uses setInterval instead of setTimeout to avoid having to manually restart the timeout).

var timeoutId

window.onload = function () {
  timeoutId = countUpFromTime("Dec 25, 2021 13:20:00", 'countup1')
};

function countUpFromTime(from, id) {
  const secondsInADay = 60 * 60 * 1000 * 24,
        secondsInAHour = 60 * 60 * 1000;

  const countFrom = new Date(from).getTime();

  // The keys of counter are the class names of the spans that should contain the result of the value method
  const counter = {
    days: (timeDifference) => ( Math.floor(timeDifference / (secondsInADay) * 1) % 365 ),
    years: (timeDifference) => Math.floor(Math.floor(timeDifference / (secondsInADay) * 1) / 365),
    hours: (timeDifference) => Math.floor((timeDifference % (secondsInADay)) / (secondsInAHour) * 1),
    minutes: (timeDifference) => Math.floor(((timeDifference % (secondsInADay)) % (secondsInAHour)) / (60 * 1000) * 1),
    seconds: (timeDifference) => Math.floor((((timeDifference % (secondsInADay)) % (secondsInAHour)) % (60 * 1000)) / 1000 * 1)
  }

  return setInterval( () => {
    let now = new Date(),
        timeDifference = (now - new Date(countFrom));

    for (const [key, value] of Object.entries(counter)) {
      document.querySelector(`#${id} span.${key}`).innerHTML = value(timeDifference)
    }
  }, 1)
}
<body>
<div class="countup" id="countup1">
<div class="yrs"><span class="timeel years">00</span><span class="countupYrs">YRS</span></div> |
<div class="days"><span class="timeel days">00</span><span class="countupDays">DAYS</span></div> |
<div class="hrs"><span class="timeel hours">00</span><span class="countupHrs">HRS</span></div> |
<div class="mins"><span class="timeel minutes">00</span><span class="countupMins">MINS</span></div> |
<div class="secs"><span class="timeel seconds">00</span><span class="countupSecs">SECS</span></div>
</div>

You could replace the counter Object with a Map and use the result of the selector as the key, but that seems excessively complicated for minor performance gains:

    const counter = new Map()
    counter.set(document.querySelector(`#${id} span.days`), (timeDifference) => ( Math.floor(timeDifference / (secondsInADay) * 1) % 365 ))
    counter.set(document.querySelector(`#${id} span.years`), etc etc
    ...

    for (const [key, value] of counter.entries()) {
      key.innerHTML = value(timeDifference)
    }  
Tibrogargan
  • 4,508
  • 3
  • 19
  • 38