Code Review: Dasmoto's Arts & Crafts

Hi all, I just finished the Dasmoto’s Arts & Crafts project and am seeking a code review, I am 7% of the way through the course.

I’m still trying to figure out how to best use section and article tags for a normal webpage structure. Any advice on the implementation for these tags is greatly appreciated as well as any other advice.

Lastly and most importantly I am happy to return the favor and perform code reviews for anyone. Thanks!

<!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>Dasmoto's Arts & Crafts</title>
  <link rel="stylesheet" href="./resources/css/styles.css">
</head>
<body>
  <header>
    <h1 id="main-banner">Dasmoto's Arts & Crafts</h1>
  </header>
  <main>
    <article id="art-supplies">
      <section id="brushes">
        <h2>Brushes</h2>
        <img src="./resources/images/paintbrushes.webp" alt="hacksaw brushes">
        <h3>Hacksaw Brushes</h3>
        <p>Made of the highest quality oak, Hacksaw brushses are known for their weight and ability to hold paint in large amounts. Available in different sizes. <span class="pricing">Starting at $3.00 / brush</span>.</p>
      </section>
      <section id="frames">
        <h2>Frames</h2>
        <img src="./resources/images/frames.webp" alt="frames">
        <h3>Art Frames (assorted)</h3>
        <p>Assorted frames made of different material, including MDF, birchwood, and PDE. Select frames can be sanded painted according to your needs. <span class="pricing">Starting at $2.00 / frame</span>.</p>
      </section>
      <section id="paint">
        <h2>Paint</h2>
        <img src="./resources/images/paint.jpeg" alt="paint supplies">
        <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 class="pricing">Starting at $5.00 / tube</span>.</p>
      </section>
    </article>
  </main>
</body>
</html>
#main-banner, #art-supplies, .pricing {
  font-family: Helvetica, Arial;
}

/* Banner setup */

#main-banner {
  font-size: 100px;
  font-weight: bold;
  color: khaki;
  text-align: center;
  background-image: url('https://content.codecademy.com/courses/freelance-1/unit-2/pattern.jpeg');
}

/* Section header setup */

#art-supplies h2 {
  font-size: 32px;
  font-weight: bold;
  color: white;
}

#brushes h2 {
  background-color: mediumspringgreen;
}

#frames h2 {
  background-color: lightcoral;
}

#paint h2 {
  background-color: skyblue;
}

/* Pricing text styling */ 

.pricing {
  font-weight: bold;
  color: blue;
}```

Greetings coder,

This is my first time doing a code review. I wish I could give you the credible feedback that your code deserves, but I will say what I came to say and hope it helps you somehow.

I think that your html code has a very readable format, and you seem to make clever use of the section tags. I think your CSS code all used very specific selectors that makes it very easy to see what the code is doing. I copied your CSS code into VS studio and noticed that there was a small error on the last line caused by the ``` symbols. But, your code was still able to run well, and the page loaded quickly. Overall I think you have done a nice job here. Best wishes.
-Austin

Thanks Austin,

I really appreciate the code review. Thanks for pointing out the error on the last line (non sure how I did that :grinning:).

Let me know if you are needing any code reviews as well. I’m glad to help!

1 Like

Nice work!

Main thing I would add is maybe comments and spacing for the html.

I would also take out <article> because it’s doing the same thing as <main>.

E.g

<body>
  <header>
  </header>
  
  <main>

    <!-- Section 1 description -->  
    <section>
      Section 1 stuff
    </section>

    <!-- Section 2 description -->
    <section>
      Section 2 stuff
    </section>

  </main>
</body>

All just personal preference though :slight_smile:

Thanks so much for the feedback!

I really like the spacing idea, it really helps to distinguish the different sections.

Let me know if you ever need a code review on one of your projects!

I’m relatively new to coding and have never done a code review, either. Why did you choose to use “id” so many times instead of “class”?

Hi sloanrobe,

I like to use id tags instead of class as they are unique to that specific section only. In other words, there should only be one “brushes” section in all of the code. Classes are typically used to identify something we would want to reuse for multiple tags. For example, maybe I would want to create a card that holds the img and h3 tags for each section. You could use a class to make the formatting for each card by using div class=“card” or something to that effect and then style with CSS. Id’s are also particularly useful for internal linking with anchor tags as well!

I’d be interested what approach you typically take on these. I’m always trying to learn new approaches that might help streamline my code.

Thanks for stopping by and let me know if you ever need me to do a code review. I’d be happy to. Thanks!

Good job on the code, I would recommend putting some ‘white space’ in the code for example
selector {

}

selector {

}

so the code is not all crammed together. Also maybe going forward add some comments, it will help a lot moving forward when you come back later after a few weeks etc. You will be able to read the comments and determine what your code is doing. Other than that I think you did well.

