Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

Submitted

Time tracking dashboard

@Chaffexd

Desktop design screenshot for the Time tracking dashboard coding challenge

This is a solution for...

  • HTML
  • CSS
  • JS
2junior
View challenge

Design comparison


SolutionDesign

Solution retrospective


How can I clean up my JS on this?

Community feedback

Elaine 11,400

@elaineleung

Posted

Hi Shane, first off, good job in building this dashboard! About the JS, the key here is really to use iteration for elements with the same style and content type, and for me I usually use forEach (a for loop works fine also). You can use this for both the timeframe labels (daily, weekly, monthly) and also for the six stats cards, but here I'll just show you what you can do with the cards:

  1. Instead of assigning an id for each area and then grabbing each one in your script, I would give them a class of .card and then use querySelectorAll to select all the cards at once:

      const allCards = document.querySelectorAll(".card");
    
  2. In the HTML, add some classes to the elements showing the "previous" and "current" stats:

    <div class="item2 card">
      <div class="work"></div>
      <div class="workTime">
        <div class="title">
          <p class="workTitle">Work</p>
          <p class="dotIcon"><a href="#">...</a></p>
        </div>
        <div class="timeRecorded stats">
          <p class="stats-current">4hrs</p>
          <p class="stats-previous">Last Week - 5hrs</p>
        </div>
      </div>
    </div>
    
  3. After that, you can loop through allCards like this:

    daily.addEventListener("click" , () => { 
       daily.classList.add("active");
       weekly.classList.remove("active");
       monthly.classList.remove("active");
    
       allCards.forEach( (card, idx) => {
       const currentStats = card.querySelector(".stats-current")
       const previousStats = card.querySelector(".stats-previous")
    
       currentStats.textContent = `${data[idx].timeframes.daily.current}hrs`;
       previousStats.textContent = `Yesterday - ${data[idx].timeframes.daily.previous}hrs`;
       })
    })
    

That's essentially all there is! Another suggestion I can give you is, instead of using a for the clickable timeframe label element, use a button or a similar kind of input element instead (I used radio inputs in mine). The reason for this is, the function of a link is for website navigation, but here, the element is being used to perform an action of changing information and not actually directing the user to another page. It's very common to see a button being used for a link (like the "call to action" links) or a link being used as a button like in this case, and if not used properly, this is something that can give you accessibility issues.

Hope this helps you a bit!

Marked as helpful

1

@Chaffexd

Posted

@elaineleung This is an amazing piece of feedback, thank you. I have been trying to implement the forEach method for a while now but cannot do it since "Uncaught TypeError: Cannot set properties of null (setting 'textContent')" on lines 77 and 73 which are (full code is copied from your comment): 73: allCards.forEach((card, idx) => { 77: currentStats.textContent = ${data[idx].timeframes.daily.current}hrs;

Did this work for you when either doing a source override or hosting locally? I'm unable to get it to work sadly.

0
Elaine 11,400

@elaineleung

Posted

@Chaffexd Hi Shane, I decided to just fork your repo and did the changes there, made a pull request just now so you can compare the code. I didn't change the class names this time and just used your original ones instead. Let me know whether that works or not!

Also, I changed the loading tab to "weekly" instead of "daily" so that I can match the original design, but feel free to change it back!

Marked as helpful

1

@Chaffexd

Posted

@elaineleung You're... a saint. Literally eliminated more than a 100 lines of code.. I really need to brush up on my iterators to make my code cleaner. Thank you so much

1
Elaine 11,400

@elaineleung

Posted

@Chaffexd Glad I could help! Actually you can try iterating the three timeframe elements and refactor the code further. I briefly thought about doing it but I think it'd be better if you get to practice as well instead of me refactoring everything. Anyway, have fun! 😊

1
Brannon 80

@ekkas303

Posted

hey bro just want to ask how did you connect json file to javascript because i did paste this: fetch("/data.json") .then((response) => { return response.json(); }) this is what i got : File "c:\Users\5750G\Desktop\time-tracking-dashboard-main\index.js", line 6 .then((response) => { ^ SyntaxError: invalid syntax PS C:\Users\5750G.vscode> Any idea why its not working?

0

@Chaffexd

Posted

@ekkas303 Hey Brannon, this fetch is looking for the path of this json file which is where I had it located in my files. Just make sure you're using the correct path and the promise should resolve. If not, can you link me to your code on github and I can take a look?

0
Brannon 80

@ekkas303

Posted

@Chaffexd hey bro this is my github site i havent started coding in javascript but please do check out whats the problem.Thanks

0

Please log in to post a comment

Log in with GitHub
Discord logo

Join our Discord community

Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!

Join our Discord