Skip to content
OverviewSolutions
13
Comments
9
John Denver
@jbidz

All comments

  • John Denver•290
    @jbidzPosted almost 2 years ago

    Hello there, great work on this challenge

    Here's some of my suggestions you might need to consider

    The best approach for this challenge is to make a form and create five radio inputs inside that form and a button for submit.

    You need to make sure a user picks a rating before they click on submit. You can do this in the javascript by creating a listener for the form.

    form.onsubmit = function(e) {
if(thereIsrating) {
return true;
}
return false;
}

    Additionally, to identify the main content of your page wrap your entire component inside the main.

    If you find this comment helpful, please consider to mark this comment as helpful😊

    2
  • John Denver•290
    @jbidzPosted almost 2 years ago

    Hi there👋

    Upon checking your solution, I noticed that it isn't mobile friendly and when I checked on your CSS code everything is fine and you're using grid very well. To make your layout responsive to mobile devices, add media query and within that specific breakpoint change your grid-template-areas to column:

    @media (max-width: 640px) {
     .container {
          grid-template-areas: "part-1"
                                               "part-2"
                                               "part-3"
                                               "part-4"
                                               "part-5"
             ;
         }
}

    Hope this helps you out😊. If you have any questions feel free to reply on this comment, and please consider to mark this comment as helpful. This would make my day😊.

    Marked as helpful
    0
  • John Denver•290
    @jbidzPosted almost 2 years ago

    Hi Kent, good job completing this challenge

    I'm really amaze by the outcome of your solution. It's really close to the original design, but I think the shadow is too hard. Try increasing the blur and lowering the opacity.

     .component {
    box-shadow: 0 0  30px rgba(0, 0, 0, 0.1);
}

    Happy Coding😊

    Marked as helpful
    0
  • John Denver•290
    @jbidzPosted almost 2 years ago

    Hi there

    I noticed that you're using absolute height to your component which causes the add to cart button to overflow. You can omit the height property and let the content dictate it's height.

    Also, to make your component adapt to any screen sizes wrap it inside a div and add necessary styles to center it:

    div {
     max-width: 600px;
     margin: 0 auto;
     display: flex;
     flex-direction: column;
     align-item: center;
     justify-content: center;
}

    That's it. Congratulations on making a step on this challenge. If you have any questions feel free to reply on this comment, and if you think this helps you a little please consider to marked this comment as helpful . Happy coding!!!

    0
  • Amy•130
    @aepostenSubmitted almost 2 years ago

    I struggled a bit getting the images to be the correct size and ended up using min and max width and height properties. Is there a better way to go about this?

    I'm new to responsive design and would appreciate any suggestions to make my code more dry and semantic.

    Responsive Product Card Page Using Flexbox and Mobile First Design

    2
    John Denver•290
    @jbidzPosted almost 2 years ago

    Hello Amy,

    To make your layout simpler you can make a <div class="container"> and wrap your entire component in that div. Also, make sure to wrap that container inside the main element and remove all the styling for that main.

    <main>
    <div class="container">
      <div class="your-component"></div>
    </div>
</main>

    To center you component, add this style to the container

    .container {
    max-width: 600px;
    min-height: 100vh;
    margin: 0 auto;
    display: flex;
    flex-direction: column;
   align-items: center;
    justify-content: center;
}

    Make it a display of flex and flex-direction to column to center your component in the screen.

    Now, all you need to do is to style your component. The easiest way to achieve this is using flexbox and grid. I recommend flexbox for this case.

       .component {
       display: flex;
    }
   .hero-img {
      flex: 1;
   }
   .product-desc {
      flex: 1;
}

    Now, to make it mobile responsive use media query to set the flex-direction of a component to column.

    @media (max-width: 600px) {
       .component {
          flex-direction: column;
       }
}

    And that's it. Congratulations on making a step on this challenge. If you think this helps you a little. Please consider to marked this comment as helpful😊.

    If you have any questions feel free to reply on this comment😊

    1
  • John Denver•290
    @jbidzPosted almost 2 years ago

    Hello there, good job completing this challenge

    Here are my suggestions you might need to consider

    • To make your component size or width responsive you can use relative units like vh, %, vmin, etc.

    • Fonts and images should be included in the starter files of this challenge. Look for assets folder, there you can find fonts and images used for this challenge. To use the font, all you need to do is to import it to your css.

    @font-face {
       font-family: 'font-name';
       src: url('path/to/your/font/file.ttf');
}
.price__text {
      font-family: 'font-name';
}
    • Additionally, to improve accessibility and to indicate the main content of the website you could wrap your entire component inside the main element. This will get rid of the warnings above.

    If you find this feedback helpful, please consider to marked this comment helpful

    0
  • John Denver•290
    @jbidzPosted almost 2 years ago

    Hi there, good job on taking this challenge

    Here are my suggestions that you might need to consider

    • To make the image responsive for every devices you could use picture element and display different images in different breakpoints using media attribute. The code will look like this:
    <picture>
        <source srcset="image-mobile.jpg" media="(max-width: 600px)"/>
        <img src="image-desktop.jpg" alt="" />
</picture>
    • To identify the main content of the page you want to wrap your entire component inside the main element. main element helps search engines and other web technologies understand the most important content on a page, which can affect how the page is indexed and displayed in search results.

    Refer MDN main element

    If you found this feedback helpful, please consider to mark this as helpful.

    2
  • John Denver•290
    @jbidzPosted almost 2 years ago

    Hello Nadun, here are some suggestions to help improve your code

    • To identify the main content of your page, you need to wrap your entire component inside the main element

    MDN main element

    • p element shouldn't be place inside a button try using <span> element
    Marked as helpful
    1
  • Michael Johnson•970
    @mikej321Submitted almost 2 years ago

    Thanks for viewing my code!

    This is my first project and I really struggled with many aspects of it.

    I'm wondering if my code is up to snuff for this project. I feel like I have a lot of redundant code in my text. Are there any better ways to complete this project? My biggest issue that I had was that I wanted to add a transition effect that would fade in my .thanks container but I couldn't get it to work in the long run. Most of the ways that I found to do it in Vanilla JS, I couldn't wrap my head around and it's easier with JQuery but I don't have any experience with it yet. Thank you all for reading!

    Interactive Rating Component Vanilla JS

    2
    John Denver•290
    @jbidzPosted almost 2 years ago

    Hello Michael, you did a great job on taking this challenge, and I really love the outcome of your design

    I noticed in your design that the submit button always bring me to the thankyou page even if I didn't choose a rating. You can fix this issue by adding condition to your submit function like this:

    let value;

submit.addEventListener('click', function(i) {
  if (value == undefined) return; // or you can add an alert to tell the user to choose a rate first
  mainForm.remove()
  
  attribution.remove();
  
  selectedValue.innerText = value;
  
  thanks.style.display = 'flex';
  document.body.append(attribution);
  
})
    Marked as helpful
    0