Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

Submitted

four card feature section made with html and css

Abdihafid Adan• 800

@Abdul400

Desktop design screenshot for the Four card feature section coding challenge

This is a solution for...

  • HTML
  • CSS
1newbie
View challenge

Design comparison


SolutionDesign

Solution retrospective


please feel free to leave any feedback. Thanks

Community feedback

Raymart Pamplona• 16,090

@pikapikamart

Posted

Hey, great work on this one. Desktop layout looks fine, just needed for the background-color of the site a bit dim and font-sizes for me are too little. Mobile state looks fine, but resizing it first, the layout is very squished, you could made something like a 2x2 column before going into mobile view so that the layout won't look squished and transition properly.

Ken already gave great feedback on this one, just going to add some suggestions as well:

  • Avoid using vh unit as the font-size value, never use it. It might look good on your end but users who have smaller screen-size-height will get different sizings. Use rem values instead to make it more consistent.
  • Avoid using height: 100% or height: 100vh on a large container like the body as this makes the element's height capped based on the viewport/screen's height. Instead use min-height: 100vh so that the element will expand if it needs to.

Okay ugm, removing the height the layout has just been destroyed. Looking at it, you almost use position: absolute to each element on the site. Why? Using position: absolute removes an element from the flow and must be used only when you are sure that you control the element properly. On this one, it is not idea to or in general, use position: absolute on each element. It would be great to have another go with this challenge and complete it without using position: absolute.

But still, here are some continuation of my suggestions, but I won't include css for now, those are on your part:>

  • You should only use a single h1 to wrap those text above. That is just a single phrase so only use 1 h1 to wrap those text. Use max-width to make the text wrap, adjust it until you get the desired look.
  • Avoid using id to target and style an element since it is a bad practice due to css specificity. Instead, just use class to target element.
  • Each of the card titles should be using h2 and not h3. When using a heading tag, make sure you are not skipping a level. If you use h3 then make sure that h1, h2 are all present.
  • Those card icons are only a decoration on the site. Decorative images should be hidden for screen-reader at all times by using alt="" and aria-hidden="true" to the img tag or only aria-hidden="true" if the image is using svg.
  • Also, only use descriptive alt on images that are meaningful and adds content to the site otherwise hide the image for screen-reader users.

Those only for now, but as I suggested, it would be great to have another go with this one with proper css usage.

But still, great job for this one.

  • When using img tag, you don't need to add words that relates to "graphic" such as "logo" and others, since img is already an image so no need to describe it as one.

Marked as helpful

1

Abdihafid Adan• 800

@Abdul400

Posted

@pikamart thank you so much for your feedback. Your feedback is very in-depth and I really appreciate it. I will definitely redo the CSS code by incorporating your advice and also that of Ken. Thank you so much :)

1
Ken• 935

@kenreibman

Posted

Looks good at 1440px initially.

I would add a breakpoint around 1300px and make a "tablet layout" as the containers get too squeezed together.

I think you also forgot to make a mobile layout for this project at 375px.

In fact, I would suggest using max-width with a set value (rem) for your containers. Using % is the reason why your containers stretch like that, and get too small when the resolution shrinks.

You can then adjust the containers to become smaller or larger as you change screen resolutions.

Your heading levels should also increase by one. You jump from h1 to h3 in your project.

I hope this helped! Good luck on your future projects.

Marked as helpful

1
Abdihafid Adan• 800

@Abdul400

Posted

Hello @Imaoken. Thank you so much for your feedback. I will definitely consider a more tablet friendly layout. in regards to the mobile layout, I intentionally started at 450px to cater for 'phablets'. I used chrome's inbuilt mobile simulator and found that that is the case. Phones these days have very big screens and so I thought it would be wise to start mobile design at a higher resolution than 375px.

To a certain extent, the % were very responsive and it was a quicker solution instead of using a lot of media queries. I will definitely take your advice into consideration regarding the % because I've also noticed although its responsive, it be very tricky for smaller devices as it gets squeezed and stretched although a workaround can easily be done with media queries.

0

Please log in to post a comment

Log in with GitHub
Discord logo

Join our Discord community

Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!

Join our Discord