Blog preview card: nunjucks SCSS

Community feedback

• 

  T

  Grace•31,270

  @grace-snowPosted about 1 month ago

  One thing I notice is you're misusing headings in this.

  This is a card that should only have one heading. The publish date and author name are paragraphs (they are not acting as headings for other content, they are just information about this blog).

  This card would never be used to serve the main heading on a page. So you know it shouldn't have a h1. Use a lower importance heading level like h2 instead for the blog title.

  The image alts also need making blank in my opinion. Neither image brings any beneficial information to the content.

  I'm not sure what the card-header div is for... You shouldn't need any extra wrapping element.

  I'm confused actually... I was reading the pug file but it's like the markup is stored in 3 different places and I'm not sure which is the one to look at...

  The main image shouldn't have a height. Just the css reset content is enough for that image - display block and max-width 100%. You can give it an aspect-ratio property if you want a performance boost.

  There's no need for any media query in this challenge.

  There is no need for a min-width variable... The card only needs one max width and this should be in rem. That way it will adapt to all screen sizes and the layout will work even if users enlarge their text size.

  The component shouldn't ever hit of come very close to screen edges. (Usually people add some padding to a wrapping element to prevent this). Remember in this challenge the cards have that thick shadow so they may need some extra space on the right and bottom to make sure it can't get cut off.

  Marked as helpful

  1

  ---

  

  ortiz-antonio•180

  @ortiz-antonioPosted about 1 month ago

  @grace-snow Thank you!

  I’ve made the following changes based on your recommendations:

  • Updated the headings as per your advice—can’t believe I missed that one!
  • Adjusted the alt text for images. I agree, the images are decorative.
  • Apologies for the confusion about the Pug files. I had used Pug in a previous version but forgot to remove them. I'm now using Nunjucks (I prefer the HTML syntax and JavaScript-like variables), and the partials are located in _includes/src/partials.
  • Added padding to the component container so it doesn’t touch the screen edges.

  I have a few questions:

  • The card header div is used to manage different gaps between elements. I could switch to using margin-top instead. I remember you mentioned in accessibility, less is more. When I check the accessibility tool in Chrome, the card header div gets ignored. Would switching to the margin-top approach improve accessibility?
  • The main image has the same height on both mobile and desktop layouts in the figma design. If I make it proportional, the image height will be shorter on the mobile version. Do you think I should keep it consistent or adjust it?
  • Regarding media queries, I know it’s possible to use clamp, but I find it challenging to adjust font sizes for the tag, date, title, and description. Each would need a separate clamp, but the design doesn’t require scaling between values.

  Thanks again for taking the time to review my work, and I apologize for all the questions. My learning approach is to fully understand things before making changes.

  0

  

  T

  Grace•31,270

  @grace-snowPosted about 1 month ago

  @ortiz-antonio

  If you want to use gap that's fine. If you want to use margin-top that's also fine. That's just a question of personal preference. It has nothing to do with accessibility.

  I'm not sure quite what you mean about the image height. You still have an unnecessary media query in the styles which is changing the width of the card and that will impact the height of the image. You don't want a fixed height on an image as (as well as it being unnecessary) it's going to give it a completely different aspect-ratio on different sized small screens. If I viewed on a very narrow screen for example the image would almost become square if it has a fixed height.

  Changing the font sizes is unncessary.

  Marked as helpful

  1

  

  ortiz-antonio•180

  @ortiz-antonioPosted about 1 month ago

  @grace-snow

  • I understand the issue with the image height now. When I set a height, the image stops being responsive. I was a bit confused because, in the Figma mobile and desktop versions, the image has the same height but different widths.
  • The font sizes for the tag, date, title, and description are different in the Figma design for mobile. They are all 12px, except for the title. However, the 12px typography size is not part of the Figma design system.

  0

  

  T

  Grace•31,270

  @grace-snowPosted about 1 month ago

  @ortiz-antonio 12 is extremely small. I would ignore the font resizing.

  Marked as helpful

  1

  

  ortiz-antonio•180

  @ortiz-antonioPosted about 1 month ago

  @grace-snow It's done, thank you

  0
• 

  jevcenkokozlovska•150

  @jevcenkokozlovskaPosted about 2 months ago

  Looks perfect

  Marked as helpful

  1

  ---

  

  ortiz-antonio•180

  @ortiz-antonioPosted about 2 months ago

  @jevcenkokozlovska Thanks, if you need any help reach me out :)

  0