Code Review Dasmoto's Arts

Hey, I wrote this code for the project.
I’m unsure if this is the most efficient way for this work, do you you have any suggestions?

I saw a lot of codes which have included themeta data, did I miss this so far in the unit?

HTML:

    <!DOCTYPE html>
    <head>
        <meta charset ="UTF-8">
        <meta name="viewport" content="width=device-width, initial-scale=1.0">
        <link href="C:\Users\phili\Documents\Programmieren\Dasmoto's Arts & Crafts\resources\css\index.css" rel="stylesheet">
        <title>Dasmoto's Arts & Crafts</title>
    </head>

    <body>
        <header>
            <h1 id="header">Dasmoto's Arts & Crafts</h1>
        </header>

        <main>
            <section class="brushes">
                <h3 class="Header1">Brushes</h3>
                <img alt="brushes" src="C:\Users\phili\Documents\Programmieren\Dasmoto's Arts & Crafts\resources\hacksaw.jpeg">
                <h6>Hacksaw Brushes</h6>
                <p>
                    Made of the highest quality oak, Hacksaw brushes are known for their and ability to hold paint in large amounts. Available in different sizes. <span class="s1">Starting at $3.00 /brush.</span>
                </p>
            </section>
            <section class="frames">
                <h3 class="Header2">Frames</h3>
                <img alt="frames" src="C:\Users\phili\Documents\Programmieren\Dasmoto's Arts & 
                 Crafts\resources\frames.jpeg">
                <h6>Art Frames (assorted)</h6>
                <p>Assorted frames made of different material, including MDF, birchwood, and PDE. Select frames can be 
                      sanded and painted according to your needs. <span class="s2">Starting at $2.00/ frame.</span></p>
            </section>
            <section class="paint">
                <h3 class="Header3">Paint</h3>
                <img alt="Paint" src="C:\Users\phili\Documents\Programmieren\Dasmoto's Arts & 
                   Crafts\resources\finnish.jpeg">
                <h6>Clean Finnish Paint</h6>
                <p>Imported paint from Finland. Over 256 colors available in-store, varying in quantity (1 oz. to 8 oz.). Clean 
                      Finnish paint microbinds to canvas, increasing the finish and longevity of any artwork. <span 
                      class="s3">Starting at $5.00/ tube.</span></p>
            </section>
        </main>
    </body>

CSS:

h1 {
    background-image: url("https://content.codecademy.com/courses/freelance-1/unit-2/pattern.jpeg?_gl=1*wynx75*_ga*OTA0NTQ0MTIxMi4xNjgyMzQ4NTA2*_ga_3LRZM6TM9L*MTY5MDIxODQwNS41My4xLjE2OTAyMTg0MTMuMC4wLjA.");
    font-family: Arial, Helvetica, sans-serif;
    font-size: 100px;
    font-weight: bold;
    color: khaki;
    text-align: center;
    }

h3 {
    font-family: Arial, Helvetica, sans-serif;
    font-size: 32px;
    font-weight: bold;
    color: white;
}

h3.Header1 {
    background-color: mediumspringgreen;
}

h3.Header2 {
    background-color: lightcoral;
}

h3.Header3 {
    background-color: skyblue;
}

h6 {
    font-family: Arial, Helvetica, sans-serif;
    font-weight: bold;
    font-size: 20px;
}

span {
    font-family: Arial, Helvetica, sans-serif;
    font-weight: bold;
    color: blue;
}

p {
    font-family: Arial, Helvetica, sans-serif;
}

Thanks for Suggestions :slight_smile:

Hi there :grinning: Great take on completing this project :smiley:

First off - the metadata you see in some code is probably because people are training to include them or because the use the shortcut (!) in VS Code. The shortcut will make a HTML template with the metadata included :blush:

