Profile card component

Community feedback

• 

  T

  Grace•31,270

  @grace-snowPosted over 2 years ago

  Hi

  I'm afraid this one isn't finished. I'd encourage you not to rush through multiple challenges per day, instead slow down, focus on code quality and ask questions to get more feedback.

  I'll add some Screenshots to slack for you to show what I'm seeing on mobile with lots of empty space and the card in strange positions. The card is also missing the shadow from the design

  Much more important than that though is the html needs rewriting

  • you've used almost entirely heading tags. They serve a very specific purpose and are the main method of giving structure to the Web document. They are used by search engines and screenreaders to parse and navigate a page and it's essential they are used correctly, and in the right order.
  • this component should only have one heading
  • text should never be in spans or divs alone. Always use a meaningful element (eg paragraph, list, button, anchor, img, form, heading are all examples of meaningful elements)
  • the stats should ideally be an unordered list. There are some other options that could work but that's simplest
  • look up how and when to write image alt text. The card background image is definitely decorative so alt should be blank. The profile image needs an actual description of who the image is of - even a name is a better alt than generic text

  I haven't looked at the css yet but those html issues are the first that need addressing

  Marked as helpful

  0

  ---

  

  T

  Grace•31,270

  @grace-snowPosted over 2 years ago

  Some big issues in the css from a quick glance too.

  1. Never ever ever do this please I beg cannot emphasise enough: html { font-size: 10px; }. If you did that with % it would be marginally better but only if you are aware of how to mitigate against the accessibility issues you may be introducing. It's essential that text is a readable size and that's why all browsers have agreed a default. By scaling down you have to be aware of potential consequences for people. Add to that, it actually brings no benefits to you whatsoever. Designers don't work in 10px units and you don't need to either. Let 1rem be 1rem. Let users change that as they need to. "Because it makes it easier for me to work in rem" is not a good reason. Once you progress further you'll never have to work out px/Rem, it won't even be relevant

  2. Never resize the body. Think of that as your piece of paper - the body is the users screen. Limit things within it if you want but never limit the body

  3. large margins are not how to build layouts. Remember also to remove extra margin declarations if you have multiple under one selector

  4. Card does not need a width in px, and definitely doesn't need a height at all. Let it be width 100% and give it a max-width. Then you are letting it grow as wide as space allows up to a point. You never want to limit height on cards like this, that will only introduce bugs. Let the card be as tall as it needs to be based off the content and margins or paddings within it

  5. A 1440px media query for larger screens is waaaay too late. For example my macbook pro screen is about 1200 wide - think I should see the mobile version on desktop? Choose a much smaller value

  I hope all this is helpful. Sorry for the font size rant, just need to get this one across, it's so common to see it in beginner projects because of poor tutorials out there

  Marked as helpful

  0
• 

  Jeffrey Tamatini TEAHUI•20

  @tamatiniPosted over 2 years ago

  Heyo,

  nice job on this challenge.

  here some point i think you should look for.

  • as Samuel said above, you should use a wrapper for your content. Instead of applying your style on your body, you can either use the main tag or a div. In good practice you rarely apply style to the body appart from resetting your positioning and apply essential style such as default font, default size and background color, etc...

  • also you applied a width on your body which break your design on higher resolution screen, and moves everything to the left side of the screen.

  • an easier solution to handle image positioning would have been to use a wrapper and apply a relative position, then add the image in your html with absolute position applied documentation: https://developer.mozilla.org/en-US/docs/Web/CSS/position

  Marked as helpful

  0
• 

  Shashree Samuel•9,260

  @shashreesamuelPosted over 2 years ago

  Hey good job completing this challenge

  Keep up the good work

  Your solution looks great however I think that the lower half of the card needs some margin on the bottom using margin-bottom.

  In terms of accessibility issues simply wrap all your content between main tags

  I hope this helps

  Cheers Happy coding 👍

  0