Responsive clipboard landing page

Community feedback

• 

  Vanza Setia•27,775

  @vanzasetiaPosted over 2 years ago

  Hi again, Luis! 👋

  It's great to see you completing another challenge! Your solution is responsive and looks great! 👍

  You've already gotten some great feedback from @FluffyKas, here is from me.

  • The alternative text for the logo should be the company name. Currently, a screen reader user or a search engine knows the site/project as the "logo" website instead of Clipboard website.
  • Alternative text for images should not contain any words that related to image (e.g. picture, photo, logo, icon, graphic, avatar, etc). It's already an image element so the screen reader will pronounce it as an image.
  • For the download links, I recommend adding download attribute to each download link.
  • All the device images and icons are decorative so leave the alt empty.
  • Just like an image where you don't need to specify or tell the users that it is an image, the same thing applies to a link. The aria-label only needs to tell the site name (Facebook or Twitter).
  • Just like normal users would know that those social media icons are link elements so the screen reader users would know that because the screen readers will announce them as links (in their own way). Currently, with Narrator it pronounces the social media link as the following, "link - go to the creator´s facebook".
  • Changing the html or root font size can cause huge accessibility implications for those users with different font size or zoom requirements.
  • In addition to the previous one, by default the font size is 16px so you don't need to specify it.

  I hope you find this feedback useful! Keep up the fantastic work! 👍

  Marked as helpful

  3

  ---

  

  Luis paz•390

  @PazispeacePosted over 2 years ago

  @vanzasetia thank you to you too for the tips. I forgot you told me to not change the root font size, my bad here! Thank you for pointing it out again

  Also I didn´t know there was a download attribute, thanks!

  1

  

  Luis paz•390

  @PazispeacePosted over 2 years ago

  @vanzasetia did the changes, thanks!!!

  1

  

  Vanza Setia•27,775

  @vanzasetiaPosted over 2 years ago

  @Pazispeace You're welcome! 👍

  It's great that you've made some improvements to your solution, however, I still notice the download links have no download attribute, and the aria-label inside the footer hasn't been fixed.

  Also, as I said earlier, by default the browsers' font size is 16px so you don't need to specify it and don't specify the font-size on the html element. Specify it on the body element with rem unit instead or in this case you don't need to specify it. 🙂

  Marked as helpful

  2

  

  Luis paz•390

  @PazispeacePosted over 2 years ago

  @vanzasetia Thank you!!, I forgot to make those changes even though you told me, my bad again. And thank your for your patience :-)

  I did the changes, thank you once again!

  0

  

  Vanza Setia•27,775

  @vanzasetiaPosted over 2 years ago

  @Pazispeace No problem! 👍

  I took a look at your repository and I just want to say, good job with the updates! 👏

  Keep up the great work!

  0
• 

  P

  Fluffy Kas•7,715

  @FluffyKasPosted over 2 years ago

  Heyo,

  Your solution looks great I think, on all screen sizes! You got most things covered pretty well, I only have a few small suggestions left to make:

  1. Websites should have an <h1> for their main title (the very first heading should be the one here, in the home section).

  2. I recommend reading this great article about alt texts and updating your solution accordingly.

  3. I don't think you should use the <article> tag here, that's used for something else. A simple div is perfectly fine for styling!

  4. Your social links and footer links could be wrapped in <ul> instead of a div.

  Marked as helpful

  3

  ---

  

  Luis paz•390

  @PazispeacePosted over 2 years ago

  @FluffyKas @vanzasetia Thank you for the feedback, I really appreciate it, I will take those useful tips into account!

  1

  

  Luis paz•390

  @PazispeacePosted over 2 years ago

  @FluffyKas I read the article you recommended. Really interesting and useful, thank you so much!!!

  One question, is it okay if I use div instead of the articles that are direct children of the tag section then?

  I also read the article you recommended of MDN

  1

  

  Luis paz•390

  @PazispeacePosted over 2 years ago

  @FluffyKas Did the changes, thanks!

  in this case, where the companies logo are nested... could I change that section tag with the class "companies" for a div tag?

  or what could I do in order to don´t see errors in the report if I dont want to add a heading to that section

  is it a bad practice to leave the section without headings, right?

  Thank you in advanced!

  1

  

  P

  Fluffy Kas•7,715

  @FluffyKasPosted over 2 years ago

  @Pazispeace @Pazispeace Well... read this article, it's about section elements. I'm fairly sure, @vanzasetia would've recommended the same >.<

  To sum it up though: it matters very little whether you use div or section in 99% of the cases. That being said, I like to use sections sometimes purely because I feel like it defines "regions" in my code and that makes it easier for me to find things (so I do the same as you did in your code!). It's perfectly fine, as long as you're aware section carries very little semantic meaning.

  About the issue in the report: I always try to get rid of these issues but to this day, there's the exact same issue in my report for this challenge. I just left it because I didn't think it's super important. Like... solutions will be always a bit unique and perhaps you can't apply strict rules to every single use case.

  But! If you'd like to get rid of it, what you could do is to add a heading with a screen reader only class that would visually hide the element but could still be picked up by screen readers:

  .sr-only {
    position: absolute;
    width: 1px;
    height: 1px;
    padding: 0;
    margin: -1px;
    overflow: hidden;
    clip: rect(0,0,0,0);
    border: 0;
  }
  

  Perhaps this would be the best solution, considering those brand logos may not make a lot of sense when their alt text is announced by screen readers out of the blue >.<

  I may even go back and add it to my solution now.

  Marked as helpful

  3

  

  P

  Fluffy Kas•7,715

  @FluffyKasPosted over 2 years ago

  @Pazispeace And yes, div is fine instead of article, I'd think! ^^

  Marked as helpful

  3

  

  Vanza Setia•27,775

  @vanzasetiaPosted over 2 years ago

  @Pazispeace As a general rule of thumb, you only need to use section if you want to highlight it as a "section" by using aria-labelledy (otherwise it would just be like a div). The most important thing is the heading order than using the article or section tag.

  I agree with @FluffyKas about section and I also highly suggest reading the article, and to learn more about div you can read this article by the same author (Scott O'Hara).

  In general, it's okay to use div as long as it doesn't cause any accessibility issues. 🙂

  Marked as helpful

  3

  

  Luis paz•390

  @PazispeacePosted over 2 years ago

  @FluffyKas Thank you so much for providing those articles!! I´m going to read them

  and for providing the sr only solution!, I wouldn't have thought about it! Thankss

  I learned a lot wit your tips!

  the same goes to @vanzasetia

  thank you guys for taking the time to help! highly appreciate it

  0

  

  Luis paz•390

  @PazispeacePosted over 2 years ago

  @vanzasetia Thank you!, and yes, I´m going to read the article you both recommended to me!

  Have a nice day!

  0