@adrivasarhelyi
Submitted
In this challenge my struggling point was the main picture's hover status. I couldn't do the exact color/shape/style of the NFT picture. Thanks if you're helping me out!
@SasaVatic
@adrivasarhelyi
Submitted
In this challenge my struggling point was the main picture's hover status. I couldn't do the exact color/shape/style of the NFT picture. Thanks if you're helping me out!
@SasaVatic
Posted
Hi Adri, I just wrote one of the possible solutions for picture hover effect with comments so that it is easier to understand. Example JSFiddle link
Hope it helps.
Marked as helpful
@mo-oe
Submitted
I'll just say one thing don't do lazy coding, if you're gonna build something, build right or leave it. I really spent a lot of time refining this code, maybe more time than the first build. I need help please review the code and tell me what's wrong.
@SasaVatic
Posted
Hi Mostafa Mohi, Your solution is very nice. Two things that I would change as if I was one writing css. I Would not use relative position for .feature-logo. You can position it with padding or margin. I would do something like this to escape overflowing:
.feature-logo {
margin-left: auto; ---> add
display: block; ---> add
/* position: relative; */ ---> delete
/* top: 20%; */ ---> delete
/* left: 75%; */ ---> delete
}
And for div I will exclude fixed height. In this case it is not mandatory. But what if in the future you add some extra text. Text and .feature-logo will overflow their div parent if their content height is larger then parent height.
div {
width: 100%;
max-width: 400px;
/* height: 250px; */ ---> delete
padding: 30px;
border-radius: 5px;
box-shadow: 10px 4px 30px var(--shadow-color);
}
That is my brief summary. If you have any more questions feel free to ask. Good luck Mostafa :)
Marked as helpful
@eghosaclinton
Submitted
I have an Issue.
Anyone knows how to make the background image look similar with the design on desktop?
Thank you in advance
@SasaVatic
Posted
Hi Clinton. You can try with this ---> background-position: bottom 190px right -630px; Play with values in media query to adjust bg position. If you want to find out more here are some resources: https://developer.mozilla.org/en-US/docs/Web/CSS/background-position https://css-tricks.com/almanac/properties/b/background-position/ Hope it helps bye :)
@zoetlam
Submitted
🤩 Welcome, everyone, to my sign-up form project! This project offers a seamless and user-friendly experience for registration. Users can effortlessly input their email addresses, with the added convenience of a spellcheck feature to avoid any pesky typos. 🛠️ Built with • HTML5 • CSS3 • Vanilla Javascript 😁 Thanks I'll be happy to hear any feedback and advice!🤗 Thank you very much!
@SasaVatic
Posted
Hi Thi Lam, you have done nice work. I will give you some hints in improving your solution which you can use on any project you encounter. First thing I noticed is that page refresh when you submit the form. This is default behavior with forms which can be prevented using javascript. Also I excluded some of javascript code that is not necessary.
let emailError = document.querySelector('#email-error');
let emailField = document.querySelector('#email-field');
let form = document.querySelector('form');
// No need for this emailFiled.value property holds value from the input field
// let inputValue = "";
// emailField.addEventListener("input", function(event) {
// inputValue = event.target.value;
// });
// Prevent form submit so that page does not refresh
form.addEventListener('submit', (e) => {
e.preventDefault()
})
function emailCheck(e){
if (!emailField.value.match((/^[A-Za-z\._\-0-9]*[@][A-Za-z]*[\.][a-z]{2,4}$/))){
emailError.innerHTML = "Valid email required";
} else {
emailError.innerHTML = "";
document.querySelector('#sign-up').classList.add('hidden');
document.querySelector('#success-popup').classList.remove('hidden');
// Remove line under since we use emailField.value now
// document.querySelector('span').innerHTML = inputValue;
document.querySelector('span').innerHTML = emailField.value;
}
}
document.querySelector('#dismiss').addEventListener('click', function(){
document.querySelector('#sign-up').classList.remove('hidden');
document.querySelector('#success-popup').classList.add('hidden');
})
Whole submit logic can be written inside form submit event callback function. But if I modify code to much it wont be so clear. Also I have created JSFiddle example of responsive div which uses max-width CSS property. It is much better to use max-width in this case then width because width is more strict and not responsiveness friendly when it is set up using px units. There are some comments in the example so you can more clearly understand why and what. If you want to learn to make more responsive solutions there is Conquering Responsive Layouts course by Kevin Powell. You can find it on Frontend Mentor. It helped me to improve my html and css code. Happy coding Thi Lam 🙂
Marked as helpful
@KingMisach
Submitted
I found it difficult to add the image on the add to cart button and also having the rounded borders. I used the rounded border pixels but the images just did not have rounded borders. I am having fun practicing with front end mentor, any feedback is highly welcome and encouraged i want to get better.
@SasaVatic
Posted
Hi KingMisach, great work you have done here 🙌🏻 and I am happy to improve my own skills by looking into your codebase.
Here are some suggestion for this and future projects:
*, *::before, *::after {
margin: 0;
padding: 0;
box-sizing: border-box;
}
Keep learning and improving you skills. Best wishes on your programming path 😊🙏
Marked as helpful
@CinArb2
Submitted
Hi, I hope you are all doing great.
Here is my order summary card. I tried to do my best :).
While I was doing this exercise I got a problem with padding and margins.
I have read we have to reset the css style. But most of the time I spend it on giving margin and padding again to the elements.
Do we really need to reset those 2 properties? Or what is your best approach with margins and paddings.
Also is the semantic correct in my html?
Any help I will really appreciate it.
Thanks :)
@SasaVatic
Posted
Hi Cindy, your solution is on point 👌. From semantic point of view for this simple layout this is not necessary but I will do something like this:
<section class="hero"> -----> section is a wrapper for one or more containers
<div class="container"> -----> container is a wrapper for hero section rows and determines
width of row element
<div class="row"> -----> row is a wrapper for hero section content, on rows we can set
display options such as flex, grid; row can be parent for multiple card elements
<div class="card"> -----> this is a content element of hero section and parent/wrapper for
all card elements
<div class="card__header"> -----> this is a div for image
</div>
<div class="card__content"> -----> this is a wrapper for card content
<h3 class="card__heading"></h3>
<p class="card__text"></p>
<div class="card__order-box"> -----> this is a wrapper for card payment plan content
<div class="card__icon"></div>
<div class="card__price-box">
<h4 class="card__price-heading"></h4>
<span class="card__price"></span>
</div>
<a href="#" class="card__link"></a>
</div>
<a href="#" class="btn btn--order"></a> -----> buttons are reusable elements for whole
site so in btn class put all styles that are mutual for all buttons and in btn--order
and btn--cancel class put all styles that are unique for that particular button
<a href="#" class="btn btn--cancel"></a>
</div>
</div>
</div>
</div>
</section>
Any feedback would be great
@SasaVatic
Posted
Hi Jenni, I've been playing with your solution a little bit and modified your code along the way and came up with more responsive solution. Your work is good 👏 I just added minor tweaks so you can try to implement it in your code and see what will happen. Keep in mind that in CSS there are many ways to came up with solution. Here are changes I adopted. Good 🤞 and keep 👩💻
body
.container
.leftContainer
.rightContainer
.list
img
comment out max-height: 100%;
comment out min-width: 100%;
add width: 100%; // set img width to 100% of parent element width in this case parent is rightContainer
add height: 100%; // set img height to 100% of parent element height in this case leftContainer
add media query for larger screens
@media screen and (min-width: 768px) {
.container {
flex-direction:row; // on larger screens container should display right and left container horizontally so its row
}
.leftContainer {
text-align: left; // text in left container should be aligned left according to design files
width: 60%; // in this case 60% works fine because padding is pushing content inside 50px so we give content some space to breathe by setting width to 60% }
.list {
flex-direction: row; // on larger screen list items should be displayed horizontally
} }
Marked as helpful
@srijan-srijan
Submitted
I pretty much enjoyed coding this design.
However, when placing the center image, I opted to use negative margins.
Is there a standard way to achieve such scenarios that I am unaware of?
@SasaVatic
Posted
You can set parent div of img to be position: relative and img to be position: absolute then use property left: 50%; on img. It will add gap between parent div left edge and picture left edge exactly 50% of parent div width. Now to finally center img horizontally use transform: translateX(-50%) to move picture to left for half it's width. To position img vertically simply use top: property on img element;
Im still learning. Any feedback is helpful.
@SasaVatic
Posted
I changed your code little bit. Lines that are under comment you don't need. Also I changed in media query min-width: 1320px to min-width: 768px because desktop view should start before. 1320px is to big resolution to start media query for desktop. Padding in .col class was too big so content was overflowing outside of screen in desktop view. Overall good job 👍. Try your code with these values and see why are things happening way they do.
body { /* margin: 50px 20px; / padding-top: 10vh; ------------------------------------> added font-size: 15px; font-family: "Lexend Deca", sans-serif; background: hsl(0, 0%, 95%); line-height: 1.6em; } .row { / display: inline-block; / padding: 20px 10px; color: hsla(0, 0%, 100%, 0.75); } @media only screen and (min-width: 768px) { .row { display: flex; justify-content: center; margin: 0 auto; max-width: 1200px; ------------------------------------> added } .col { / width: 25%; */
/* padding: 100px; */ padding: 30px;------------------------------------> added } .sedans { border-radius: 10px 0 0 10px; } .luxury { border-radius: 0 10px 10px 0; } }
Marked as helpful
@Andrii-Rohov
Submitted
Hello, i am a newbie to JS. So i wrote the most basic Js code off all time)) Anyway, if you have any feedback about my solution please leave a comment.
@SasaVatic
Posted
You could use forEach method to avoid duplicate code like this:
button.forEach((btn, index) => btn.addEventListener('click', function() { box[index].classList.toggle("text-visible"); box[index].classList.toggle("text-hide"); head[index].classList.toggle('bold-text'); }));
so foreach btn in button node list you can access btn element and its index and add click event to it and use index argument to access box and head node list elements with same index as btn which is clicked. I think it will also be better when you use querySelectorAll to use words in plural buttons, boxes, heads for naming your variables. Sorry for bad english, hope it helps.
Marked as helpful