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

Expenses Chart w/ SASS & Vanilla JS

#sass/scss
Robert• 170

@waffleflopper

Desktop design screenshot for the Expenses chart component coding challenge

This is a solution for...

  • HTML
  • CSS
  • JS
2junior
View challenge

Design comparison


SolutionDesign

Solution retrospective


Anything you find that I can improve on, please let me know! Can't get better without critique and I don't want to cement any bad habits!

Cheers!

Community feedback

P
Eileen dangelo• 1,600

@Eileenpk

Posted

Hi Robert! your project looks good, and this might be a helpful tip.

In your HTML, you have nested <main> tags. It is best practice to only have one per page as screen readers and SEO uses the landmark tag to let users and bots know that this is the "main" content of the page.

Also in your JS you have

const dynamic = document.getElementById("chart");

if (dynamic) {
    ...
}

I think you can take out the if statement because you have hard-coded the id=dynamic in your HTML and so it will always be a true value.

Here is a link to learn more about semantic elements. https://www.w3schools.com/html/html5_semantic_elements.asp

Hope you found this helpful!

Marked as helpful

1

Robert• 170

@waffleflopper

Posted

@Eileenpk Good morning! Firstly I would like to thank you for the time you took to look through the code and give me feedback. As I'm gearing up to finish out my last year of active duty service I've been leaning full tilt into brushing up on my front-end skills and catching up with everything that's happened since I last did web development in the early 2010s.

Thanks for the link on the semantic HTML. The vast majority of the semantic elements came out after I joined the military so I'm very... uncomfortable with their use. I made a stab at using them in the way that made sense in my brain but I didn't take into account the accessibility portion of what they provide, so only having one main makes total sense now in my head. Thanks for that! I'm guessing, then, that I would use section tags instead?

Appreciate the feedback on JS as well. I've been doing a lot of SvelteKit recently and it just became a habit to have conditionals in a context like that. It's always funny the things that make sense at the time until someone points to it and says "why?" =).

Again, I appreciate the time you took to look through the project. A friend of mine was a founder of Radix and took a look from a designer compliance point of view so I'm grateful to you for the code feedback.

I'll send you a connection on LinkedIn. Never too early to start networking. If you happen to find yourself needing help on... well anything - please don't hesitate to ask. I'm always happy to return a favor in any way needed.

0
P
Eileen dangelo• 1,600

@Eileenpk

Posted

@waffleflopper,

Thanks for connecting with me on LinkedIn. I sent you a pull request on GitHub with some of the changes I would do for your project so that you can see the diff in the code. Hopefully, it's helpful to you. I am always happy to look over code and debug if you ever need it in the future, don't hesitate to reach out via LinkedIn.

Cheers!

Marked as helpful

1
Robert• 170

@waffleflopper

Posted

@Eileenpk Thank you! Above and beyond :)

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