Skip to content
OverviewSolutions
5
Comments
19
Chris
@ChristopherParke

All comments

  • Chris•240
    @ChristopherParkePosted over 2 years ago

    Hi Drew,

    You did a good job on the animations, and it's cool that you used Next. My only issues with it were that the spacing wasn't quite accurate to the design. Nor was the typography. Also, I'm not sure if its a FOUC that I'm seeing every time I click a link in the navbar, or an animation -- but either way I don't like it. Also there is a weird scrollbar on some of your body images at certain larger screen widths. At some tablet-sized widths, your images actually cover their accompanying text.

    Great try.

    1
  • Anietie Akpanumoh•30
    @TukooldegreatSubmitted over 2 years ago

    1. I had difficulty using CSS flexbox to style and opted for the Grid. Do you think I would have achieved this much easier with CSS flexbox? I love flexbox by the way but couldn't just figure it out on this project.

    2. I struggled with using SASS and opted for Vanilla CSS.

    3. What's the best practice in styling your web pages using CSS?

    Responsive landing page using CSS Grid

    1
    Chris•240
    @ChristopherParkePosted over 2 years ago

    Grid is definitely the right choice for this one imo. Getting the vertical spacing with flexbox can be difficult. Any container that requires different vertical spacing for the children, I tend to choose grid. It just behaves so much simpler.

    We used to prefer flexbox over grid because of browser compatibility. But by now grid has been adopted by all major browsers. So grid is more and more the best choice.

    0
  • Sourabh•420
    @SOURABH358Submitted over 2 years ago

    Hey, guys check out my solution for Bookmark Landing Page took me some time to do it using Tailwind CSS. Also, check out my code, and do suggest some improvements to help me improve. Thanks ✌.

    bookmark landing page

    1
    Chris•240
    @ChristopherParkePosted over 2 years ago

    Not too bad. I see from the screenshot that the footer is not quite sticky. You can fix this by making the body have height of 100vh. I also like to make the body display as a grid. I do this because I like to make the page layout with grid since it's 2 dimensional, instead of 1 (like flexbox).

    body {
     display: grid;
     height: 100vh;
}

    Also, your mobile design is quite off. You should inspect your page with the "device toolbar" active in the dev tools. You will see that you have a side scrollbar and things aren't the right size or alignment.

    Marked as helpful
    0
  • Jérôme Haas•320
    @jeromehaasSubmitted over 2 years ago

    Hi community.

    For this one, I had no specific goal in mind. I just started to develop on my new Setup with the OS from Ubuntu and I wanted to get familiar with it.

    I used way to much '!important' in my scss and the logic for the toggle could be improved. If anyone has a hint on thoose two topics or anything else - it would be highly appreciated.

    Thanks in advance and have a nice weekend!

    Clock App with SCSS, Gulp, Webpack and VanillaJS

    1
    Chris•240
    @ChristopherParkePosted over 2 years ago

    Looks suuuuper good. I don't think you used too much !important either. I only counted 3. The only optimization I would consider is pretty much code flavor.

    I would go from using

    .class {
grid-template-areas:
      "quote ."
      "greeting . "
      "time toggle"
      "location toggle";
    grid-template-columns: 1fr 160px;
    grid-template-rows: 1fr auto auto auto;
}

    to using the shorthand:

    .class {
grid:
      "quote ." 1fr
      "greeting . " auto
      "time toggle" auto
      "location toggle" auto
      / 1fr 160px;
}

    Hope it helps.

    Cheers.

    0
  • Chris•240
    @ChristopherParkePosted over 2 years ago

    It looks really good! the only thing you seem to have missed is the media breakpoint.

    I see you have:

    @media screen and (max-width: 375px) {

    but you should have:

    @media screen and (min-width: 1440px) {

    as per the style guide. Note that I've changed your code from MAX width to MIN width, as is best practice. So you may need to re-arrange some of how you've styled it to make this work. The quick and dirty solution would be just keep max -- but you should get in the habit of using min. The reason is the base styles should be a "mobile first" approach (styling for smallest screen widths), and your media queries should control the style changes once the screen size becomes wider.

    You'll also notice it looks better if you do it this way if you resize the window. Larger mobile and tablet sizes are super squished on your current build.

    Awesome job!

    Marked as helpful
    0
  • Aryan•10
    @Aryan-Rahul-JaiswalSubmitted over 2 years ago

    Is there any better alternative way to make this using just html and css? Please let me know how i could make my code better.

    Qr Code Component Site

    3
    Chris•240
    @ChristopherParkePosted over 2 years ago

    Hey Aryan,

    Yes, there is. What you would do is put the code in a framework. I finished this challenge using React.js, so if you want to have a look at my profile, you can see how I wrote the code. Mine is also pixel perfect since I have pro and used the Figma.

    Cheers,

    Chris.

    0
  • Chris•240
    @ChristopherParkePosted over 2 years ago

    Looks great. My only note would be to use 3 different static sized images (mobile, tablet, desktop) rather than using relative measurements. At some screens the banner looks blurry, squished, or cut off.

    Cheers!

    1
  • Chris•240
    @ChristopherParkePosted over 2 years ago

    Hey there. Looks good! One main issue I see is that you have desktop html, and mobile html. That's not best practices. What you should do instead is have one html structure, and change it with CSS using media queries. I see you have media queries, so I would just think about how you can change the layout using them, rather than hiding and showing html elements. Cheers

    Marked as helpful
    0
  • Chris•240
    @ChristopherParkePosted over 2 years ago

    Looks good. My only note would be that yes the API fetch works, but it would be better in a framework like React or Vue. That's because it would be more portable and easier for others to understand and use your code. Great work though. Basically perfect.

    Marked as helpful
    1
  • Chris•240
    @ChristopherParkePosted over 2 years ago

    Really good job. Seems like it was quite a bit of work on the JS side. One thing I would recommend is using a front end library or framework like React or Vue. You won't have to try as hard for state management. Well done!

    Marked as helpful
    1
  • Wyatt•80
    @00WyattSubmitted over 2 years ago

    I used CSS grid on the numbers, but I couldn't seem to get them perfectly center, there seemed to be some space underneath the text I couldn't remove. Is there an easy way to fix this?

    Interactive Rating Component using CSS-Grid

    1
    Chris•240
    @ChristopherParkePosted over 2 years ago

    the easy way to center the numbers is to put them into a container and give it: .container { display: flex; justify-content: space-evenly; }

    and it's done.

    1
  • szam•800
    @k-stopczynskaSubmitted over 2 years ago

    Any feedback is much appreciated.

    Because of the positioning of the images I couldn't manage to keep it responsive. If someone could look at the css and point the place where I could improve it, I'll be gratefull.

    article preview component

    1
    Chris•240
    @ChristopherParkePosted over 2 years ago

    Looks great to me. Seems to be responsive and matches the design very well.

    Only thing it seems you missed is the box shadow.

    8/10

    0