First project v1.1: Vanilla CSS/HTML from XD design

Community feedback

• 

  T

  Grace•31,230

  @grace-snowPosted almost 4 years ago

  Hi again Jeremy :)

  I've had a chance to look through your code as requested and have some more suggestions for you now. I hope you find these helpful - please upvote comments if they are helpful as that helps me learn how to mentor well too

  1. Yes you need to be using html tags designed for text for your text elements, such as paragraphs, headings or list items. You can place classes on spans inside these tags if you need to adjust styling for each part, like in the statistics. e.g. <div class="item-number bold-blue-font">80K</div><div class="item-text">Followers</div> could become <p><span class="item-number bold-blue-font">80K</span><span class="item-text">Followers</span></div> with some css or a line break in the middle to make them stack.

  2. It's most common to put variables (css custom properties) on the :root element, but if you set up any that are specific to a particular component, you can put them inside that class and they will be scoped to it. This is handy if you need to override an existing variable for a one-off use case, for example.

  3. BEM is used in css as well, it's not a scss thing. So go for it if you want to use it!

  4. I think you could refactor this yes. Rather than duplication though, it's more that you're setting position absolute on things that wouldn't be necessary if you use background images, and giving some elements properties like z-index, height and width when they don't need it (see other points below)

  5. I've already covered for you. Because of the hidden overflow, you cannot see the card when you change to landscape orientation on a phone. So I would revisit this a bit more...

  Other general points

  • You shouldn't need to do anything to make this responsive, except maybe reposition the bg images for some screen sizes. You definitely don't need explicit widths and heights on your card. A max-width and a width of 100% would be fine, with the height not needing to be defined.
  • I would make the background images into background images, not place them in the html. In fact I think that's what i did in my solution for this
  • The profile image of Victor should have alt text. That is a meaningful image I think.
  • Some of your font is smaller than it should ever go. 10px is unreadable for a lot of people. This is a common issue with these challenges as the designer seems to favour very small fonts, but I would always overrule that and agree a sensible minimum with a designer.

  There you go, all done!

  1

  ---

  

  T

  Grace•31,230

  @grace-snowPosted almost 4 years ago

  Here is my version of this challenge if you want to take a look: https://github.com/grace-snow/fmentor_profile-card-component

  I recommend inspecting in devtools, including emulating mobile in different orientations. As you can see, no media-queries required on this one

  1

  

  Jeremy Ng•200

  @lanechangerPosted almost 4 years ago

  @grace-snow Thanks so much Grace!

  1. I think I take the paragraph tag too literally and was hesitant about using it for a single word but that's good to know.
  2. Funny enough I started off using background image for those 2 bg circles instead of <img> but had some z-index issues with the circles overlapping the card so ended up hacking it the way I did. I'll re-visit them as background images then!

  Yeah I was very literal with the design regarding that font 10 size. So in the "real world" it sounds like you're expected to make a lot more of these judgement calls then. Good to know.

  This has been very helpful, thanks again for taking time out of your day to go through my newbie code, I appreciate it! I will check out your solution and do some analysis from there :)

  0
• 

  T

  Grace•31,230

  @grace-snowPosted almost 4 years ago

  Hi Jeremy, that's a lot of questions, all great ones to be asking though!

  I'm just off to bed so will only touch on one answer for you for now.

  No 5 - yes you need to consider all viewport sizes, including smaller and larger than the supplied designs, and Everything in between. Design files are just meant to be a guide for us, and outside of those provided screen sizes frontend devs are meant to make sensible decisions about how best to present the content.

  You'll probably find it best if you try to work from the smallest screen sizes first as those will usually be the simplest and give a good base for your css. Then start to enlarge your viewport and add a min-width media query in when needed to adjust styles for larger screens. This may seem counterintuitive at first but will result in cleaner and easier to understand css with fewer overrides needed.

  Good luck with it! I'll try and take a look at your code tomorrow ☺

  1