Design comparison
Solution retrospective
How can I clean up my JS on this?
Community feedback
- @elaineleungPosted about 2 years ago
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
(afor
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:-
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 usequerySelectorAll
to select all the cards at once:const allCards = document.querySelectorAll(".card");
-
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>
-
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 abutton
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 helpful1@ChaffexdPosted about 2 years ago@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@elaineleungPosted about 2 years ago@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 helpful1@ChaffexdPosted about 2 years ago@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@elaineleungPosted about 2 years ago@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 -
- @ekkas303Posted about 2 years ago
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@ChaffexdPosted about 2 years ago@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@ekkas303Posted about 2 years ago@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 GitHubJoin 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