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

HTML and CSS only

Tom 45

@TdZink

Desktop design screenshot for the Stats preview card component coding challenge

This is a solution for...

  • HTML
  • CSS
1newbie
View challenge

Design comparison


SolutionDesign

Solution retrospective


This is my first challenge. My goal was to be able to complete it with what I know. There are a number of things I could of done better and already recognized a few things I will change when I tackle the next challenge.

Would specifically like feedback around getting my main card container to stay center as the screen size changes. It does not have additional responsiveness items such as clamp or min/max but would like to at least have it stay centered.

Community feedback

T
Grace 30,550

@grace-snow

Posted

Hi,

I'm not sure why the project has such a complex set up? You are calling render blocking js in the document head, which doesn't seem to be being used. I recommend you remove that.

I also think you should take another pass at this challenge tbh. There is a quite a lot of learning points that need addressing before moving on.

On mobile for me, things are not positioned right and there is loads of scroll in all directions

I suggest

  • remove excessive divs in the html that aren't needed for this layout
  • semantic html - use meaningful elements for the stats. Numbers don't make sense as headings, and they need to be in the same meaningful element as the word that follows them
  • use css to capitalise text, don't write it like that in html
  • remove all position absolutes and transforms. Create the layout with flexbox. You'll find it easiest if you style mobile first
  • remove large paddings and margins
  • change units in css - font sizes must never be in px, use rem/em; line-height should be unitless like 1.5;
  • remove all the widths and heights. You won't need almost all of them. Just a max-width on the card, min-height on the image, maybe flex-basis 50% on desktop.
  • remove visibility hidden from attribution

I hope these suggestions help your learning

2

Tom 45

@TdZink

Posted

@grace-snow I appreciate your feedback. This will help when I take another pass at it.

In regards to the font size should you still set the main font-size provided by the guide in px then use rem/em for any changes away from the base?

0
Tom 45

@TdZink

Posted

@grace-snow I published a new solution being able to address some of the issues you pointed out. I have not yet been able to figure out how to remove all the widths and heights thought I did reduce how many I am using. Same with the paddings and margins.

0
T
Grace 30,550

@grace-snow

Posted

@TdZink no, font sizes should never be in px. You can change the html/root size with percentages but I wouldn't recommend that either. People with different zoom settings or visual impairments need the root size to be left alone and sizes in rem/em so it scales correctly for them.

I use a scss function to change px to rem and use it all over the place. It's just pixel-size-you-want / 16 so is pretty straightforward

1

@Gascoigne09

Posted

By the looks of it you've tried to use transform to centre your container. You'd need to include this i believe:

position: absolute;
top: 50%;
left: 50%
transform: translate(-50%, -50%);
2

Tom 45

@TdZink

Posted

@Gascoigne09 Thank you! This is what I ended up adding to my solution.

Appreciate the feedback.

0
T
Grace 30,550

@grace-snow

Posted

@TdZink I wouldn't recommend that technique. Instead make the outer container flex or grid and use their properties to center.

Eg display grid, place-content center; or display flex, justify content center, align items center

You may need min-height 100vh on there too

0
Tom 45

@TdZink

Posted

@grace-snow Thank you for your feedback. I appreciate it.

0

@vitorlfaria

Posted

Your design looks very good. The only thing remaining is to center it, like @Gascoigne09 said.

Other thing that you can do is use flexbox on the body. Something like this should work:

body{
   display: flex;
   align-items: center;
   justify-content: center;
}

Let me know if this works for you. Keep coding and happy coding! =D

1

Tom 45

@TdZink

Posted

@vitorlfaria Thank you! Used grid for the card elements did not think about applying flex from the body to center the card-container.

Appreciate the feedback.

1

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