blog-preview-card-main

Community feedback

• 

  T

  Grace•31,270

  @grace-snowPosted 8 months ago

  Hi, I hope you find this feedback helpful. I'll just list it out so hopefully it's easier to follow:

  1. Don't include empty divs in html. There is no reason to use background image over image for this card, and it would have a negative impact on performance if you did this on this kind of component in a real site.
  2. Think about the context of where this component would be used when you build small components like this. This card would very likely be alongside other blog cards —multiple would be used on one page. it is extremely unlikely that this type of card would ever act as a page title which tells you it must not have a H1. H2 would be much more appropriate.
  3. There’s currently no way for anyone to access the blog being signed posted by this card because you’ve not included an anchor link anywhere. There should be a link inside the H2 wrapping the blog title text. (Always test solutions with your keyboard as a good step to ensuring you've used interactive elements correctly)
  4. to make the whole card clickable you would make the card position relative, then add a pseudo element on that link, absolutely positioned and sized to fill the space of the card.
  5. "Author image" is not an appropriate alt description of the image. alt should never include words like "image" because it’s already got an image role. take a look at the excellent post in the resources channel on discord about how and when to write good alt text.
  6. The author name is definitely not a H2. Using headings appropriately is very important. They must only be used when something is an actual heading for content underneath it. this author name is just a paragraph.
  7. get into the habit now of always, including a full modern CSS, reset at the start of the styles in every project. Andy Bell or Josh Comeau, both have good examples you can look up and use.
  8. there is a bug in your code caused by limiting the height of the body to 100 VH never limit the height of elements that contain text, including the body. Instead use min height. this will allow the body to extend beyond the limit of the viewport height when necessary.
  9. The main landmark should not be your card component. This component would sit on a page within main. Therefore, the component must be its own div with a class so it can be styled independently.
  10. there is no benefit to making the card into a flex column, unless you’re planning on using the gap property. all of the child elements within the card stack vertically by default.
  11. The card image must not have a explicit width. Once you have changed the image to be an img element instead of background image the sizing would be simple: A standard part of the CSS reset would be to set image elements to have a max width of 100%. That is all this image needs. if you were still using background image, the div would not need a width setting in CSS, but the background image would be sized using background properties.
  12. The whole card should have padding. The child elements within the card should only have vertical margins. Make sure you understand the difference between padding and margin.
  13. there is no need for a content div at all in this challenge, and it definitely would not need an explicit width.
  14. this follows on from the point above about including a link. Remember that anywhere you see a hover style in a design. It means that there is an interactive element there. Never just add cursor, pointer and hover styles to non-interactive elements as they won’t actually function.
  15. delete the media query and everything within it in the CSS. This challenge does not require a media query.
  16. when you do need media queries in future challenges, make sure you know how to use them well. You should be styling mobile first that means the mobile styles are the default CSS and the larger screen overrides, go within a min width media query. Media queries must always be defined in rem or em not pixels.

  Marked as helpful

  1
• 

  YasserAO•330

  @YasserAOPosted 8 months ago

  Hello, @Sarank

  The problem consists of using a fixed values of width in your containers which is not advisable for a responsive design. because when you use a @media query you will have to redefine every single item.

  The solution would be to use a relative width like % and vw, and Center the container using either flex parent or adjusting the margin.

  example(using margin to center)

  main {
  width:90% ;  //this means no matter what my window size it , it will always leave a 20% of white space.
  margin : 0 auto ; //0 is for top and bottom , it will do nothing, auto is to center the item between left and right
  }
  // now lets do some queries
  @media(min-width:375px){
   width : 360 px;
  }
  @media(min-width:768px){
   width : 400 px;
  }
  

  1. With the Code above, if my window width is under 375px (small screen sizes) the main width will be always 90% of window's width, which is gonna take all the Device screen as expected without crashing our content (10% of space is left and spread over the margins).

  2. If the window is at least 375px or higher the main-width = 360 px ,If the window is at least 768px or higher the main-width = 400 px. Ofcourse you can add more queries if the it seemed very small for higher window sizes.

  hint: you can apply this to the content of main, so that it would change relative to the main container, and that's how you'll get the responsive look.

  i hope this comment helped, good luck.

  1

  ---

  

  T

  Grace•31,270

  @grace-snowPosted 8 months ago

  @YasserAO this isn't a great approach tbh. All the component needs is a single max width in rem. No widths should be set and there is no need for a media query in this challenge.

  If you did need a media query in future challenges, it would also be important to define it in rem or em not px.

  1

  

  YasserAO•330

  @YasserAOPosted 8 months ago

  @grace-snow that's helpful, thanks

  0