Skip to content
Submitted about 2 years ago

Using JavaScript and CSS to dynimcally size charts from JSON file

Shayne Rushton•60
@mshaynerush
Preview site (opens in new tab)View code (opens in new tab)
This is a solution for...Expenses chart component
  • HTML
  • CSS
  • JS
2junior

Design comparison

SolutionDesign

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.

Community feedback

  • romila•3,570
    @romila2003Posted about 2 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 👍.

    0

Please log in to post a comment

Log in with GitHub

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