John Denver
@jbidzAll comments
- @GoodMorrrrning@jbidz
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 fiveradio 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😊
- @um1chc5@jbidz
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 - @kentbuno@jbidz
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 - @muhammad-ehtisham-haider@jbidz
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!!!
- P@aeposten@jbidz
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 themain
element and remove all the styling for thatmain
.<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 tocolumn
.@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😊
- @Karamvir10@jbidz
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
-
- @gambi358@jbidz
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 usingmedia
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.
- To make the image responsive for every devices you could use
- @Nadunnissanka@jbidz
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
p
element shouldn't be place inside a button try using<span>
element
Marked as helpful - To identify the main content of your page, you need to wrap your entire component inside the
- @mikej321@jbidz
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