html5, css3, sass, javaScript, git, github, vscode, slack

Hey, great work on this one. As Shanin said, on desktop layout, the hero-section seems too squeezed. In mobile layout, some sections were also squeezed and some elements needs to be resized.

Addition to what Shanin said:

• I wouldn't include the header bookmark logo in the nav since you are not treating the website logo as a link, better to separate it from the nav.
• The alt text of the header logo should have been alt='bookmark". The logo itself have the name as the alt value, also when using alt attribute, do not include any word that relates to "graphic" like "logo, icon, image". Assistive tech will handle those for you.
• The list of links in the header is the one that nav needs to nest.
• On the header on the ul element, do not put any other html element inside it, unless it is nested inside li:

<ul>
  <li></li>
  # do not use any other html element unless it is <li />
</ul>

• The login should have used a tag since it is being treated as a link in the navlinks.
• You are missing an h1 element. Every webpage should at least contain 1 h1 element. The heading tag in your hero-section could have been the h1.
• It is good that you are trying to implement tabbed interface in the features section, however right now, it is not usable. You forgot to add the javascript that moves the focus from the selected tab to another one using arrow-keys. You can see my solution and inspect it and go to the sources, you will see how I implemented my tabbed interface.
• The aria-selected attribute is not applicable to div alone. You should use it inside button element that are selected by the user in the tabbed interface.
• It is good that you added tabindex="-1" on the selected feature container, however, maybe customized the outline :>.
• "more info" should have been a a tag since it is being treated as a link to view "more info".
• alt value of the browser img should only contain their name and lose the word "logo".
• "more info" in faq section should have used a tag as well.
• "Stay up-to-date with what we're doing" text in the cta section should have been a heading tag, since it is a topic title for the email subscription.
• On the input in the cta section, you might want to move the stylings from the :focus to :focus-visible .
• The input type="submit" could have used a button type="submit". The latter one is not recommended now, I think. Also, having a cursor: pointer on the desktop state as well would be really good.
• Now, as you can see, the error appears visually when a user submitted a wrong email, to provide more accessibility especially for people who only uses screen readers. It would be great to add aria-invalid and aria-describedBy on the input element if the user submit it wrong. Let me create a pseudocode:

const email = query the email input;
const error = query the email error text;

if ( email is invalid) {
  email.setAttribute("aria-invalid", "true"); 
  email.setAttribute("aria-describedBy", "id of the error message element"); # binds
  error.make it appear
} else {
  email.removeAttribute("aria-invalid");
  email.removeAttribute("aria-describedBy");
  error.hide it
}

Using aria-invalid and aria-describedBy, this will create a guidance for screen reader users, knowing that they submitted a wrong email input. It would be great to make an element have a aria-live attribute to make it even more accessible.

MOBILE

• The hamburger menu button should have been the one triggering the dropdown event. Right now, I can't use my keyboard to toggle it, that is why put the event on the button.
• The button as well for the hamburger dropdown should use aria-expanded attribute, to inform the user that the dropdown has appeared.
• The button as well should have the aria-label and the alt text for the img inside the button remove all of those, you only need 1 and it must be on the button element.
• On the dropdown, the social media links should be wrapped inside a a tag. They are links right, better to use a tag with aria-label on them.
• Lastly, you might want to check the layout in mobile flow. There are elements that are not really good to look, since the widths, they overflow, and other. Check those one out.

Still, you did a great job on this one.

Hi, Awesome job! Everything looks good, however please look into re-styling the .hero class; on the desktop it looks too squeezed.

I tried to add: justify-content: space-evenly; to the .hero class, it seems to help; but a little re-work would be required.

Also, I'd embed the .hero image as using the <img> tag instead of using the ::before pseudo-element. And then add the following;

 img {
   max-width: 100%;
}

for more control over my image.

Hope the above helps!

Keep on coding and best of luck on your coding journey!