Hello, guys! Here it is, my Product preview card component solution. I'd love to hear it from you on what I could do better. And, I'd like to know how do you guys do to present different images for desktop and for mobile. 😊
Thanks, right away
Nice work!
Here is how you can toggle between the two images:
<img
class="desktop-img"
src="images/image-product-desktop.jpg"
alt="Product image" />
<img
class="mobile-img"
src="images/image-product-mobile.jpg"
alt="Product image" />
In CSS:
.mobile-img {
display: none; // Completely hides the image
}
(max-width: 500px)
{
.desktop-img {
display: none; // Hides desktop image
}
.mobile- img {
display: block; // Displays mobile image
}
}
A few points you can improve:
a element and img element should have an alt attribute that describes the image and what the link does. This is essential for screenreaders. You can add the alt attribute like this: alt="YOUR TEXT"I hope you find this helpful.
Happy Coding!
Hello Devs, I hope y'all are well, Enjoy your holiday and have a Happy New Year. As usual Comments, Suggestions, and ideas are welcome.
Thanks Have Samuel
Nice work.
Here are some points you can improve:
You should change the background color of the selected rate.
You can add animation on the input and button elements like this:
.number, .button {
transition: all 0.5s;
}
Read more about transitions here https://www.w3schools.com/css/css3_transitions.asp
You can fix it like this:
// Insdie the submitHandler function. Add the following line right after the loob
if (!selectedValue) {
return; // If the user doesn't select a rate
}
Overall, that was a nice job. I hope you find this helpful.
Happy Coding!
Please Rate It.....
Great work and very clean code!
Here are some points you can improve:
alt attribute. This is essential for screenreaders. You can set the alt attribute like this:a="A QR code to FrontendMentor website"
You should replace the div element with the .card class with a main element instead. div elements are generic elements. They don't have a semantic meaning and you should put your main content inside of a main element.
The div elements with the classes of (text, details) are not necessary, you can remove them and add the text class to the h3 element and the details class to the p element
I hope you you find this helpful
Happy Coding!
This challenge was a good learning experience for me overall.
If there is another efficient way, I would like to know if I can write a for loop of some sort to make is less tedious to write JS event listeners to Rating Buttons 1-5.
Any feedback is welcome. I have thick skin so please don't shy away from giving constructive criticism.
Nice work!
Here are how I did the JS event listeners:
const ratingList = document.querySelector(".rating-group"");
// This will choose all the button elements and give you a Node List
const ratingBtns = document.querySelectorAll(".rating-btn");
rating-value class:const selectedRate = document.querySelector(".rating-value");
const form = doqument.getElementById('form');
// Select rating handler
// 1. Add event listener to common parent element - (ratingList)
ratingList.addEventListener("click", function (e) {
e.preventDefault();
// 2. Determine what element caused the event - Very important step
const clicked = e.target.closest(".rating-btn");
// Gaurd Clause - return if no button is clicked
if (!clicked) {
return;
}
// console.log(clicked);
// Remove active class from all buttons
ratingBtns.forEach(btn => btn.classList.remove("active"));
// Add active class to clicked button
clicked.classList.add("active");
// Assign rate to clicked button value
selectedRate.textContent = ` ${clicked.textContent}`;
});
// Submit form
form.addEventListener("submit", function (e) {
e.preventDefault();
// Make sure a rating is selected
if (!selectedRate.textContent) {
console.log(`You must Choose a rate!`);
return;
}
console.log("Submitted");
// Hide rating container
ratingContainer.classList.add("hidden");
// Display thank container
thankContainer.classList.remove("hidden");
});
The rating part is possible because of something called event propagation. You can read more about that here:
[Event Propagation] (https://www.tutorialrepublic.com/javascript-tutorial/javascript-event-propagation.php)
Overall that was a great job on the UI, you just need to refactor the JS code.
If you find this helpful, click on the Mark as helpful button.
Happy Coding
Any tips and comments help. Also I feel like I am positioning things wrong. For example in the section for the Annual plan/music icon area I feel like there is a easier way to accomplish positioning everything if someone can check the code out and let me know that would be great.
thanks.
Nice work!
A few points to improve:
In HTML:
a element should have an href attribute. If the link doesn't go anywhere, assign it to # href="#" <div class="cancel-order">
<a href="" class="cancel">Cancel Order</a>
</div>
I also saw that you left an image without an alt attribute. alt attributes are essential for screenreaders. The alt attribute should be descriptive of the image.
You are using div elements where you don't have to, this is a bad practice. Only use div elements when you need to wrap other elements inside one container.
You didn't need the div element with the card class. You can remove it and add the class to the main element.
The div element with classes of content, order, button are not necessary.
In CSS:
Never never use position: absolute and position: relative unless there is no other solution. Flexbox and Grid are much easier and much more powerful.
You're using position: absolute and position: relative where you don't have to. You can instead use flexbox to align and center the elements.
Flexbox is more than enough in one-dimensional layouts, so you didn't need to use Grid in this challenge. Grid is awesome of course, but it was not necessary.
You declared the universal selector * twice. You can add all the properties once like that:
* {
box-sizing: border-box;
margin: 0;
padding: 0; // It's a good practice to set the `padding` to 0
}
I know that was too much to take in my friend. You did a great job and the learning never stops. If you need any more help, feel free to tell me in the reply.
If you found this helpful, click the 'Mark as helpful' button.
Happy Coding
I had the help of a tutor to do the bit where you select the rating and it goes into the thank you card, any suggestion on where I can get documentation to look into it further-or any other way that could have been tackled is appreciated!!!
Nice Work!
You asked for a way to implement the select rate part. Here is how I did it:
id or a class like you did, you gave it an id='rate'const rate = doqument.getElementById('rate')
const btns = doqument.querySelectorAll('.btn')
btn-listdocument.querySelector('.btn-list').addEventListener('click', function(e)) {
// Determine what element caused the event - what button was clicked
const clicked = e.target.closest(".btn");
// return if no button is clicked - if the user clicks on the list button but NOT on a button
if (!clicked) {
return
}
// Assign the value of the clicked button to the span element
rate.textContent = clicked.textContent // No space
// You can add space before the number by using bracket notations like this:
rate.textContent = ` ${clicked.textContent}` // There is a space
}
}
// return if NO rating was chosen - No button is clicked
// If no rating was selected, then the value will be null
if (!rate.textContent) {
return
}
The UI is very nice, but you must improve the semantic HTML.
Use form instead of ul
If you need more help on the JavaScript part, feel free to reply and I'll be glad to help.
Happy coding :)