Skip to content
OverviewSolutions
29
Comments
39
Favourite Jome
@jomefavourite

All comments

  • Favourite Jome•1,110
    @jomefavourite

    Hello Anna!

    I really love how you take your time to make the layouts of each project you've done pixel-perfect, and also add really smooth animations. I wish I spent more time on animations 😅

    Welcome back to coding!

  • Favourite Jome•1,110
    @jomefavourite

    Hi Adeola, nice solution.

    The email validation is okay, using just HTML to validate is fine. I guess you could change the text alignment to left when you're on desktop view by adding a media query.

  • Favourite Jome•1,110
    @jomefavourite

    Nicely done, Agata!

  • Favourite Jome•1,110
    @jomefavourite

    Hello Clinton, you did a very good job on your first challenge and here're my suggestions.

    • I noticed that the section element with the class name site has a div element with an image element with no src attribute link which produces an unnecessary space in the layout. I suggest removing that div element.

    • Also, I suggest applying a max-weight property to the paragraph p elements in other for the text not to be stretched out on bigger screens.

    • Also the images, should have a max-weight property applied. For example:

    .device_showcase .device {
    width: 75%;
    max-width: 1000px;
}

    The max-weight property prevent the image from growing along with the viewport when that max weight is reached.

    • Finally, I'll suggest following a much better semantic layout structure. I noticed you didn't include the main element. Well, this is my personal structure though. Example below
    <header>...</header>
<main>...</main>
<footer>...</footer>
  • Favourite Jome•1,110
    @jomefavourite

    Very good solution, it's very close to the design.

    1. Adding a max-width to the selector .mobile-flex-item would fix it from growing. You could try max-width: 350px;.

    • top: 100% simply means that the element should be at the bottom of the viewport.

    Think of it like you want white space to be at the top of the viewport, so you assigned a value of 100%, which pushes the element to the bottom.

    • bottom: 100% is the opposite, so the element would be at the top of the viewport.
    • right: 0% simply means there shouldn't be any white space left on the right side of the screen. So therefore the element should occupy that white space, that's why the element moves to the right.
    1. It depends, there isn't one right way of achieving a responsive design. I noticed you use percentages along with the values in your margin/padding to achieve the device. I'll try to avoid using percentages though, it could cause some issues on smaller issues but ah, you pull it off.

    Keep on coding, I hope my explanations were simple enough.

  • Favourite Jome•1,110
    @jomefavourite

    Very good solution, it's very close to the design.

    1. Adding a max-width to the selector .mobile-flex-item would fix it from growing. You could try max-width: 350px;.

    • top: 100% simply means that the element should be at the bottom of the viewport.

    Think of it like you want white space to be at the top of the viewport, so you assigned a value of 100%, which pushes the element to the bottom.

    • bottom: 100% is the opposite, so the element would be at the top of the viewport.
    • right: 0% simply means there shouldn't be any white space left on the right side of the screen. So therefore the element should occupy that white space, that's why the element moves to the right.
    1. It depends, there isn't one right way of achieving a responsive design. I noticed you use percentages along with the values in your margin/padding to achieve the device. I'll try to avoid using percentages though, it could cause some issues on smaller issues but ah, you pull it off.

    Keep on coding, I hope my explanations were simple enough.

  • Favourite Jome•1,110
    @jomefavourite

    Great job, I like the work and here are my feedbacks

    • Making use of the % unit to determine the overall width, using margin and padding of each section, without including max-width property will make the layout to keep growing on large screens.

    • Adding a max-width property to the texts on the page will be very helpful, so as to stop the texts from growing at a certain point.

    • The model with the selector .access, I noticed it isn't always centered so I made some changes.

    .access {
    padding: 5% 5%;
    box-shadow: 3px 3px hsl(0deg 0% 5%);
    height: auto;
    width: 80%;
    max-width: 700px;
    /* margin: -10% 21%; */
    background-color: hsl(217, 28%, 15%);
    position: absolute;
    margin-top: -110px;
    left: 50%;
    transform: translateX(-50%);
}

    Also on the media query

    @media (min-width: 320px) and (max-width: 500px)
.access {
    /* margin: -2% 5%; */
    padding: 1.5rem;
    width: 90%;
}

    Lastly, I'll like to talk about the percentage unit you used frequently on margin and padding, it might cause issues as the viewport increases so I'll advise you make use of other responsive units like em, rem.

    Then on the width of each sections, you can use percentages. Well, it varies and depends on the use case.

    Here's an article I wrote that might be helpful.

    Also, I'll strongly advise you take this course Conquering Responsive Layouts by Kevin Powell.

  • Favourite Jome•1,110
    @jomefavourite

    100 % 🎉🎉 I love the animations you added. I've got to try out this challenge too

  • Favourite Jome•1,110
    @jomefavourite

    Wow, the animations are just amazing 🤩

  • Favourite Jome•1,110
    @jomefavourite

    Great work. I remember battling with this same challenge so you can checkout my solution here.

    It's not also perfect though but hope it helps

  • Favourite Jome•1,110
    @jomefavourite

    Nice solution but I noticed you used padding: 60px to surround each section.

    The only problem with this is that on a bigger screen it fails, I mean the layout looks weird. So I'll suggest using something like this:

    .container {
  width: 90%;
  max-width: 1440px;
  margin: 0 auto;
}

    Using those properties will ensure the width is always 90% until it gets to the max-width

  • Favourite Jome•1,110
    @jomefavourite

    Woo your solution is really amazing 🤩

    I've been battling with the same challenge for a week now using React.

  • Favourite Jome•1,110
    @jomefavourite

    Hello @OmoniyiTim, nicely done 👍

    But here are my thoughts:

    • You didn't include a max-width, so it too stretched out.
    • You probably forgot about the font-family (I do too 😁)

    Here's my solution, you could go through the code.

    P.S : I'm not saying my approach is the best but from what you did, you need more work on responsiveness. Just my thoughts

  • Favourite Jome•1,110
    @jomefavourite

    Hello

    I noticed you did a torn of projects today. My feedback for the project will be at this point padding having % isn't good mostly on a large screen.

    @media (min-width: 1440px)
.showcase {
    width: 100%;
    /* padding-top: 7%; */
}

    Having a fixed height also causing some issues

    @media (min-width: 1000px)
.header {
    background: url(./images/bg-hero-desktop.svg) no-repeat center center/cover;
    padding-top: 3rem;
    width: 100%;
    /* height: 550px; */
}

    You could use mine as reference here

    Note this was my very first challenge from frontendmentor so it's not that's good but it doesn't have the issues yours' have.

  • Favourite Jome•1,110
    @jomefavourite

    It isn't mobile friendly 🤔

  • Favourite Jome•1,110
    @jomefavourite

    Hi, you could try out details tag to achieve this task. That what I did, here is the my solution

  • Favourite Jome•1,110
    @jomefavourite

    Nicely done, desktop view it looks amazing but it isn't mobile friendly. You should probably work on that 👌