Submitted 6 days ago

Four Card Responsive using 3 grids

justine1607• 90

@justine1607

Preview site(opens in new window)View code(opens in new window)

This is a solution for...

Four card feature section

HTML
CSS

1newbie

View challenge

Design comparison

SolutionDesign

Solution retrospective

What are you most proud of, and what would you do differently next time?

the most I am proud off is how I align the 1st card to be exactly the same as the design, i was not expecting that i can make it like that i thought it would be so hard do it.

What challenges did you encounter, and how did you overcome them?

the one i mentioned above that I think is the challenge I have encountered. I overcome this challenge by motivating my self that i can really make it

Community feedback

Djamel1133• 470

@Djamel1133

Posted 2 days ago

ok ...Keep it up and see you next challenge.

Marked as helpful

Kamran Kiani• 900

@kaamiik

Posted 5 days ago

I see your heading like this: A h2 tag that has a strong tag inside of it. I think the whole is a heading.
Maybe It's better that your cards heading be a h3. It seems a one level reduce with your top page heading.
I suggest you add a proper CSS reset to your style. Andy Bell and Josh Comeau both have a good one.
Use rem for your font-size and max-width instead of px.
Your grid layout need 4 rows. It's better for your layout because If your content changes, your grid items will mess up. Check this link on discord: https://discord.com/channels/824970620529279006/1214884151728152577/1215761500225011872
Instead of height: 100vh; It's better to use min-height: 100vh. This works better.

justine1607• 90

@justine1607

Posted 3 days ago

@kaamiik thank you for your feedback.

i will try to change the h2 into h3.
yes next time will do add a proper css reset for my styles.
i believe i use rem for my font-size, just in max-width i didn't but will change it to rem.
for this I commented also https://discord.com/channels/824970620529279006/1214884151728152577/1215761500225011872
okay noted on using min instead of height

Djamel1133• 470

@Djamel1133

Posted 5 days ago

Great job, and what clean code! I really liked it.

I completed the same challenge on Frontend Mentor, and they asked me to provide feedback to other coders on it. So, I have a few questions if you don't mind:

Why didn't you use variables for font-size and font-weight as you did for colors?

:root {
  /* Primary colors */
  --Red: hsl(0, 78%, 62%);
  ...
  /* Font weights */
  --font-weight-small: 200;
  ...
  /* Font sizes */
  --font-size-small: 1rem;
  ...
  /* Spacing */
  --spacing-small: 1rem;
  ...
}

Why did you use px instead of rem?

  margin-bottom: 76px; 
  margin-top: 15px; 
  gap: 15px;
  ...

You could also use calc or clamp for more flexibility. Check out this article: Typography in Web Design.

Why did you use a div inside another div in your HTML code? Why not insert HTML elements within a single div like this:

<div class="card__supervisor">
  <h3>Supervisor</h3>
  <p>Monitors activity to identify project roadblocks</p>
  <img src="images/icon-supervisor.svg" alt="Magnifying glass icon with an eye in the center">
</div>

Finally, I picked up some tricks from your code to optimize mine. Thank you, and well done again!

justine1607• 90

@justine1607

Posted 3 days ago

@Djamel1133 thank you for your'e feedback I do appreciate it. for your questions:

I believe that the font-size and the font-weight should really be there, it just i forgot to put that there.
i use px for this because i want to not change so it will have that value as always.
i use another div inside the div, it's purpose is for wrapping the element so later we can add style on it.

Please log in to post a comment

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