Project Feedback

[derpachu]

@chrisvfritz Can you take a look at this? It's hosted here and meets the following criteria:

• [x] Place a nav element at the top of the page that includes (fake) links to other pages
• [x] Use the main element appropriately
• [x] Include three products your store sells, all listed on the same Products page
• [x] For each product, correctly use an article, header, and footer element and one or more section elements
• [x] Use figure and figcaption to show what each product looks like
• [x] Each product includes a nav element with one or more links to related products (the links don't have to go anywhere)
• [x] Use an aside in one or more products to advertise a promotion or discount for that product

not sure why I completely missed this project

[chrisvfritz]

Looking mostly good! The semantic elements are almost always used correctly here, 🎈 with just a couple exceptions and a few other areas for improvement:

Inconsistent indentation

90% of your indentation is consistently using 2 spaces 👍 , but there are a few spots with 4-space indentation, so let's fix that. Let me know if you're not sure what I mean or you'd like some tips to maintaining consistency for the future.

Invalid attribute values for width and height

The width and height attributes actually don't work like CSS, so these attributes:

• width="auto"
• height="300px"

are actually invalid. Instead, they accept a number of pixels without a unit specified (e.g. height="300") and in cases where you want auto behavior, you'll simply not include the attribute.

That said, I don't recommend actually using width and height - especially in cases like this, where you are trying to apply the same styles to every image. Instead, I'd take this opportunity to add a CSS file to select img elements. That way, whenever you decide you'd like the images to appear a little differently, you only have to update your code in a single place! 😄

Empty content for meta name="description"

Right here, I would either fill in a value for the content attribute or remove this element, since it doesn't do anything currently.

h2-h6 elements should not by used as subheadings

This is a very common mistake, but there's actually an issue with this h4 element:

<h4>by me</h4>

According to the HTML spec:

h2–h6 elements must not be used to markup subheadings, subtitles, alternative titles and taglines unless intended to be the heading for a new section or subsection. Instead use the markup patterns in the §4.13 Common idioms without dedicated elements section of the specification.

Instead, it's typically best to use a p element in these cases, with a custom class that you can use to style it. And generally, any time you find yourself wanting to "skip" a heading (e.g. going from an h1 straight to an h2) or have two heading elements next to each other, that's an indication that you're probably using them incorrectly.

All img elements have the same alt value

All img elements currently have an alt of table image. Since most of them are not tables, let's update these to use more descriptive names.

And as a sidenote, it's not actually necessary to include words like "image" in the alt attribute. Browsers and screen readers will know from context that it's an image, because the attribute is on an img element.

Placement of header and footer elements

The header and footer are outside the article elements they're associated with, while they should be inside it instead. Otherwise, the browser won't know that these are the header and footer for that article.

Stray p elements outside of section elements

I see all the prices (e.g. <p>price:$20.00</p>) have been moved outside the section element, which seems a little strange to me, since it seems very related to the content inside the section. Was there a reason for this or was it just an oversight?

---

Let me know if you have any questions or other items I can help you with. 🙂

[derpachu]

made the changes. Yeah I didn't really get the section usage in this assignment as there wasn't much else to write that needed to be sectioned off per item. I guess just to use it more often. I didn't write a css file just as I really only wanted to format the 3 pictures. If you want I could include one.

[chrisvfritz]

Everything looks great! 🎉 :shipit: 🌟

Yeah I didn't really get the section usage in this assignment as there wasn't much else to write that needed to be sectioned off per item.

Yeah, we've actually just removed the requirement for a section element in the project. They make sense for larger posts, but they have a marginal enough effect on SEO that we don't want students to have to write a lot more content, just so it makes sense to use it.

I didn't write a css file just as I really only wanted to format the 3 pictures. If you want I could include one.

The general rule is, as soon as you want to style anything, it's time for a CSS file. It definitely saves work in the long run! As it is right now, whenever you want to change the height of your images, you now have to remember to do it in 3 separate places. 😔 I'll let it pass on this project, but it's a good practice to get into for the future.