Skip to content
OverviewSolutions
8
Comments
89
Amal Karim ✅
@amalkarim

All comments

  • Amal Karim ✅•1,290
    @amalkarimPosted almost 2 years ago

    Hi Enn Ko, congratulations for completing the challenge!

    Please check at this line:

    <img src="./images/image-hero-mobile.png" class="w-100" style="height: 450px;" alt="">

    I think it's better that you only set its width using class="w-100", or only set its height using height: 450px;. Don't use them both, because it will make the image not proportional in mobile view.

    We could also add alt for the hero image because it's an important image and adding alt will make it more accessible to all users. For example:

    <img src="./images/image-hero-mobile.png" class="w-100" alt="A curly hair man standing with a laptop on his hand in front of a yellow wall">

    Please refer to this article from W3Schools about image accessibility for more explanation.

    Hope this helps. Happy coding!

    Marked as helpful
    0
  • Amal Karim ✅•1,290
    @amalkarimPosted almost 2 years ago

    Hi Ted!

    When I visit your solution page using a smartphone in landscape mode, some of the content are hidden and I can't scroll down to see them. I believe this is not because of overflow: hidden; of the section element, but rather because of this style:

    html, body {
    height: 100%;
}

    When the height of the browser is not enough to show all of its content, some of the content become hidden, because the browser height is not allowed to grow taller than 100%. It can be solved by changing height: 100%; to min-height: 100%; or min-height: 100vh;.

    Other than that, I think it's quite a nice solution. But if you want to take the extra step, here is what I suggest:

    That's it. Hope this helps. Happy coding!

    Marked as helpful
    0
  • Dennis Zhang•320
    @ieatnSubmitted almost 2 years ago

    What's up. I was lazy so I decided to share the card component between the survey and submitted screens.

    1. How do I keep the button highlighted after clicking?
    2. Are the colors wrong? I tried all the colors for the background of the number buttons.

    Grid, Flex, JS Solution

    1
    Amal Karim ✅•1,290
    @amalkarimPosted almost 2 years ago

    Hi Dennis,

    How do I keep the button highlighted after clicking?

    You used <button> element for the number button. Button doesn't have on and off state, so it's difficult to make the button highlighted after clicking. Though I think it could be achieved by javascript, I'll consider it as hacking. The best element you could use for that is <radio>. It has on and off state (checked or unchecked), so when the user click a number, it becomes checked, and you can give it the style that you want.

    Here's an example.

    <input type="radio" id="rate1" name="rate" value="1">
