Multi-page website using HTML, SCSS & Vanilla JS

Community feedback

• 

  Vanza Setia•27,775

  @vanzasetiaPosted about 2 years ago

  Hi, Jeuri! 👋

  Congratulations on finishing this challenge! 🎉 Nice heading structure on the home page! 👍

  Regarding the images, which image that you are talking about? I don't think there are images that change sizes.

  Some suggestions from me.

  • I recommend creating the "line" above some text with a pseudo-element.
  • There is already alternative text for the logo. There's no need to enhance that with aria-label.
  • Alternative text for the logo should not contain the word "logo". The semantic meaning of the img element is good enough to tell users that it is an image.
  • Use interactive element for something that has interactivity. In this case, the hamburger menu button should be a button element. Then, make the button listens to the click event (not the icon).
  • There should be only one HTML markup for the mobile and desktop navigation. I recommend reading my README (#accessible-menu-component section). I wrote about the HTML markup and some ARIA attributes to improve the experience for the users of assistive technology.
  • For images that are used only as decorations (they do not give users any information and serve only an aesthetic purpose), the alt tag should still exist but should be left empty: alt="". This will tell the screen reader to simply skip over the image.
  • I recommend taking a look at "Quick tip: Using alt text properly - The A11Y Project".
  • For the social media links, if the social media icons have alternative text, then it's already becoming the label for the link. So, remove the aria-label from all social media icons.
  • On the about page, I recommend trying to look at the page without styling (just HTML). There are two h3 with the same text content. I would think that the second name of the director as a paragraph instead.
  • Use button element for the card__btn.
  • In your CSS, I noticed this selector .about .directors .container .grid .card__container.flipped .card__btn img which would much be as .card__btn img . I recommend only nesting when you want to style pseudo-elements, pseudo-classes (:hover, :focus, etc), and @media queries. It's a good practice to keep the CSS specificity as low and flat as possible. High specificity will make your stylesheet hard to maintain.
  • I recommend reading "5 Handy Sass Code Snippets" article. In that article, you will find some useful @mixin to help you write Sass.
  • Input element must have an accessible name. You can use aria-label to label each input.
  • I recommend reading "How To Create Accessible Form with Boring Design?", which will give you a guide to creating accessible form validation with aria-live and aria-describedby (and much more!)

  I have three recommended videos. The first one tells how hard HTML is (HTML is not easy). The other two talk about modern CSS techniques and approaches.

  • Manuel Matuzović - Lost in Translation - YouTube
  • Andy Bell – Be the browser’s mentor, not its micromanager - YouTube
  • Stephanie Eeckles - Scaling CSS Layout Beyond Pixels - YouTube

  I hope this helps! Happy coding!

  Marked as helpful

  1

  ---

  

  Jeuri Morel•1,385

  @JeuriMorelPosted about 2 years ago

  @vanzasetia Thank you so much for the feedback, it helps a ton. I've made some progress since I submitted this solution, but your comments are helpful nonetheless. I've bookmarked everything you've linked to and will hopefully get through them sometime in the next week. Thank you so much for taking the time to review my code and comment.

  The images I was referring to were the company names in the about page.

  Thanks again and have a great day!

  0

  

  Vanza Setia•27,775

  @vanzasetiaPosted about 2 years ago

  @JeuriMorel

  You are welcome! 👍

  Okay! I made the parent of those company logos a flex container. Then, I used max-width and flex properties to control the width of those images. I think flexbox is a good solution for this problem.

  Have a nice day too! 🙂

  Marked as helpful

  1

  

  Jeuri Morel•1,385

  @JeuriMorelPosted about 2 years ago

  @vanzasetia That makes sense! Flexbox is a great tool for these kinds of issues.

  1