2nd Project - 3-Column Preview Card Component

Community feedback

• 

  P

  Øystein Håberg•10,390

  @Islandstone89Posted about 1 year ago

  Happy to help. Been a long day, hope some of this makes sense xD

  Yes, you can copy and paste the Reset and use it in all your projects. Personally, I don't use everything, but I always include these:

  *,
  *::before,
  *::after {
    box-sizing: border-box;
  }
  
  body, h1, h2, h3, h4, p,
  figure, blockquote, dl, dd {
    margin-block-end: 0;
  }
  
  img,
  picture {
    max-width: 100%;
    display: block;
  }
  
  input, button,
  textarea, select {
    font: inherit;
  }
  

  In terms of dimensions, you could set a max-width in rem on the .containers. I used to use margin: 0 auto quite a bit previous, these days my preferred method to center something is to use Flexbox. You can do this on the body:

  display: flex;
  justify-content: center;
  align-items: center;
  min-height: 100vh;
  

  All block elements (paragraphs, headings, divs, etc) are display: block, so don't use it unless:

  1. You have an inline element you want to change
  2. You have given a display: flex and you want to change it back to display: block.

  " is there any good practice to have all elements inside a container with a white space in between such as img, h2, p, and a tags?"

  Do you mean how to space elements inside a container? There are several ways. If you want the same distance between all elements, you could set display: flex on the container, and give it a gap, usually in em or rem.

  " in the future where to apply certain fonts and their weight to the elements?"

  It depends. If the font should be used on all, or most of the elements, you should put it at a higher level, like the body. The descendants of the body will inherit the font properties. That is much more efficient than setting the same font properties on each of the elements.

  0

  ---

  

  FrontEndHighRoller•170

  @FrontEndHighRollerPosted about 1 year ago

  @Islandstone89 Thanks for the feedback

  Please check this out, I revised the project based on your comments

  https://github.com/FrontEndHighRoller/3-Column-Preview-Card-Revised-

  If there is anything to make it even better just let me know

  Cheers

  1
• 

  P

  Øystein Håberg•10,390

  @Islandstone89Posted about 1 year ago

  Hi, here are a few tips to improve your code.

  HTML:

  • Replace <section> with <main>. Every webpage should have a <main> that wraps all the content, except the header and footer.

  • The icons are decorative, so should have empty alt text: alt = ""

  • There should only be one h1 on a page. Change the headings to h2.

  • "Learn More" would navigate to another page, hence it must be a link.

  CSS:

  • You need a bigger CSS Reset. Andy Bell has one that is much used.

  • You don't need to set text-align: left on the container, as that is the default value.

  • Remove all fixed widths and heights. You rarely want to set fixed dimensions, as that causes overflow when the size is bigger than the viewport.

  • The columns should stack on top of each other on smaller screens. You need to set flex-direction: column on the .containers. It's considered best practice to have the mobile styles as default, then use media queries for larger screens.

  0

  ---

  

  FrontEndHighRoller•170

  @FrontEndHighRollerPosted about 1 year ago

  @Islandstone89 Thank you so much for the feedback, It a great to have someone more experienced who can help me improve in coding.

  I fixed all the HTML parts.

  In the CSS part I don't know the following:

  • 1 With the CSS Reset shall I copy and paste everything there and also use this setting for all of my projects?

  • 3 How to remove those fixed widths and heights and achieve the result that is necessary. I have a couple of options in mind:

  a) - remove the fixed dimensions from the .container element and then use the margin to size it as necessary on the .containers element

  b) - remove the fixed dimensions and set them to .container flex-basis: 300px;

  • 4 I am going to change that, but is the bad practice to use display: block; ?

  questions:

  1> is there any good practice to have all elements inside a container with a white space in between such as img, h2, p, and a tags?

  2> in the future where to apply certain fonts and their weight to the elements?

  0