- Is my code too long?
- Have I wrote a code following the best practices? What can I improve?
- Is the responsiveness well made?
- In terms of skills of both CSS and HTML, what can I improve?
- To do the responsiveness I got to use the flex-wrap property in order to move my image to the top. Are any other ways to do this?
Pon Huang
@ponhuangAll comments
- @R4f0soSubmitted over 2 years ago@ponhuangPosted over 2 years ago
Hello Rafael
You’re doing quite well on the project. 😃
There are few parts would suggest change, and it would improve the code.
1.) In order to set the image border-radius in each angle, it will be better to set
overflow: hidden;
to the parent container. Then you won't need to do it every single time, the border keeps the radius.preview-card{ overflow: hidden; // add this line } content-img{ Border-radius: 10px 0 0 10px; // delete this line Background-size: cover; // this could complete show the whole image without setting any specific unit }
2.) The other is the cart icon didn’t show up in the button. That is cause the src you add one **/ ** before images, delete this / and it can work. Also, give specific width and height to the icon.
<img src="images/icon-cart.svg">
3.) If you only want to have an empty container to set image in CSS, then better use <div> instead of <img> element.
When use empty <div> for image, we can set role=“img” and aria-label=“(brief description of the image)”.
<div class=“img” role=“img” aria-label=“perfume picture”>
Hope this help :) Have a nice day
Marked as helpful0 - @SergioZF09Submitted over 2 years ago
1.- It was a bit confusing the sizes to desktop I had to use, because the size of my monitor is different from other monitors. However, I guess I did it well.
2.- I don't know if it would've been better to use another div or something different to group the three sections that I did.
3.- Not now. But, if you see something wrong in the code, you can write a different option or different way to do.
Any feedback is welcomed!!
@ponhuangPosted over 2 years agoHello Sergio
The desktop size looks quite ok in my monitor, so I guess there's nothing wrong.
There is one technique that I learned from the course about writing dry code. By setting a reusable class to the elements that you want to apply. For example, in your code, you set 3 times same height and width to each section.
In the case, we can set one class and give the same properties they all need, then we only need to write it once.
I quite like this code writing concept, so share with you, maybe next time you can try it and see how it works.
Have a nice day :)
Marked as helpful1