Sunnyside Agency Landing Page

Community feedback

• 

  PhoenixDev22•16,950

  @PhoenixDev22Posted about 2 years ago

  Hi Mateusz Dobrogowski,

  Congratulation on completing another frontend mentor challenge. I have some suggestions regarding your solution:

  HTML

  • The alternate text of the logo should not be empty. You may set alt=”Sunnyside". Use the website's name as an alternate text. Remember that a website-logo is one of the most meaningful images on a site so use proper alt for it. The same for the footer’s logo.

  • Use the <nav> landmark to wrap the navigation.

  • Avoid creating duplicate content (duplicate navigation). You can style the same navigation in mobile and desktop differently using media queries. Practice like this can result in a poor user experience, when a visitor finds substantially the same content repeated within a set of search results.

  Toggle element:

  • It’s not recommended to add event listener on non-interactive elements class="hamburger-icon". You can use a <button> with type=”button” around the img.

  • As the toggle element class="hamburger-icon" has no discernible name, put an aria-label on your trigger to describe its purpose. For example, you can have: aria-label='Mobile Navigation Trigger' or 'Open Menu.’

  • Add an aria-expanded attribute to your toggle element letting the user know if the content the button controls is expanded or not. At first, it has the “false” as a value then you use JavaScript to change the value.

  • You should use aria-controls attribute on the toggle element, it should reference the id value of the <ul> element.

  • For the alternate text of the testimonials avatars should not be empty, you can use only the avatar name omitting the word image.

  • look up a bit more about how and when to write alt text on images. Learn the differences with decorative/meaningless images vs important content. For decorative images, you set an empty alt to it with an aria-hidden=”true” to remove that element from the accessibility tree. This can improve the experience for assistive technology users by hiding purely decorative images.

  • For example in: <span class="more more-2">Learn more</span>,never use <div> and <span> alone to wrap a meaningful content. Just keep in mind that you should usually use semantic HTML in place of the div tag unless none of them (the semantic tags) really match the content to group together.

  • Use the<nav > landmark to wrap the footer navigation with aria-label=”secondary “ or aria-label=”footer”. A brief description of the purpose of the navigation, omitting the term "navigation", as the screen reader will read both the role and the contents of the label. Thenav element in the header could use an aria-label="primary" or aria-label=”main” attribute on it. The reason for this is that, You should add the aria-label for a nav element if you are using the nav more than once on the pag..you can read more in MDN

  • Use the interactive element <a>around the images in class="media" , in this instance, they are not clickable , neither navigate the user.

  • Instead of using a generic div to wrap the .media links , you put your links within an unordered list structure so that a screen reader will read out how many things are in the list to give visually impaired users the most information possible about the contents of the navigation.

  • The links must have aria-label orsr-onlytext tells where the link navigates the user. For example: Visit our facebook. For images, you should set aria-hidden=”true” to be ignored by screen readers and to avoid redundancy and repetition.

  Overall, great job on this one. Hopefully this feedback helps.

  Marked as helpful

  0