Skip to content
OverviewSolutions
9
Comments
6

Nick Carlisi

@nickcarlisiAtlanta, GA180 points

Atlanta based web dev on a never ending quest for knowledge.

I’m currently learning...

- React, Node.js, Express

Latest solutions

View more solutions

Latest comments

  • Nick Carlisi•180
    @nickcarlisi

    Hi, you did a great job! A few things that jumped out to me at a quick glance...

    • It looks like some of the fonts (fraunces) don't match. The headers and buttons, some links.

    • When you open the mobile menu, it pushes the content below it down. Try making the mobile menu position: fixed or absolute. That way it won't affect the elements around it. You would then need to adjust some of the mobile menu styles, such as width and you can position the menu using top, left, etc. Also, a small but important thing that I think a lot of people forget is adding cursor:pointer to the hamburger menu to tell the user it's a clickable element.

    • The menu links don't seem to go anywhere. You can add an id to each section and then link to the id. For example, the section below the hero could have an id="aboutSection", then in the menu, the a tag would be, <a href="#aboutSection">About</a>. You could do this in the footer menu as well. When doing this, you also want to be sure to close the mobile menu when you click a link, the same way you would when clicking the hamburger. While you're at it, you might want to change the sections from <div> to <section>.

    • The client testimonials section looks a bit crammed when it first changes from column to row. Maybe try a different, larger breakpoint, so it doesn't change to row until a larger screen size that looks a little less crammed. Also, maybe adjust the padding and margin at smaller screen sizes, which would give it more room to "breathe"

    Hope those tips are helpful. Happy coding!

  • Nick Carlisi•180
    @nickcarlisi

    Hey Anthony,

    Good job!

    For your comment about mobile friendly designs, it's highly recommended to use a mobile first approach and then add media queries to change things up at larger screen sizes when needed. If you go to youtube and search ''kevin powell mobile first', the first video that pops up is really helpful. That guy is a great CSS teacher in general.

    Some things I noticed...

    • It looks like you used height in a lot of places when you really don't need it. You have a height on the container and the product image divs. In dev tools, I went in and removed all of the heights and everything looks pretty much the same. Often, you don't need to specify heights, you can let the content dictate the height.

    • I noticed you used flex-box more than necessary. I used to do this a lot. Once again, in dev tools, I removed display: flex (and other flex related properties) from .product-image and .product-description and things looked the same, so generally you can avoid writing excess code.

    • I noticed for the word perfume, you have it capitalized and you added extra spaces in the html. You should avoid manually adding spaces. Instead, in your CSS, you can use letter-spacing to control exactly how much space you want between each letter and you can use text-transform: uppercase to change the casing

    • Finally, you can avoid specifying grid-template-rows in this case. Since your grid container has only 2 children (.product-image and .product), when you set the grid-template-columns to 1fr, it automatically will add the second row.

    I hope these suggestions help. Happy coding!

    Marked as helpful
  • Nick Carlisi•180
    @nickcarlisi

    Hi Ibrahim,

    Good job! A few things I noticed that might help. For your grid, I think you might be over complicating it a bit, which is something I used to do myself. I noticed that you made your grid-template-columns: repeat (12, 1fr), and then each "box" spans multiple columns, but I recommend just setting the columns to the exact amount that you need. For example, on mobile, you can literally make your grid-template-columns: 1fr and then set a media query for desktop and change it to grid-template-columns: repeat (4, 1fr). That way you should be able to avoid setting the grid children to span columns and rows. They should automatically fall into place.

    Another thing I would recommend is avoiding making the images in the grid background images. If you just make them images with a width and height of 100% and then add object-fit:cover, you should be able to avoid setting a specific height to your boxes. If you want to take it a step further, you can use the <picture> tag and set the srcset according to screensize to choose the "mobile" version at smaller screen sizes and the "desktop" at larger sizes.

    Hopefully, those suggestions help. Happy coding!

  • Nick Carlisi•180
    @nickcarlisi

    One more thing I forgot to ask about is the 'joined' date. The API gives it to you in the ISO format, but the design has the month as "Jan" instead of 01, for example. Anyone know how to convert the month to text like that? Thanks again!

  • Nick Carlisi•180
    @nickcarlisi

    Awesome! Can't wait to implement these changes tomorrow and see how it turns out! Thanks so much for the thorough tips and recommendations!

  • Nick Carlisi•180
    @nickcarlisi

    Hi Brendan,

    I'm still working on this challenge myself, but regarding your first question, I had a similar issue. Try using percentages instead of rems in transform(translate). Hope that helps.

    Marked as helpful