huddle landing page with single introductory section master

Community feedback

• 

  Jonathan•295

  @Nathan-FrontPosted about 3 years ago

  Hello Hassan! Congrats on your submission. As a fellow beginner in HTML and CSS couple of things I would like to share:

  1. Rather than using a divs inside the <ul> you should use <li> instead, try this link for more reference about list in HTML >> https://www.w3schools.com/html/html_lists.asp
  2. Your page is missing an h1. For accessibility reasons, pages require one h1 element. In your code you used h2 even though no h1 can be seen, h1 should not be skipped. I recommend you insert h1 then try to hide it using css to fix the accessibility issue. Try this site to learn more about accessibility in html >>>>https://webaim.org/techniques/css/invisiblecontent/
  3. You don't need vertical scrollbar because site should be landing page
  4. Try fixing the background-image in mobile view, in the challenge it should be on top of the screen during mobile view. Try experimenting on background size using cover and contain during desktop view and mobile view to see the difference.

  Marked as helpful

  2

  ---

  

  Hassan Elabadi•150

  @Elab4d1Posted about 3 years ago

  @Nathan-Front thank you for your feedback, I am new to html accessibility so I should learn more about it. Thank you for your advices.

  0

  

  Hassan Elabadi•150

  @Elab4d1Posted about 3 years ago

  @Nathan-Front Hi again, I tried to fix the issues i had, can you give me your feedback please ?

  0
• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey, great job on this one. Layout looks good both desktop and mobile layout.

  Just some other suggestions besides Jonathan's helpful feedback:

  • The alt for the img for the website logo should have been alt="huddle", since the image already has the text needed, better using it.
  • If you remove the outline for interactive element, make sure to add another visual indicator on the :focus-visible state of the element, if you don't want, just remember to not remove the outline property.
  • Those social media should have been inside an anchor tag a since those are links to the social media itself. Also, for every a tag , they should have an aria-label on it, where the value if the name of the social media. For example, the a tag that will wrap the facebook icon should have:

  <a href="facebook.com" aria-label="facebook" />
  

  This way, users will know where this link would take them.

  Aside from those, really great job on this one.

  Marked as helpful

  1

  ---

  

  Hassan Elabadi•150

  @Elab4d1Posted about 3 years ago

  @pikamart thank you for your feedback. These are really good advices, thank you very much. I'll try to improve.

  0

  

  Hassan Elabadi•150

  @Elab4d1Posted about 3 years ago

  @pikamart Hi again, I tried to fix the issues i had, can you give me your feedback please ?

  0

  

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  @Elab4d1 Hey, just saw the site again and it looks good as before and the above suggestions were used, great!

  Also since you had made the changes, you could "generate report" again so that your issues will be fix and be updated if there are still issues on your site.

  Marked as helpful

  1