blog-preview-card-component

Community feedback

• 

  P

  Kamran Kiani•1,280

  @kaamiikPosted about 1 month ago

  Hi. I have some notes I wanna mention to you:

  1. There is some issues with your html. For the main image because It is decorative as I see, you do not need alt message. Even If you needed one, you have to use a good alt message to describe your image to a screen reader. For the profile picture you used an alt message but It's not describe it very well. So use a proper alt first.

  2. You can not wrap two interactive element with each other. button and a tags are both interactive. Whenever you have a button shape in your design, If this button take you to a new page, So you have to use a otherwise button is a choice. So you don't need both of them together. Now think for this, Which tag is proper.

  3. I think for your a tag and the date, you do not need to use div. Also for your h1 and p There is no need to use it. Also For the profile name and picture, You need to have a space between these two. You can simply use flexbox for this.

  4. Your .attribution is a footer so first it should not be inside your main then use footer tag instead of div.

  5. On your CSS, You never ever have to limit your width and height for text containers. One thing you need for this and many other projects is to use max-width in rem unit` So your container does not bigger and bigger.

  6. For your padding and margins also try to use rem and em too.

  At the end, unless you refactor this project, Do not start the next project. This is really important to solve this project problems then go for the next one.

  Marked as helpful

  1

  ---

  

  Bintou karah•50

  @KarahDotjsPosted about 1 month ago

  Hi,

  Thank you so much for your detailed feedback! I appreciate you taking the time to go over these points.

  CSS adjustments: I’ll replace hard width and height values with max-width in rem units for text containers, and start using rem and em for padding and margins where appropriate to improve scalability.

  Finally, I’ll make sure to refactor @kaamiik

  0
• 

  ِAhmed•650

  @Axsel519Posted about 1 month ago

  card-container needs to add a margin at the top

  Marked as helpful

  0

  ---

  

  Bintou karah•50

  @KarahDotjsPosted about 1 month ago

  @Axsel519 Yes I tried but doesn't worked

  0

  

  ِAhmed•650

  @Axsel519Posted about 1 month ago

  @KarahDotjs write this

  .card-container { margin: auto; width: 380px; padding: 30px; background-color: var(--White); border-radius: 10px; box-shadow: 6px 3px 0px rgba(0, 0, 0, 5); margin-top: 5vh; }

  Marked as helpful

  0

  

  Bintou karah•50

  @KarahDotjsPosted about 1 month ago

  @Axsel519 Thanks you for you help now it's working well I don't know can use vh unit for margin

  0

  

  ِAhmed•650

  @Axsel519Posted about 1 month ago

  @KarahDotjs You can use px as you like but I like to use vh and vw because they fit the screen dimensions and are comfortable for me + there are many units you can use

  Marked as helpful

  0

  

  Bintou karah•50

  @KarahDotjsPosted about 1 month ago

  @Axsel519 You're right, it's true that when I look at other people's projects, most of them use vh and vw for the units or rem and em

  0

  

  ِAhmed•650

  @Axsel519Posted about 1 month ago

  @KarahDotjs Yes you can use whatever is comfortable for you. + Write to me if you need help.

  Marked as helpful

  0