Submitted almost 3 years ago

Using JavaScript and CSS to dynimcally size charts from JSON file

@mshaynerush

A solution to the Expenses chart component challenge

Solution retrospective

If you can see a better way to add the elements to the page. I used a lot of divs, and while I don't think that's a terrible idea, I am wondering if I made this more complex than it had to be.

Also I am sure there are things I could have done with less code, so any feedback in that area would be sweet.

Thank you for taking time to review.

Code

Please log in to post a comment

Community feedback

romila•3,550
@romila2003
Posted almost 3 years ago
Hi Shayne,

Congratulations 🎉 for completing this challenge, the chart component looks great and responsive. There are some issues regarding your HTML, and CSS I want to address:
1. The <br> tag seems quite unnecessary in your code since the flex property centers the card already and there is enough gap between the header and the top.
2. It is best practice to wrap the main content within the main tag which would ensure that your content is wrapped within the correct landmarks e.g. <main class="container"></main>
3. You should also wrap the header within the header tag e.g <header></header>
4. Instead of using a <div> to get the line under the chart, you can use the border-top property on the .chart-footer instead.
5. Since you used the section tag, you need to a header (any header between h2 to h6).
Overall, great attempt and wish you the best for your future projects 👍.

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