Profile card component

Community feedback

• 

  T

  Alex•2,010

  @AlexKMarshallPosted about 3 years ago

  That looks great. I particularly like the polished readme file too, makes a great first impression.

  For the solution itself, I think there are too many heading elements. The user's name makes sense to be the heading for the card, but I think the others don't makes sense as headings. They're not labelling something or defining a new section.

  I'd maybe structure it something like:

  <main>
    <h1>My Page</h1>
    <article>
      <h2>Victor Crest <span>26</span></h2>
      <p>London</p>
      <ul>
        <li><span>80k</span> <span>Followers</span></li>
        <li><span>803k</span> <span>Likes</span></li>
        <li><span>1.4k</span> <span>Photos</span></li>
      </ul>
    </article>
  </main>
  

  That way you can probably avoid adding too many wrapper divs to that as well.

  It might be neat to model the bottom section as a definition list, rather than a regular unordered list, but the problem is in the design the term <dt> (Followers) comes below the definition <dd> (80k). And for the html to be valid the dd must come before the dt. You could flip the visual order using flex-order, but then that would violate https://www.w3.org/TR/WCAG20-TECHS/C27.html

  I like the animation on the profile photo, that's cool. Make sure you switch it off in a media-query for users with prefers-reduced-motion. You can see an example of how to do that in https://piccalil.li/blog/a-modern-css-reset/

  Marked as helpful

  1

  ---

  

  Yazdun•1,310

  @YazdunPosted about 3 years ago

  @AlexKMarshall Hello Alex ! I fixed the structure thing that you mentioned, only then I realized how messed up It was 😁 It reduced amount of css code too ! Thank you so much on that.

  secondly I added media-query for prefers-reduced-motion and thanks for the heads up, I'm gonna use this on my other projects too.

  I'd be grateful to know your opinion on the updated solution ❤

  0

  

  T

  Alex•2,010

  @AlexKMarshallPosted about 3 years ago

  @Yazdun That looks a lot cleaner. For the images that are purely decorative patterns like the circles in the background, the alt text should just be an empty string (though the property must still be present)

  Marked as helpful

  0
• 

  Mark Mitchell•1,820

  @markup-mitchellPosted about 3 years ago

  I'm impressed by your commitment to browser fallbacks! I should probably think about that a bit more when there's no tooling to add them automagically...

  To me the heading hierarchy seems odd - if the user's age is h2 and location is h3 that suggests to me that location is a subsection of age, which doesn't make sense to me.

  Similarly, the card__profile-stat nodes seem backwards; you could make those divs into sections and reverse the order of the h and p for a better result (IMO):

  <section class="card__profile-stat">
      <h3>Followers</h3>
      <p>80K</p>
  </section>
  

  Re-ordering visually them with flex-direction won't affect their order in the document:

  .card__profile-stat {
      display: flex;
      flex-direction: column-reverse;
  }
  

  Oh, I'd leave the alt tags on your circles blank too, like you have some other images.

  Marked as helpful

  0

  ---

  

  Yazdun•1,310

  @YazdunPosted about 3 years ago

  @markup-mitchell Hello Mark ! Thank you for your feedback, as you mentioned I realized how messed up my markup was, so I fixed it now. I used ul for stats

  also I added alt to my images, Thanks for reminding me.

  for browser fallbacks, I use this website to generate prefix which is great for writing plain css.

  ✅ Thank you again for your constructive feedback

  0