Social Link Using Flex

Community feedback

• 

  Daniel 🛸•44,250

  @danielmrz-devPosted 6 months ago

  Hello @ErrorCode710!

  Your solution looks great!

  📌 It's recommended to use semantic HTML elements like <ul> and <li> for creating lists. This ensures that your code is more accessible, maintainable, and semantically meaningful.

  Here's and example on how you can refactor your code:

  After Refactoring

  <ul class="list-container">
      <li><a href="#">Github</a></li>
      <li><a href="#">Frontend Mentor</a></li>
      <li><a href="#">LinkedIn</a></li>
      ...
  </ul>
  

  By using <ul> and <li>, you convey the structure of your content more clearly, making it easier for screen readers and search engines to understand. Additionally, it aligns with best practices for HTML semantics.

  I hope you find this helpful!

  Keep up the excellent work!

  1

  ---

  

  ErrorCode710•150

  @ErrorCode710Posted 6 months ago

  @danielmrz-dev will apply soon, sir

  1
• 

  Abdul Khaliq 🚀•72,580

  @0xabdulkhaliqPosted 6 months ago

  Hello there 👋. Congratulations on successfully completing the challenge! 🎉

  • I have a suggestion regarding your code that I believe will be of great interest to you.

  ---

  IMPROPER USAGE OF ALT 📸:

  • The alt text "Avatar-icon" is somewhat vague and not descriptive enough.

  ---

  • It fails to adequately convey the content or purpose of the image, which is essential for accessibility and user experience.

  ---

  • A more appropriate alt text could describe the specific content or function of the avatar, such as "Jessica's profile picture for social media" or "Avatar representing Jessica for social link." This would provide clearer information to users who rely on screen readers or encounter image loading issues.

  ---

  • The alt attribute should explain the purpose of the image, in our case the image is a portrait of Jessica. So alt with her name itself is enough.

  ---

  • E.g. alt="Jessica"

  ---

  <img src="/assets/images/avatar-jessica.jpeg" alt="Jessica">
  

  ---

  .

  I hope you find this helpful 😄 Above all, the solution you submitted is great !

  Happy coding!

  1

  ---

  

  ErrorCode710•150

  @ErrorCode710Posted 6 months ago

  @0xabdulkhalid Thanks sir, I will apply soon.

  0
• 

  Bernardo Poggioni•5,280

  @R3ygoskiPosted 6 months ago

  Hello ErrorCode (a very interesting name for a programmer), congratulations on completing another challenge, and your solution is very close to what was proposed by the challenge.

  Firstly, a tip on how you could better center your card: instead of using margin-top on the body, you can use min-height: 100vh;. This will make the body exactly the height of the screen vertically, which, in turn, will center your card on the screen due to the flex properties you've used.

  Another thing I noticed is that in your .card-container you used three padding properties, but you could make it more concise using shorthand like this: padding: 0 2rem 1.5rem 2rem, This way, you only omit the value for the top padding.

  Regarding your <a>tags, they lack the cursor: pointer;styling, which reduces accessibility because it's not clear that they are clickable content.

  Now, moving on to the HTML part, your structure is correct. I would just change two things: firstly, you could use semantic tags to improve accessibility and SEO. Instead of div.card-container and div.social-media, you could replace them with <main> and <nav> respectively. Secondly, in the headings, you skipped <h2>, While this doesn't cause a problem in the structure, it does impact accessibility. It's highly recommended to follow the correct hierarchy of headings.

  Once again, congratulations! Your project looks great. Keep practicing and improving. If anything I said isn't clear, please comment below, and I'll try to help as best as I can.

  1

  ---

  

  T

  Grace•31,090

  @grace-snowPosted 6 months ago

  @R3ygoski social media wouldn't be a nav. Nav landmarks are for navigation within the current site. They could stay in main or go in a footer.

  1

  

  Bernardo Poggioni•5,280

  @R3ygoskiPosted 6 months ago

  Hi @grace-snow,good to know that, I thought <nav> was for navigation in general, so actually the correct usage in this case would be <ul> and <li>, right?

  0

  

  T

  Grace•31,090

  @grace-snowPosted 6 months ago

  @R3ygoski yes they should be in a list as others have said above.

  @ErrorCode710 you need to refactor this html to

  • wrap the main content in a main landmark.
  • remove all the strange headings and different heading levels. This is one heading (the person's name) then 2 paragraphs)
  • update alt text
  • use a list for the list of links
  • ensure every anchor has a href so they appear in the keyboard tab order correctly

  2