Dasmotos Arts and Crafts, code review

Projects Off-platform Practice Projects
1

Hey gang, just finished my first off platform project. after completing I took a look at the solution code and it slightly differs to mine. Could someone take a look at my code and see if anything needs changing or If i could have done something more efficiently.
Thanks!
https://github.com/Liam1Whelan/Dasmotos-Arts-and-Crafts

2

Hey Liam!

Nice work! I did the same project just 2 days ago and I did notice a few things in your code that could be improved for various reasons. I’ll try and go through the different things below.

I’ll start off with index.html.
I see you divided up the products using div, a better practice is you use and/or . This is better for SEO and readability. Using semantic elements gives the code a more understandable structure e.g “here is a section with a few articles in them”
Below is how I structured the code in index.html.

<body>
  <header>
    <h1>Dasmoto's Arts & Crafts</h1>
  </header>
  <main>
    <section>
      <article>
        <h2>Product Group 1</h2>
        <img src="" alt="" />
        <h3>Product 1</h3>
        <p>information about product 1</p>
      </article>
      <article>
        <h2>Product Group 2</h2>
        <img src="" alt="" />
        <h3>Product 1</h3>
        <p>information about product 1 in group 2</p>
      </article>
      <article>
        <h2>Product Group 3</h2>
        <img src="" alt="" />
        <h3>Product 1</h3>
        <p>information about product 1 in group 3</p>
      </article>
    </section>
  </main>
</body>

You are also missing the alt="" attribute on your images, they are important for screen readers and the like to make sense of the website.

Another thing regarding SEO and accessibility I noticed is that you are using <h1>, <h3>, <h5>. The best practice is to use them in order. This makes it easier for search engines to index you page and as i understood it, it will also help with accessibility (screenreaders etc)

Here are a few pointers for CSS.
I saw that you made three different CSS selectors defining the same thing, a more efficient way of doing this is writing one CSS class selector and using it in more than one place in index.html.
E.g:
style css

price-text {
  font-weight: bold;
  color: blue;
}

index.html

Product 1 <span class="price-text">Starting at $3.00 / brush.</span>
Product 2 <span class="price-text">Starting at $3.00 / brush.</span>
Product 3 <span class="price-text">Starting at $3.00 / brush.</span>

The great thing is that if you in the future would have to change the style of “price-text” you only have to make the changes to on CSS selector instead of 3.

I see that you have defined font-family: Helvetica; in several places in your CSS. Since you have defined it in the body selector there is no need to define it in more places and could in the future lead to problems if you would like to change the font family throughout the webpage.

I hope this helps you in your future coding endeavors!
Keep up the good work :slight_smile:

3

Hey! sorry it took so long to get back to you, your review is so well received and greatly appreciated!
Ill go back to the drawing board and create something new with the info! Thanks so much!