Dasmoto's Arts & Crafts Project- Code Review

Hey everyone! This is my first attempt at the Dasmoto’s Arts & Crafts Project. Any feedback would be greatly appreciated!

<!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">
    <title>Product Page</title>
    <link href="\resources\css\index.css" rel='stylesheet' type='text/css'>
</head>
<body>
    <header>
        <h1>Dasmoto's Arts & Crafts</h1>
    </header>
    <main>
        <div class="brushes">
            <h2>Brushes</h2>
            <img src="https://content.codecademy.com/courses/freelance-1/unit-2/hacksaw.jpeg" alt="Image cannot display">
            <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.</p>
            <p class="blue">Starting at $3.00/brush</p>
        </div>
        <div class="Frames">
            <h2>Frames</h2>
            <img src="https://content.codecademy.com/courses/freelance-1/unit-2/frames.jpeg" alt="Image cannot display">
            <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.</p>
            <p class="blue">Starting at $2.00/frame.</p>
        </div>
        <div class="Paint">
            <h2>Paint</h2>
            <img src="https://content.codecademy.com/courses/freelance-1/unit-2/finnish.jpeg" alt="image cannot display">
            <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.</p>
            <p class="blue">Starting at $5.00/tube.</p>
        </div>
    </main>
    
</body>
</html>
header {
    background-image: url("https://content.codecademy.com/courses/freelance-1/unit-2/pattern.jpeg");
    font-family: Helvetica;
    font-weight: bold;
    font-size: 100px;
    color: khaki;
    text-align: center;
 }
 .brushes h2{
     font-family: Helvetica;
     font-size: 32px;
     font-weight: bold;
     color: white;
     background-color: mediumspringgreen;
 }
 p{
     display: inline-block;
     font-family: Helvetica;
 }
 h3 {
   font-weight: bold;
   color: black;
   text-align: left;
   font-family: Helvetica; 
 }
 .blue {
     font-family: Helvetica;
     font-weight: bold;
     color: blue;
 }
 .Frames h2 {
     font-family: Helvetica;
     font-weight: bold;
     font-size: 32px;
     color: white;
     background-color: lightcoral;
 }
 .Paint h2 {
     font-family: Helvetica;
     font-weight: bold;
     font-size: 32px;
     color: white;
     background-color: skyblue;
 }

Hi Ragan,

Awesome work on the project. I just worked on it myself and I’d like to offer my 2 cents if I can. Here goes…

Index.html feedback

  1. I noticed you used

    tags to separate the products/listings which is fine but for better SEO stats and more importantly, for better accessibility, I think it’s good to use semantic tags like section, article and so on so that it’s clear what that particular aspect is for.

  2. You created a

    tag for the main text as well as the text in blue. I would suggest rather than creating two paragraphs, you can use an inline tag like strong or span.

Here is what my index.html looks like

<!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" type="text/css" href="./css/style.css" />
    <title>Dasmoto's Arts & Crafts</title>
  </head>
  <body>
    <header>
      <h1>Dasmoto's Arts & Crafts</h1>
    </header>
    <main>
      <section>
        <h3 class="brushes-title">Brushes</h3>
        <img src="./images/frames.jpeg" alt="Frames" />
        <h5>Hacksaw Brushes</h5>
        <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.<strong>Starting at $3.00 / brush.</strong>
        </p>
      </section>
      <section>
        <h3 class="frames-title">Frames</h3>
        <img src="./images/frames.jpeg" alt="Frames" />
        <h5>Art Frames (assorted)</h5>
        <p>
          Assorted frames made of different material, including MDF, birchwood,
          and PDE. Select frames can be sanded and painted according to your
          needs. <strong>Starting at $2.00 / frame.</strong>
        </p>
      </section>
      <section>
        <h3 class="paint-title">Paint</h3>
        <img src="./images/finnish.jpeg" alt="Paint" />
        <h5>Clear Finnish Paint</h5>
        <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.<strong>Starting at $5.00 / tube.</strong>
        </p>
      </section>
    </main>
  </body>
</html>

CSS feedback

  1. Based on the specification document, the only font specified is Helvetica. I see you included it in each rule you created. It’s best not to go with that approach because of the concept DRY (Don’t repeat yourself), which I believe is a great principle to keep at the back of your mind when working on projects. Please feel free to Google and read more about it. So to combat this, you could do something like:
body {
    font-family: 'Helvetica';
}

So this way the whole project uses that font and you’ve only had to write it once :wink:.

  1. Following the DRY concept again, for each product, the only thing that’s different is the background colour for the heading. You repeated the rules for each div. A more efficient approach, using your classes, would be:
.brushes, 
.Frames, 
.Paint {
   font-family: 'Helvetica';
   font-weight: bold;
   font-size: 32px;
   color: white;
}

.brushes {
  background-color: mediumspringgreen;
}

.Frames {
  background-color: lightcoral;
}

.Paint {
  background-color: skyblue;
}
  1. For the header section where your h1 tag is, if you use the dev tools, you’ll notice it’s using the user agent stylesheet which is the default stylesheet used by the browser. That’s why you’ll notice your font-size has no effect because it’s using the default h1 tag rules. To override that, just create a h1 selector and include the font-size property and you’re good to go!

Here is what my CSS file looks like:

* {
  margin: 0;
  padding: 0;
}

body {
  font-family: 'Helvetica';
}

main {
  padding: 0 10px;
}

header {
  background-image: url('../images/pattern.jpeg');
}

h1 {
  font-size: 100px;
  font-weight: bold;
  color: khaki;
  text-align: center;
}

section {
  margin: 50px 0 0 0;
}

.brushes-title,
.frames-title,
.paint-title {
  font-size: 32px;
  color: white;
  margin: 0 0 20px 0;
}

.brushes-title {
  background-color: mediumspringgreen;
}

.frames-title {
  background-color: lightcoral;
}

.paint-title {
  background-color: skyblue;
}

img {
  width: 200px;
  height: 200px;
}

strong {
  color: blue;
  font-weight: bold;
}

h5 {
  margin: 20px 0;
}

I hope this helps and please let me know if anything is unclear :blush::pray:t5:

Inappropriate alternate text. Value should be left empty if there is nothing to describe, such as with background images and window dressing.

alt=""

The attribute is required, so don’t remove it, just the value.