Thanks so much, I appreciate the feedback. Great suggestions on the spacing and comments. Since my original post I have made an effort to add this to my code.

Thanks again!

1 Like

Anyone willing to do a code review for me?

Here is my code

HTML

<!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="style.css" />
    <title>Dasmoto's Arts and Crafts</title>
  </head>
  <body>
    <nav id="nav">
      <h1>Dasmoto's Arts and Crafts</h1>
    </nav>
    <main>
      <!-- Brushes -->
      <div id="brushes">
        <h2>Brushes</h2>
      </div>
      <div id="img-brushes">
        <img src="./img/brushes.jpeg" alt="Hacksaw brushes" />
      </div>
      <div id="brushes-content">
        <p>Hacksaw Brushes</p>
      </div>
      <div>
        <p id="brushes-detail">
          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 id="brushes-price"> Starting at $3.00 / brush.</span>
        </p>
      </div>
      <!-- Frames -->
      <div id="frames">
        <h2>Frames</h2>
      </div>
      <div id="img-frames">
        <img src="./img/frames.jpeg" alt="picture frames" />
      </div>
      <div id="frames-content">
        <p>Art Frames (assorted)</p>
      </div>
      <div>
        <p id="frames-detail">
          Assorted frames made of different material, including MDF, birchwood,
          and PDE. Select frames can be sanded and painted according to your
          needs. <span id="frames-price"> Starting at $2.00 / frame.</span>
        </p>
      </div>
      <!-- Paint -->
      <div id="paint">
        <h2>Paint</h2>
      </div>
      <div id="img-paint">
        <img src="./img/paint.jpeg" alt="Finnish Paint" />
      </div>
      <div id="paint-content">
        <p>Clean Finnish Paint</p>
      </div>
        <p id="paint-detail">
          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 id="paint-price"> Starting at $5.00 / tube. </span>
        </p>
      </div>
    </main>
  </body>
</html>

CSS

/* styling for the entire page */
* {
  margin: 0;
  padding: 0;
  font-family: Helvetica;
  font-weight: bold;
}

/* nav bar styling */
#nav {
  background-image: url("https://s3.amazonaws.com/codecademy-content/courses/freelance-1/unit-2/pattern.jpeg");
  text-align: center;
  font-size: 100px;
  color: khaki;
  margin: 10px 8px 55px 8px;
  padding-top: 15px;
}

/*Category title*/
#brushes,
#frames,
#paint {
  font-size: 32px;
}

/*category tags*/
#brushes {
  background-color: mediumspringgreen;
  color: white;
  margin: 0 5px 2% 5px;
}

#frames {
  background-color: lightcoral;
  color: white;
  margin: 0 5px 2% 5px;
}

#paint {
  background-color: skyblue;
  color: white;
  margin: 0 5px 2% 5px;
}

/*product details*/
#brushes-detail,
#frames-detail,
#paint-detail {
  font-weight: normal;
  margin: 0 5px 2% 5px;
}

/**/
#brushes-content,
#frames-content,
#paint-content {
  font-weight: normal;
  padding: 15px 5px;
  /* padding-bottom: 30px; */
}

/* #brushes-content,
#frames-content,
#paint-content {
  
} */

#brushes-price,
#frames-price,
#paint-price {
  color: blue;
}

#img-brushes,
#img-frames,
#img-paint {
  margin: 0 5px 0 5px;
  position: relative;
}

Thank you

Hi mverma82,

Glad to provide some feedback. Keep in mind I’m new to all this as well :grinning:.

  • One thing I would recommend is using section tag instead of all the div’s for the various sections (paint, frames, etc.). This helps to provide a little more semantic meaning to the content of the page (as compared to the div element which has no semantic meaning).

  • It also looks like you have a lot of nested div’s within each of the sections which only have 1 element being contained. I think a simpler approach might be to use classes directly on the elements themselves to target them for styling purposes. For example, instead of having id’s for brush-detail, paint-detail, etc. you could create 1 class called detail and apply it directly to the elements you want to style in each section. This would help to refactor a lot of the HTML and CSS to increase the simplicity.

Otherwise, I think your code looks great! Your comments within the code really help to make this readable for other code reviewers. Great job and let me know if you ever need a code review on anything else!

2 Likes

thanks, I will refactor my code as you suggested.

Anytime, glad to help. Let me know if you ever need any more code reviews. Take care.

1 Like

Sure what’s your name, location and what % are you done? My name is Manny and I live in Salt Lake City UT I am 13% done with the Front End Engineer Course.

Hey Manny, nice to meet you. I’m Matt, I live in Missouri and I’m 27% of the way done with the Front End Engineer course. Happy to look over some code anytime!

1 Like

Hello Matt

likewise.