GoodMorrrrning• 20
@GoodMorrrrning
Submitted
Hello,
Dont hesitate to give me some feedback about this rating. Thanks
John Denver
@jbidz
All comments
- John Denver• 290
@jbidz
Posted
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
formand create fiveradio inputsinside 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 - um1chc5• 210
@um1chc5
Submitted
This is my solution for this challenge. I hope you guys like it and have some feedback so I can improve my skills. Have nice day ^^
John Denver• 290@jbidz
Posted
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-areasto 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 - Kent• 40
@kentbuno
Submitted
Any suggestions and recommendation in my code is well appreciated 😊
John Denver• 290@jbidz
Posted
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 - muhammad ehtisham• 310
@muhammad-ehtisham-haider
Submitted
all feedback is welcome thank you
John Denver• 290@jbidz
Posted
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
divand 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 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.
John Denver• 290@jbidz
Posted
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 themainelement 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-directionof 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😊
1- Karamvir10• 20
@Karamvir10
Submitted
I found difficulty in:
-
Adjusting the card size as per the mobile screen or viewport.
-
I was unable to find the font family used in the price.
John Denver• 290@jbidz
Posted
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
mainelement. This will get rid of the warnings above.
If you find this feedback helpful, please consider to marked this comment helpful
0 -
- gambi358• 290
@gambi358
Submitted
I don't know how to set the image in the mobile version to look like the original.
John Denver• 290@jbidz
Posted
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
pictureelement and display different images in different breakpoints usingmediaattribute. 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
mainelement.mainelement 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 - To make the image responsive for every devices you could use
- Nadun Nissanka• 40
@Nadunnissanka
Submitted
All feedback are welcomed. Thank you FM community.
John Denver• 290@jbidz
Posted
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
mainelement
pelement shouldn't be place inside a button try using<span>element
Marked as helpful
1 - To identify the main content of your page, you need to wrap your entire component inside the
- Michael Johnson• 970
@mikej321
Submitted
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!
John Denver• 290@jbidz
Posted
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
