Frontend Mentor | Kevin's profile

@Slyssy

Submitted almost 2 years ago

This was my first time building a site from a Figma design template. It took me a little bit to figure that out. I would have probably done this differently without this design. I used more fixed sizes because I had that information. I don't feel like I was very efficient with my code, but I will get better.

Responsive Landing page using CSS Grid.

Kevin• 110

@KKS1991

Posted almost 2 years ago

Hi Stephen,

I think we've somethin in common, we both are rookies but nevertheless I want to offer you my humble feedback. 😊

First at all - i like your work, good job Stephen. Especially I like the card-effects you've used. In regard to your "Report". There are some accessibility issues. You need to "frame" your code into an "main" section like this.

In my opinion it seems you need to cluster more of your content in sections as well like you did here:

I also had this problem in the past and someone recommended me the solution above. With regard to the HTML Validation it seems your chosen values for the images issues errors. If you take a closer look to the given design you can see, that there are two Breakpoints.

Mobile
Tablet (min-width: 740px)
Desktop (min-width: 1440px)

If you use Figma like me, you need the shortcut „v“ and a right click on the design-layout. The tablet-version has a width of 720px and the desktop-version has a width of 1440px. For the mobile-version there is no need for setting a media, because the image for the mobile-version (image-hero-mobile.png) displays till a width of 720px is reached. I suggest you following solution:

      <source media="(min-width: 1440px)"
              srcset="assets/image-hero-desktop.png" >

      <source media="(min-width: 720px)"
              srcset="assets/image-hero-tablet.png">

      <source srcset="assets/image-hero-mobile.png">

      <img class="hero__img" src="assets/image-hero-mobile.png" alt="A woman with a cup">

With a closer look on your style.css-file i want to suggest you to choose more specific names for your classes. Instead of

.paragraph-1 ...

you should use something like

.title-text ... or .title-description ...

In the Desktop-Version the content isn’t over the whole side and it isn’t centered. Maybe a

margin: 0 auto;

on your „page-container“ will solve the centering issue. If you’re looking for great and free CSS/HTML-Tutorials I can totally recommend you the Youtube-Channel from Kevin Powell (https://www.youtube.com/kepowob).

How I just said – good job and keep coding. 😊

Marked as helpful

Kevin

All comments

Responsive Landing page using CSS Grid.