@tired-herb
Submitted
Hello, everyone! Could you help me with my icons, please? I don't really like them.
@eddybpro
@tired-herb
Submitted
Hello, everyone! Could you help me with my icons, please? I don't really like them.
@eddybpro
Posted
Hi, @tired-herb👏
Congratulations on finishing this challenge, I hope it was fun🤣
For the icons issue here is a way to fix it:
<address>
<div class="icon-box">
<i class="your-icon"></i>
</div>
<div class="icon-box">
<i class="your-icon"></i>
</div>
<div class="icon-box">
<i class="your-icon"></i>
</div>
</address>
address{
display: flex;
align-items: center;
justify-contents: center;
gap: 1rem;
}
.icon-box{
width:2rem;
height:2rem;
border: 1px solid white;
border-radius:50%;
display: grid;
place-items: center;
}
That's it Happy Coding
@moritzrose
Submitted
Please let me know anything, you think that can be optimized to make it more responsive or better in general, still very new to this!
@eddybpro
Posted
Hi, moritzrose
Congratulation on finishing this challenge, I hope it was fun.
For the code, You did a nice job but still need some improvements mostly in CSS
:
@media only screen and (max-width: 375px){
.container {
/*remove ''margin-left'' and ''margin right'' properties*/
}
.container div{
/*change the "padding" values to (25px=1.56rem)*/
/*add "padding-bottom" property*/
padding-bottom:1.56rem;
/*remove the "height" and "position" properties*/
}
button{
/*remove the "position" property*/
}
}
@media only screen and (min-width: 376px) and (max-width: 1440px){
.container {
/* remove the "height" property */
/* remove this : align-content: center; */
/*add this */
justify-content: center;
}
.container div{
/*update this*/
padding:1.56rem;
}
button{
/*remove "position" property*/
}
}
Tips
Remember in most cases you don't have to use the height
property in containers like div
or main
..., instead you can use min-height
, Which makes your site displays well in small screen sizes.
Use relative units rem em
instead of absolute units px
.
Make use of HTML
semantic tags: header
, main
, footer
.
I recommend you start using classes and, or pseudo-elements: example:
button
you can use a class like that: <button class="btn"><button>
.container > div{}
Using classes and pseudo-elements makes your styles display faster.
That's it HAPPY CODING
@Joutee
Submitted
I'm not sure about the media section and if it should be used or if it could be done without it. I'm also not sure if i used right responsive units. I tried to use more semantic elements than just div, but I don't really know where to use which. I appreciate any advice.
@eddybpro
Posted
Hi, Joutee
Congratulation on finishing this challenge, I hope it was fun.
For the code here are some suggestions that might help improve it:
body{
/*add this line*/
overflow: hidden;
}
//You did not check for the leap years, eg: 2000;
// here is a way to deal with it:
if(year % 400 == 0 || (year % 100 != 0 && year % 4 == 0)){
//leap year
//Update the value of the second month
daysOfMonths[1] = 29;
}else{
daysOfMonths[1] = 28;
}
//You can also add the prefix 0 in front of values in the inputs like this:
inputEle.addEventListener('change', ()=>{
if(inputEle.value.length==1){
let val = inputEle.value;
inputEle.value = '0' + val;
}
})
Tips
For the CSS UNITS
, The em
unit is relative to the font size of its parent element, while the rem
unit is only relative to the root font size of the HTML document. The “r” in rem stands for “root”.
That's it HAPPY CODING
Marked as helpful
@RedDotz20
Submitted
Any suggestions how can I improve my css especially with the min and max width of both result & summary component; I do think there is some better way to do it and make it more responsive.
@eddybpro
Posted
Hi, RedDotz20
Congratulation on finishing this challenge, I hope it was fun.
For the code here are some suggestions that might improve it:
#app, .result-container, .stats {
/*add this line*/
border-radius: 1rem;
}
.result-container {
/*update the width and min-width properties*/
min-width:22rem;
width: 50%;
}
.summary-container {
/*remove max-width property*/
/*update the width property*/
width: 50%;
/*add min-width property*/
min-width: 22rem;
}
that's it HAPPY CODING
Marked as helpful
@RohyllerP
Submitted
@eddybpro
Posted
Hi, RohyllerP
Congratulation on finishing this challenge, I hope it was fun.
Here are some suggestions that might improve the code:
.p-all{
margin: 0;
}
.main{
margin:0;
}
Tips
main
element is a semantic element and should be the container like that:<main>
<!- your code here->
</main>
In general, the HTML
file structure looks like that:
<!DOCTYPE html>
<html lang="en">
<head>
</head>
<body>
<header></header>
<main></main>
<footer></footer>
</body>
</html>
e-g: margin: 0;
rem em
instead of absolute units px
.ALL DONE YOU DID A NICE JOB KEEP GOING
@Blackysynch
Submitted
Hi all, this is my solution to the order summary component challenge.
I managed to use media queries to specify display for small and large screens.
Any and all feedbacks is welcomed...
@eddybpro
Posted
Hi, Blackysynch
Congratulation on finishing this challenge, I hope it was fun.
Here are some suggestions that might improve the code:
@media only screen and (min-width: 376px)
.card {
background-color: white;
/* remove this line: height: 80%; */
/* remove this line: min-width: 30%; */
/*update this*/
max-width: 25rem;
}
.plan{
/*remove this line: justify-content: space-between;*/
/*add this line*/
background-color:/*your color*/
}
Tip
Remember to start using relative values rem em
instead of absolute values px
.
That's it HAPPY CODING
Marked as helpful
@raf0411
Submitted
I'm having problem with the div won't centered using the display: grid and place-items: center.
Your feedback would be appreciated...
@eddybpro
Posted
Hi, raf0411
Congratulation on finishing this challenge, I hope it was fun.
You did a good job, centering the div
you already answered it, you just need to add the min-height
property like this:
main{
min-height: 100vh;
display: grid;
place-items: center;
}
Tha's it, GOOD LUCK IN YOUR JOURNEY
Marked as helpful
@guptaryan73
Submitted
@eddybpro
Posted
Hi, guptaryan73
Congratulation on finishing this challenge, I hope it was fun.
Here are some suggestions that might improve the code:
<button class="mobile__nav-trigger">
<!-It's better to use the 'button' tag instead of the anchor tag in the 'mobile__nav-trigger
and also for 'anchors' with class: 'mb-row', 'left', 'right', 'lightbox__close-btn'->
</button>
/*The button with the class "lightbox__close-btn" not displaying correctly here is a way to fix it:*/
.light-box {
min-height: 100vh;
padding-top: 5.625rem;
}
/*The image with the class 'product__img-principal' should have cursor pointer when we hover over it*/
.product__img-principal:hover{
cursor: pointer;
}
Tip
It's better to use relative values rem em
instead of absolute values px
which helps makes your website render well in small screen sizes.
HAPPY CODING
@Priyanshu-WD
Submitted
Hey folks, I have completed this challenge by using picture element to make this responsive for desktop and as well as mobile.
I am always open to feedback.
Please let me know if anything lacking in my solution.
@eddybpro
Posted
Hi, PriyanshuPrama12
Congratulation on finishing this challenge, I hope it was fun. Here are some suggestions to improve the code:
#product button {
/*update 'width' property/
width: 100%;
/*add 'padding' property*/
padding: .625rem 0;
/*remove 'height' property*/
}
Remember to use relative units rem em
instead of absolute units px
.
That's it, Happy coding
Marked as helpful
@nina1234567896
Submitted
Any feedback about my projector tips to optimize my project will be appreciated =)
@eddybpro
Posted
Hi, nina1234567896
Congratulation on finishing this challenge, I hope it was fun.
Here are some suggestions that might help improve the code:
main{
*/remove min-width property*/
width:95%;
border-radius:0.625rem;
overflow: hidden;
}
@media only screen and (min-width: 769px)
main{
/*remove that line 'width: 21.875rem;'*/
}
p {
/* remove that line 'width: 15rem;' */
}
Remember there are also devices with width < 350px
, so you should update the media query in CSS
.
/*From this*/
@media only screen and (min-width: 350px) and (max-width: 768px){}
/*To this*/
@media only screen and (max-width: 768px){}
For the units, it's better to use relative units rem em
instead of absolute units px
.
That's it, HAPPY CODING
@CocoShesh
Submitted
1.Are there any issues or improvements you'd suggest for better responsiveness? 2.I'd appreciate any feedback on my code, especially regarding best practices and coding conventions. Are there any areas where you think I could have written the code more efficiently or used better practices?
@eddybpro
Posted
Hi, CocoShesh
Congratulation on finishing this challenge, I hope it was fun.
For code here are some suggestions that might help improve the code:
<div class="star-container"> </div>
main{
*/ remove the scrollbar in big screen*/
overflow: hidden;
}
.best-tech{
padding-left:.25rem;
}
.star-container {
width: 6.25rem;
height: 1rem;
display: flex;
justify-content: center;
background: url(./images/icon-star.svg);
background-position: center;
background-repeat: space;
}
Tip
The value of the property font-whight
does not have a unit.
If the value of margin, padding
is 0 you don't have to add a unit.
-Examples:
h1{
font-whight:700;
p{
padding-top:0;
I hope my suggestions were helpful.
Happy coding
Marked as helpful
@Mahmoudamin11
Submitted
Any feedback please and any advice to reduce the code : )
@eddybpro
Posted
Hi, Mahmoudamin11
Congratulation on finishing this challenge, I hope it was fun.
Here are some improvements that might be helpful:
@media (max-width: 430px)
main {
/*I think its better to change the value of 'width' to 95%*/
width: 95%;
}
It's more professional to add classes to your HTML
elements, That makes it easy to identify the element easily in CSS
.
And you may need to do some tasks later with the class in JS
(when the project gets more complex)
<button class='payment-btn'>Proceed to Payment</button>
You may need to add some hover effect to the button:
.payment-btn:hover{
/*some code*/
}
Tip
Use relative values rem em
instead of absolute values px
that help make your website responsive in different screen sizes.
Good luck in your jouney
Marked as helpful