Finally finished my landing page. Any feedback is appreciated :)

Code Feedback
1

Core Rebounding Landing Page

2 Likes
2

Wow, I’m a really picky reviewer, and I’m not finding much to complain about.

I would say that I would want some more use of semantic HTML.

And things like this:

  .footer-nav li {
    padding: 20px 20px 20px 20px;
  }

could be simplified to:

  .footer-nav li {
    padding: 20px;
  }

Have fun with the technical documentation page.

3

Thank you so much Kevin!

4

I’m going to agree with @kevinSmith this is really nice. I’m super picky too, with a focus on accessibility, so I’ll suggest a few things pertaining to that.

  • While not technically required, the overwhelming majority of accessibility experts will tell you to always have an h1 heading near the top of the page, and then don’t skip heading levels as you work your way down. Usually the h1 is at the beginning of main but it could be in the header. If you don’t want it to be seen on the page you can visually hide it so that it is still available to screen readers. And then instead of jumping from h2 → h4 turn those h4s into h3s.

  • I would change “Click here to start your order” to just “Start your order”. You’ve left the underline on the link (which is good) and so everyone knows that it is a link and should be clicked on. This also makes it function more as a heading (which should be changed to an h2 by the way). I’m not 100% sold that this should actually be a heading but I’m not going to fail you for it :slight_smile:

  • Also, you’ve set the display to block on the start your order link so the click area stretches all the way across the page, just asking for someone to accidentally click on that white space :slight_smile: Undo that. If you want to center the link there are other ways to do that (you could do text-align: center on its parent for example).

  • I make this next suggestion to everyone :slight_smile: You are relying on the browser’s default keyboard focus outline. Technically that passes accessibility guidelines right now but that is going to change in the future. A lot of browsers do not have good defaults for this (I am viewing your page with Firefox and I can barely see it). I would recommend you customize the CSS outline property for links when they have focus (a:focus) so that the outline is much more prominent.

  • Don’t include words like “logo”, “image”, etc… in the alt descriptions for images. These are redundant and add unnecessary verbage for screen readers. “Core Rebounding” is enough for the logo.

Again, very nice job.

1 Like
5

I really appreciate you taking the time to give me this feedback. I definitely need to think about the accessibility aspect of my projects more. Thank you!

6

congratulations .
there was something that caught my eye and it was the @media. you used min-width: 630px two times. you can put them all together like:

@media (min-width: 630px) {
  #header-img {
    position: fixed;
    left: 35px;
    top: 15px;
  }
  .nav-list {
    display: flex;
    flex-direction: row;
    justify-content: flex-end;
    padding: 30px 0px;
    list-style-type: none;
  }
  .nav-items {
    padding: 0px 10px;
  }
  #header {
    height: 180px;
  }
}

and the max-width too

7

Good tip. Thank you :slight_smile:

8

This topic was automatically closed 182 days after the last reply. New replies are no longer allowed.