About your code, there are some things that stands out for me, that you can improve :blush:

  1. On each section you’ve made a class, which you dont define in your CSS = you are not using them. And they are not needed :slightly_smiling_face: Instead you can use comments to section your code and stick to minimal use of code :slight_smile:

  2. On each H3-tag you’ve made a class (Header1, Header2, Header3), but in your CSS you call them ‘h3.Header1’ etc. You only need to call the class by ‘.Header1’, ‘.Header2’, ‘.Header3’ :slight_smile: On a side note you could have made them an id instead of a class, because you are only using each one once
    HTML

<h3 id="Header1">Brushes</h3>

CSS

#Header1 {
    background-color: mediumspringgreen;
}
  1. In your p-tags you are using the span-tag which each have a class (s1, s2, s3), but you are not defining them in your CSS. In your CSS you are only defining ‘span’.

  2. When using heading-tags (h1, h2, h3 etc), you have to use the hierarchy. You are using H1, then H3 and the H6 :slight_smile: The right way would be H1 (dasmotos arts…), H2 (brushes, frames, paint) and then H3 (Hacksaw brushes etc). A good thing to read about is semantic HTML :slight_smile:

  3. In your CSS you are defining part of the code several times ex. the font-family and font-weight. You are doing this on every h-tags and span. You can make this part of the code global; meaning you can define the font under the ‘*’ in the CSS. This will make the font Helvetica all over the site.

* {
    font-family: "Helvetica", sans-serif;
}

You can define the font-weight by defining it once for several tags:

h1, h2, span {
    font-weight: bold;
}

I hope this makes sense for you and gives you the idea how you can write your code more efficient :grinning:

Here is my take on the solution:

HTML (semantic):

<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="UTF-8" />
    <meta http-equiv="X-UA-Compatible" content="IE=edge" />
    <meta name="viewport" content="width=device-width, initial-scale=1.0" />
    <link rel="stylesheet" href="./resources/css/style.css" />
    <title>Dasmoto's Arts &#38; Crafts - testproject</title>
  </head>
  <body>
    <header>
      <h1>Dasmoto's Arts &#38; Crafts</h1>
    </header>
    <main>
      <!-- brushes -->
      <section>
        <h2 id="brushes">Brushes</h2>
        <figure>
          <img src="./resources/images/image2.jpeg" alt="paint brushes" />
        </figure>
        <h3>Hacksaw Brushes</h3>
        <p>
          Made of the highest quality oak, Hacksaw brushes are known for their
          weight and ability to hold paint in large amounts. Available in
          different sizes. <span>Starting at $3.00 / brush.</span>
        </p>
      </section>
      <!-- frames -->
      <section>
        <h2 id="frames">Frames</h2>
        <figure>
          <img
            src="./resources/images/image3.jpeg"
            alt="many colorful frames"
          />
        </figure>
        <h3>Art Frames (assorted)</h3>
        <p>
          Assorted frames made of different material, including MDF, birchwood,
          and PDE. Select frames can be sanded and painted according to your
          needs. <span>Starting at $2.00 / frame.</span>
        </p>
      </section>
      <!-- paint -->
      <section>
        <h2 id="paint">Paint</h2>
        <figure>
          <img src="./resources/images/image4.jpeg" alt="oilpaint tubes" />
        </figure>
        <h3>Clean Finnish Paint</h3>
        <p>
          Imported paint from Finland. Over 256 colors available in-store,
          varying in quantity (1 oz. to 8 oz.). Clean Finnish paint microbinds
          to canvas, increasing the finish and longevity of any artwork.
          <span>Starting at $5.00 / tube.</span>
        </p>
      </section>
    </main>
  </body>
</html>

CSS:

* {
    box-sizing: border-box;
    font-family: "Helvetica", sans-serif;
}

h1, h2, span {
    font-weight: bold;
}

h1 {
    font-size: 100px;
    color: khaki;
    text-align: center;
    background-image: url(../images/image1.jpeg);
}

h2 {
    font-size: 32px;
    color: white;
}

span {
    color: blue;
}

#brushes {
    background-color: mediumspringgreen;
}

#frames {
    background-color: lightcoral;
}

#paint {
    background-color: skyblue;
}

Wow thanks a lot for your detailed answer!! :slightly_smiling_face: I’ll owrk on it