<label for="rate1">1</label>
<input type="radio" id="rate2" name="rate" value="2">
<label for="rate2">2</label>
<input type="radio" id="rate3" name="rate" value="3">
<label for="rate3">3</label>
<input type="radio" id="rate4" name="rate" value="4">
<label for="rate4">4</label>
<input type="radio" id="rate5" name="rate" value="5">
<label for="rate5">5</label>

    You could then hide the input and style the label according to your need.

    input[type="radio"] {
    display: none;
}
input[type="radio"] + label {
    font-weight: var(--fw-h1);
    width: 40px;
    height: 40px;
    border: none;
    border-radius: 100%;
    color: var(--Light-Grey);
    background-color: hsl(213deg 20% 18%);
    display: flex;
    justify-content: center;
    align-items: center;
    cursor: pointer;
}
input[type="radio"] + label:hover {
    color: var(--White);
    background-color: var(--Orange);
}
input[type="radio"]:checked + label {
    color: var(--White);
    background-color: rgb(126 135 150);
}

    Hope this helps. Happy coding!

    1
  • Daniel Filipe Santos•70
    @Lipe-SantosSubmitted almost 2 years ago

    Hi guys, I am new to this community and I would like to know your opinion about this landing page. I joined this community because I really want to improve my HTML and CSS, so I really appreciate your advice and opinions.

    Responsive Landing Page

    3
    Amal Karim ✅•1,290
    @amalkarimPosted almost 2 years ago

    Hi Daniel,

    When I turn on my developer tools in chrome, which is sitting at the bottom of the browser, the <div class="container"> is overlapping with the element after it: <section class="main-content">. I think it's because you set height: 100vh; for the .main-header .container. It's better to remove it, or change it to min-height: 100vh;

    Also, please look at the <div class="main-header-mockup"></div>. In my opinion, it's better to place the image inside it using <img> tag rather than as a background-image. First, it's an important image that has meaning, not just a decorative image. Background image is best suited for an image that is just a decorative one. Please refer to this article about image accessibility. And second, in mobile view, the image is vanished. It can't be seen under the Get Started For Free button. By using <img> tag, that could be prevented from happening.

    That's it from me. Hope this helps. Happy coding!

    Marked as helpful
    0
  • Effy Zhang•60
    @Effyz1228Submitted almost 2 years ago

    I feel it's difficult to put the image and the production info inside my <article> component. How to evenly divide space between these two? I also feel hard to decide whether to set a width or height for my component?

    There are something wrong about my shopping cart icon as well.

    What are some suggestion for using media query?

    What is the best practices to set a component center?

    Thank you in advance!

    Product preview design

    2
    Amal Karim ✅•1,290
    @amalkarimPosted almost 2 years ago

    Hi Effy,

    At some point, like in the design comparison above, the image height is shorter than the description area. I think it's caused by this style: align-items: center; of the article element. Change it to align-items: stretch; will make sure the image height is always the same as the description area. But, at the same time, it could change the image proportion at certain width. To prevent that, add this style to the image:

    img {
    object-fit: cover;
}

    •••

    Your shopping cart is affected by the style that you actually want to implement to the main image (while in desktop view). It is affected by this style:

    @media (min-width: 600px) {
    img {
        content: url(image-product-desktop.jpg);
        ....
    }
}

    My advice is you better change the selector for the main image by adding class to the main image, so that it won't affect the cart image anymore.

    Hope this helps. Happy coding!

    Marked as helpful
    1
  • Eduardo Falarz•80
    @ETUUUSubmitted almost 2 years ago

    Hello there!

    This is my first time in the front-end development world and also my first attempt to create a webpage. For this project, i used pure CSS, like flexbox and grid, and HTML with a little JavaScript for the mobile menu.

    Although i am a novice, every constructive critic is welcome here since it is important for my technical development. P.S: I would be very pleased if some tips were given along with the comments!

    Hope you all like it :)

    Responsive news homepage using CSS Flexbox, Grid and basic JavaScript

    1
    Amal Karim ✅•1,290
    @amalkarimPosted almost 2 years ago

    Hi Eduardo. Great solution!

    I'm just curious as why you chose to use vh unit to define font-size, like for example font-size: 2.6vh;. When I visit your page using mobile view, the text is too small to read. I think it's better to define font-size in html using px, and then define it in other element using rem. For example:

    html {
    font-size: 16px;
}
h1 {
    font-size: 2rem;
}
footer {
    font-size: 0.8rem;
}

    Let me know what you think.

    1
  • Pedro Beltran•70
    @Alex-Beltran97Submitted almost 2 years ago
    1. For me, the eye image hover was difficult because I did know how I can put the image above the NFT image.
    2. I think It is the part of NFT image with its hover.
    3. Yes, I would like any other idea to implement this in better way

    NFT Component challenge

    1
    Amal Karim ✅•1,290
    @amalkarimPosted almost 2 years ago

    Hi Pedro.

    I think there are some ways to implement that. Yours included. However, with your method, the eye image is only 50% opaque. In the design sample, we could see that the eye image has 100% opacity. Only its background that has 50% opacity. To achieve this, we only need to change a few codes. We also add a little bit of transition for opacity.

    Change this:

    .nft-eye {
    ...
    background-color: var(--color-primary);
    ...
}
.nft-eye:hover {
    opacity: .5;
}

    to this:

    .nft-eye {
    ...
    background-color: #0bffff99;
    ...
    transition: opacity 0.3s;
}
.nft-eye:hover {
    opacity: 1;
}

    Feel free to check out my solution if you want to know how I did it.

    Hope this helps. Happy coding!

    0
  • Amal Karim ✅•1,290
    @amalkarimPosted almost 2 years ago

    Hi Haile,

    Congratulations for completing the challenge!

    It's a great solution to be honest. However, allow me to give a little bit of advice to further improve it:

    • For the website logo, give it width only, or height only. No need to set them both. That's to make sure the image is resized proportionally.
    • There's a better way to give meaningful alt text for the image. For example, for the logo, we could write alt="Home of Snap". Please refer to this W3 Schools article about images for better understanding.

    Hope this helps. Happy coding!

    Marked as helpful
    0
  • Abdul Ganuine Alhassan•120
    @Abdul-ganuineSubmitted almost 2 years ago

    This is my first javascript project .The site is unresponsive tho but apart from that any feedback on my project on the html , css and js is welcome.

    Accordian (Non responsive)

    1
    Amal Karim ✅•1,290
    @amalkarimPosted almost 2 years ago

    Hi Abdul,

    Congratulations for completing the challenge!

    Although it's only desktop mode (unresponsive), it's already nice. You could always add responsiveness later.

    I have one thing that I hope could improve your solution. When we open all of the answers under the FAQ, the text will flow outside the box. We could prevent this by adding overflow: auto; to the .faq-card.

    Hope this helps. Happy coding!

    0
  • Amal Karim ✅•1,290
    @amalkarimPosted almost 2 years ago

    Hi Phuong,

    Congratulations for completing the challenge!

    It's an interesting solution, and a great one too! It has a nice structure, and you're using <section> which is semantic in my opinion. If you want to go further and make the page more accessible, I will suggest you to check out the Accessibility report and HTML validation report tabs in this page. They will give you suggestions to improve your page. For example, we could add <main></main> element and wrap everything inside it. We could also add another landmark: <footer></footer> and place the attribution code inside it.

    Hope this helps. Happy coding!

    0
  • Amya•140
    @Mia703Submitted almost 2 years ago

    One thing I found difficult for this project was making the mobile menu's grey/black overlay extend the whole height of the page. I currently have the height as a variable in JS that calculates the window's height, but are there any better solutions for this?

    Responsive News Homepage

    2
    Amal Karim ✅•1,290
    @amalkarimPosted almost 2 years ago

    Hi Amya,

    That's an interesting solution.

    If you want to make the overlay extends the whole height of the page, there's approach other than using javascript. I don't know if this is better solution or not, but I think it's more concise. Just add this to the existing style:

    body {
    ...
    position: relative;
}
.menu-overlay {
    ....
    height: 100%;
}

    Change height of navigation mobile too:

    nav.navigation.mobile {
    ...
    height: 100%;
}

    That's it. Hope this helps. Happy coding!

    Marked as helpful
    1
  • vigneshajith•80
    @vigneshajithSubmitted almost 2 years ago

    Hi Everyone 👋 I have some Questions for you

    • How to make the illustrations positions fixed for all view size ? For this issue i use position absolute and percentage. and lot of media query. I don't know which is the best way to complete this. However I complete this challenge. Pls let me know the best way to do it.

    • How much time it will take to complete this challenge in your guess ?

    -Happy Coding 🎄🎁

    FAQ Card - Dropdown Component Only using HTML and CSS

    1
    Amal Karim ✅•1,290
    @amalkarimPosted almost 2 years ago

    Hi VIGNESHAJITH,

    To make the illustrations positions fixed for all view size, I think it's easier using position: absolute; and transform property. Here's what I will do to make the position of the woman illustration and background pattern fixed and easy to maintain:

    .bg-pattern {
    position: absolute;
    left: -50%;
    top: 50%;
    transform: translateY(-58%);
}
.woman_illu {
    position: absolute;
    left: -10%;
    top: 50%;
    transform: translateY(-50%);
    width: 50%;
}

    I think we could safely remove z-index property because it's not necessary here.

    We could also use the same technique to position the small-box image.

    Please note that the above code is only for desktop view. You will need to adjust them for the mobile view.

    Hope this helps. Happy coding!

    Marked as helpful
    0