Feedback for a tribute page is welcome. Bill from Kill Bill tribute

Code Feedback
1
2

@missKatiaPunter This looks nice! I like how the design is similar to the movie art. Here are some suggestions for improvements:

  • The alt attributes for your images are not descriptive of the images. Screen readers will describe your images as “First Image”, “Second Image”, and “Third Image” which doesn’t tell the people using the screen readers what the images are. WebAIM has a very thorough explanation of alt attributes and how to use them.

    The alt attribute should typically:

    • Be accurate and equivalent in presenting the same content and function of the image.

    • Be succinct. This means the correct content (if there is content) and function (if there is a function) of the image should be presented as succinctly as is appropriate. Typically no more than a few words are necessary, though rarely a short sentence or two may be appropriate.

    • NOT be redundant or provide the same information as text within the context of the image.

    • NOT use the phrases “image of …” or “graphic of …” to describe the image. It usually apparent to the user that it is an image. And if the image is conveying content, it is typically not necessary that the user know that it is an image that is conveying the content, as opposed to text. If the fact that an image is a photograph or illustration, etc. is important content, it may be useful to include this in alternative text.

  • You’re using inline styling like style="background-color:rgb(229, 229, 229);". Even though this works and gives you the style you want, it’s not recommended because it blurs the line between content and design. It’s generally better to give elements their own class names and style those elements in your style.css file.

  • In the following code, you have some duplication. Did you mean to remove one of the text-decoration rules in each selector:

    a.internal-links:hover{
    color:inherit;
    text-decoration: line-through;
    text-decoration: white line-through;
}
a.internal-links:active{
    color:inherit;
    text-decoration: line-through;
    text-decoration: white line-through;
}

    I found this by clicking on the down arrow in the CSS section of CodePen and selecting “Analyze CSS”. You may also want to paste your CSS code into this CSS validator to see the other errors and warnings.

  • I noticed that on line 8, it looks like you forgot to close your <h1> element. Closing it seems to change some of the spacing around it, so I’m not sure it’s what you want, but the element needs to have a closing tag.

  • Your headings need some work as I see an <h3> before the <h1>:
    https://developer.mozilla.org/en-US/docs/Web/HTML/Element/Heading_Elements

    • Heading information may be used by user agents, for example, to construct a table of contents for a document automatically.

    • Do not use lower levels to decrease heading font size: use the CSS font-size property instead.

    • Avoid skipping heading levels: always start from <h1>, next use <h2> and so on.

    • You should consider avoiding using <h1> more than once on a page. See “Defining sections” in Using HTML sections and outlines for more information.

Those are the big things that I notice. Your CSS and JavaScript shows that you have a pretty good grasp of what you’re doing as there is some relatively complex code in there. Great work! I’m looking forward to seeing your future projects and learning what I can from them. Thanks for sharing! :sunny:

1 Like
3

This was proper feedback, thank you so much. I have learned a lot. Who are you, camper?

1 Like
4

@missKatiaPunter I’m glad it’s helpful. I’m someone trying to learn web development and I notice that when I get a review like this, it really helps. So, I want to give reviews like this in order to help others. While giving reviews like this, I learn something new each time, so giving reviews turns out to help me as well. :slightly_smiling_face:

1 Like
5

I cannot agree more.