Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: count js console error #181

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fix: count js console error #181

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1a835c1
fix: count js console error
gitcommitshow Nov 20, 2023
f636963
fix: add null check
gitcommitshow Nov 20, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 9 additions & 10 deletions assets/js/count.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,14 @@ minute = second * 60,
hour = minute * 60,
day = hour * 24;

var countDown = new Date('Nov 27, 2021 10:00:00').getTime(),
var countDown = new Date('Jan 27, 2024 10:00:00').getTime(),
x = setInterval(function() {

var now = new Date().getTime(),
distance = countDown - now;

document.getElementById('days').innerText = Math.floor(distance / (day)),
document.getElementById('hours').innerText = Math.floor((distance % (day)) / (hour)),
document.getElementById('minutes').innerText = Math.floor((distance % (hour)) / (minute)),
document.getElementById('seconds').innerText = Math.floor((distance % (minute)) / second);

var now = new Date().getTime(),
distance = countDown - now;
if(distance > 0){
document.getElementById('days').innerText = Math.floor(distance / (day)),
document.getElementById('hours').innerText = Math.floor((distance % (day)) / (hour)),
document.getElementById('minutes').innerText = Math.floor((distance % (hour)) / (minute)),
document.getElementById('seconds').innerText = Math.floor((distance % (minute)) / second);
}
}, second)
Copy link

@coderabbitai coderabbitai bot Nov 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes correctly update the countdown target date and add a conditional check to ensure that the countdown does not display negative values. This should resolve the issue with JavaScript console errors related to past dates. However, it would be beneficial to also handle the scenario when the countdown reaches zero. Currently, the code does not specify what should happen once the countdown is finished. Consider updating the display to show a message or hide the countdown elements when the target date and time are reached.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the code has issue as it does not deal with the case when an html element is not found and we still call its innerText method

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gitcommitshow You're absolutely right. The current implementation assumes that the HTML elements with the specified IDs will always be present. To make the code more robust, we should add checks to ensure that these elements exist before attempting to update their innerText. Here's how we can modify the code to include these checks:

var now = new Date().getTime(),
distance = countDown - now;
if(distance > 0){
  var daysElement = document.getElementById('days'),
      hoursElement = document.getElementById('hours'),
      minutesElement = document.getElementById('minutes'),
      secondsElement = document.getElementById('seconds');
  
  if(daysElement) daysElement.innerText = Math.floor(distance / (day));
  if(hoursElement) hoursElement.innerText = Math.floor((distance % (day)) / (hour));
  if(minutesElement) minutesElement.innerText = Math.floor((distance % (hour)) / (minute));
  if(secondsElement) secondsElement.innerText = Math.floor((distance % (minute)) / second);
}

This way, we only attempt to update the innerText of an element if it has been successfully retrieved from the DOM. Thank you for pointing out this improvement.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai review

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@gitcommitshow Thank you for the review. If there are any specific issues or further improvements you would like to suggest, please let me know, and I'll address them accordingly.