Feedback on my tribute page fcc

hi devs, finally I’ve finished my tribute page project, i will be really happy if you just look at it and give me feedback there’s the link

2 Likes

Overall this is very nicely done. Just a few minor things to point out:

  • It is considered best practice to only jump down by one heading level at a time. So your <h3>'s should be <h2>'s.
  • The last heading you have there isn’t really a heading, it is just a regular old <p>. You can wrap it in a <footer> if you want to make it stand out semantically, but you need to put the <footer> outside of the <main>.
  • I don’t think you need to wrap the timeline in a <section>, especially since you aren’t using it for CSS. Granted, it isn’t really doing any harm, just adding a slight bit of clutter to the DOM.
  • It’s great that you have alt text on the image but it really isn’t doing much. Just repeating the name of the person in the image doesn’t really provide anything we don’t already know since this page is about that person in the first place. You could try to describe the image a little more, such as what she looks like (skin complexion, hair style, black and white photo, etc…). If you don’t feel that those details are important then I would probably leave the alt text blank (alt="").
  • The figure caption is great but I would include her name, or at least her last name, at the beginning of the caption.
  • The wikipedia link text could be better. You want to try to have the link text describe where the link will take you. Yes, it will take you to wikipedia, but there are a lot of pages on wikipedia :slight_smile: Granted, it’s probably safe to assume that this wikipedia link will take you to the page of the person being discussed here. But you might try to reword that sentence so that the link text can include both her name and wikipedia.
  • I’d be tempted to put a max width on the figure.
  • Speaking of max widths, I would use rem units instead of px, for example on the timeline list. This allows the width to take changes in font size into account. In general, I try to use rem units for any type of widths as much as possible.

Again, great job.

2 Likes

Everything that @bbsmooth said… :point_up_2: Also I’m a fan! Good work :slight_smile:

1 Like

Your page looks good @SinaNikzad-1999. In addition to what has been said;

  • I don’t know if you noticed but your h3 element is not being styled as you want because in HTML you have this;
    <h3 id="hedline">
    but your declaration block in CSS is #headline
  • Don’t copy code from the sample. For example, the font-family really only needs a font and a default font. What was copied from the sample contains a font that you don’t link or import.
  • It’s a bit of a nit but look into semantics and see when you would use the b tag versus the strong tag.
  • I’m going to reword one of the comments above. Accessibility is about being accessible to all users. Review the giving meaningful text to links lesson. For a more thorough explanation read Web Accessibility in Mind.
    • wikipedia” is not accessible
1 Like