Responsive Stats-Preview-Card-Component-Main using CSS

Community feedback

• 

  Raymart Pamplona•16,100

  @pikapikamartPosted about 3 years ago

  Hey, great work on this one. Desktop layout looks great and like what you said, changing screen-size makes the image not look good, though mobile state looks great.

  Well in here, the main problem is just the image's size and the other elements like text are fine I supposed, let's see if we can manage to fix that, but first:

  • Always have a main element to wrap the main content of your page. On this one, the .container should be using the main instead of div. Also, using margin to center a layout is not consistent, what you could do is that , first remove the margin in the .container and on the body tag, you could :

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

  This way it is more consistent and properly centered.

  • Also avoid using id attribute to style an element since it is a bad practice because of css specificity. Just use class to target element.
  • When wrapping a text-content do not just use div to wrap it, use meaningful element like a p tag if it just a regular text or heading tag if it is an heading.
  • Your .details could have used ul since those are "list" of information about the company website.
  • Lastly on the image, you could try to use background-size: 100% 100% or background-size: cover to make it occupy the whole size of the element.

  Aside from those, great work on this one.

  Marked as helpful

  1

  ---

  

  Nikesh•90

  @NikeshGamalPosted about 3 years ago

  Thank you for your detailed feedbacks. It was very helpful going into details made me realize the bad practices I am using and should avoid it. Really appreciate it.

  1