Looking to hire developers?
@PipouwPieuw
@AnaLuisaFav
Submitted
To be honest, I don't know if it's right. I didn't know if each square on the page was supposed to be seen in the full size of the screen (mobile), but when in doubt, I did it that way. I thought it would look better.
Any suggestions are welcome ♥
@PipouwPieuw
Posted
You did well, I think it looks good on desktop and mobile!
That being said, there's an horizontal scroll on desktop. The section linha-5-imgs seems to be at fault.
In the footer, I guess social media icons need to be linked to their respective social media page.
Your interactive elements (links and buttons mostly) would look even better if you put hover effects on them, like you did on the Contact button. You may also set a transition so the effect would be more pleasing to the eye :)
Well done overall!
Marked as helpful
@leonpahole
Submitted
My goal was to solve the problem as fast as possible, sacrificing small design discrepancies. I used Tailwind, which is not usual for me - I typically use Sass modules.
@PipouwPieuw
Posted
Your page looks really neat!
I was wondering about the loading="lazy" error in the HTML validation report. I got the same errors when I submitted my solution so I just removed the loading attributes from all my images. Does this mean the loading attribute should not be used or is the report tool simply not up to date ? Seems a bit odd.
@atmahana
Submitted
I always found it difficult to center element inside a div.
display: flex;
align-items: center;
justify-content: center;
Sometimes this code block would work. But sometimes it fails to center any element. In this particular case, when centering the card container in the main body. I did put the property block above in my parent container but it doesn't work. Did some research and asking for help from other friend, and come up with this solution
body {
display: grid;
place-content: center;
min-height: 100vh;
}
What is the best practice for centering an element inside a div?
In which situation I should use the first code block and the second code block in my code?
@PipouwPieuw
Posted
Hello and well done completing this challenge!
To answer your question, in this case you can use either flexbox or grid. I don't think there is any good practice that makes one of these better than the other.
Flexbox didn't work in your example because you also need to make sure that your element's height is at least equal to the window's height, as you did in your grid example. Adding min-height: 100vh should work. You would also need to set flex-direction: column in this case because of the .attribution element being a sibling of .card-container:
body {
display: flex;
align-items: center;
justify-content: center;
flex-direction: column;
min-height: 100vh;
}
Hope this helps. Keep coding :)
Marked as helpful
@arjuyyy
Submitted
To be honest, I easily build the markup and was able to quickly imitate the provided desktop design. What I found hard is the mobile design since this must be responsive based on the width of the screen. At first, I'm quite hesitant to manually change the width and height of the "image-desktop-product.jpg" (which is the perfume) since I might possibly ruin the image. But when I was about to upload all the files in my repository, it turns out that there's the "image-mobile-product.jpg" inside the images folder!!! The heck lmao, this is just a reminder to be mindful of the provided resources so that we can all save more time lol.
The ability to manipulate and use display properties such as FLEX, BLOCK, HIDDEN, and of course MEDIA QUERY really helped me in this challenge. Big thanks to the StackOverflow community for providing me with the answer on "How to make an element's background color a little darker using CSS".
@PipouwPieuw
Posted
Hello and well done completing this challenge, your component look good!
I have a few suggestions to further improve it:
alt attribute shouldn't just repeat the name of the product. Here you should really describe the image as you would describe it to someone that can't see it.Apart from these minor matters, you did a really good job! Keep going :)
@ChrisLee-04-20
Submitted
In fact, I find some problems when I style the background image of the product. The size of the image is kind of weird. Therefore, my responsive design may not be good at this moment.
If you can provide some advice on my responsive design and styling background image, I would be appreciated it.
@PipouwPieuw
Posted
Hello and well done completing this challenge!
This image is not purely decorative and is important to the page content, therefore it shouldn't be displayed as a background. I would use an <img> here, or even better a <picture>.
This may even fix your sizing issues. :)
Also you put a <h2> right before the <h1> which is semantically incorrect. You could wrap the word Perfume inside a <p> or maybe a <span> instead.
Keep going, happy coding!
Marked as helpful
@adityaphasu
Submitted
Hey Everyone! This is my solution to the Sunnyside landing page 🙂
I chose to take on this challenge because of the landing page's excellent design ✨
Questions I have:
Any suggestions on how to make the code better would be greatly appreciated.😊
@PipouwPieuw
Posted
Hello! Well done completing this challenge, your page looks really neat! 🎉
To answer your question, I may be wrong but I don't think hover effects have any negative impact on performance. They are good for the user experience though, so ideally every interactive element should have one :)
Keep coding! 🧑💻
@azammustafa66
Submitted
HELP!!
how to add border-radius and hover to main image
@PipouwPieuw
Posted
Hello, well done completing this challenge!
The border-radius you set doesn't show because you gave a padding to your image. Remove this padding and instead apply it to the element that contains the image, here .card.
To create a hover effect you could wrap the eye image inside a <div>, give this div a coloured background, center the eye icon inside using flexbox and use absolute positioning to put the div over the image. Then you give it an opacity of 0 which you switch back to 1 when the element is hovered, using div:hover. This should work :)
Hope this help. Dont hesitate if you have further questions. Keep going!
Marked as helpful
@marudever
Submitted
I'm not sure about the HTML and CSS I'm writing. I feel like my code can do less than I do and is it any good if I declare lots of classes in HTML? sometimes it's annoying and the reason for doing it is because I think it's the best even though other people can do it with less class. Can you give me a solution to make my HTML and CSS classes more efficient?
@PipouwPieuw
Posted
Hello and well done completing your first challenge!
To answer your question, I don't see anything wrong with using lots of classes. I'd even say it's best to target a class rather than a tag name like div / h1 / p etc.
Frameworks like Bootstrap and Tailwind heavily rely on classes, same for the BEM methodology, and it's perfectly fine! Classes exist so they can be used this way. :)
Your component looks good both in desktop and mobile mode. There are a few HTML issues you could fix, everything is listed in the reports above and they're quite self explanatory.
Well done and keep going!
@Walkwithrahul
Submitted
This HTML and CSS project I recently completed was a fun. The project involved creating a responsive website that showcased a responsive card element. The main goal of the project was to provide an engaging user experience by presenting high-quality images that were optimized for different screen sizes.
Throughout the project, I enjoyed the process of building and refining the website, and I didn't encounter any major issues. Overall, the project was a great learning experience that allowed me to explore the power and versatility of HTML5 and CSS in creating a responsive and visually appealing website.
@PipouwPieuw
Posted
Hello @Walkwithrahul, well done completing this challenge!
You did a good job otherwise, keep going! :)
@bnmbz
Submitted
Pleas tell me, is this page responsive?
@PipouwPieuw
Posted
Hello 👋
It displays well on my phone so I guess you managed to make it responsive! 🎉 I can't inspect your code further right now though but I may come back later. In the meantime you may want to look at the reports above ⬆️ to improve your HTML structure.
Well done completing your first challenge, keep going!
Marked as helpful
@Luzefiru
Submitted
I'm not sure if I was supposed to hardcode the width & height values of the different sections of the card.
Was my :hover overlay the best practice in the industry?
How do I automatically size the ::after element's background to match the original element with 100% accuracy?
Is it best practice to use background-size: contain for SVG images?
How do I recolor SVGs using CSS without using CSS masks or editing the .svg path directly?
@PipouwPieuw
Posted
Hello @Luzefiru and congratulations for completing this challenge!
To answer your questions :
1. Your hover overlay method is fine! However I would change a few things :
z-index: 1 to the :before.2. To make an absolute element take the full size of its parent, first make the parent's position relative. An absolute element's position is calculated based on its closest relative ancestor. Then you can make the element match its parent's boundaries like so :
.element {
position: absolute;
top: 0;
right: 0;
bottom: 0;
left: 0;
}
3. I wouldn't say it's a bad thing to use background-size: contain, as long as the icon displays correctly. Here again I would have used an <img> tag but as it is a decorative icon, it's fine this way I guess. :)
4. I don't think you can change the colors of a SVG with CSS if you insert them as background image or even with an <img> tag. However you can do so by inserting the raw SVG inside your HTML, then you'd have access to the elements inside. But I really don't know if it's a good thing to do. Another way to do this is by converting your SVGs into a webfont. The icons can then be inserted as characters and you can change their color and size using CSS. This process is a bit complex though. For beginners, I'd say your method is really fine. :)
I hope my comment was helpful. You did a good job, keep going!
Marked as helpful
@AnaLuisaFav
Submitted
feel free for suggestions and corrections ♥️
@PipouwPieuw
Posted
Hello 👋 Well done here, your hover effect looks neat!
I have a few remarks:
<img> tag instead.<h1> on your page. This adds meaning to your page and shows what the main topic is. Here, the <h1> should be Equilibrium #3429. You can of course keep the <a> wrapped inside or around the <h1> :)Keep going 🎉
Marked as helpful
@AnaLuisaFav
Submitted
feel free for corrections and suggestions ♥ :)
@PipouwPieuw
Posted
Hello Ana Luisa! Well done completing this challenge.
height: 100vh by min-height: 100vh on the .home element.Anyway well done here, there is very little to say :)
Marked as helpful
@LoaiEsam37
Submitted
Any suggestions on how I can improve and reduce unnecessary code are welcome!
Thank you. 😊✌️
@PipouwPieuw
Posted
Hello Loai, well done once again !
I noticed a small issue with your Proceed to payment button. Currently only its label is clickable but ideally we should be able to click anywhere on the button to trigger it. This is because you applied the styles to the wrapper element card__accept--button. Instead you should apply them to the <a> inside.
Otherwise your component looks good! Well done :)
Marked as helpful
@AnaLuisaFav
Submitted
Applying knowledge about bootstrap grid. Love it 🥰
@PipouwPieuw
Posted
Hello @AnaLuisaFav, well done completing this challenge! :)
To go further, here are a few suggestions:
perfume inside a <p> or maybe a <span>.!important in your CSS but this may be due to Bootstrap? You should usually try to avoid using this. The less importants the better :)Overall, you did a really good job! Your page looks good and clean in desktop and mobile sizes :)
Marked as helpful
@LoaiEsam37
Submitted
Any suggestions on how I can improve and reduce unnecessary code are welcome!
Thank you. 😊✌️
@PipouwPieuw
Posted
Hello Loai, good job completing this challenge!
Your page looks good :) Here are a few suggestions to further improve it:
perfume image is a good alt attribute. Think of how you would describe the image to someone that can't see it. For exemple A bottle of Gabielle Essence Eau De Pafum lying between two leaf branches.Overall I think you did well, and the mobile version looks neat too! Good work :)
Marked as helpful
@Rajoz-dev
Submitted
Hi, this is my very first project. I find difficult to do:
I did use pick-up site for colors and fonts but it's not ideal As you can see, the font and color is bit different from the orginal .jpg challenge design. I would love to hear feedback and how could i improve it! Cheers, Daniel.
@PipouwPieuw
Posted
Hello! First of all, good job completing your first challenge!
Here are a few answers and suggestions that might be of interest to you:
#container {
display: flex;
justify-content: center; // Centering horizontally
align-items: center: // Centreing vertically
min-height: 100vh; // To make the container at least as tall as the screen
}
style-guide.md file included in the challenge archive. You should check it out :)<br> tags to skip lines inside a text. This could make for bad text display if the text container were to get narrower.max-width instead of width on your .box to make it responsive.height: auto will make the image's height automatically adapt to its width. Also you can just set its width to 100% instead of a fixed pixel value. This way the image wouldn't overflow if the box were to shrink.Overall you did a good job, keep it up!
Marked as helpful
@r-colleti
Submitted
@PipouwPieuw
Posted
Hey there, well done completing this challenge!
Your component looks neat, I love the loading animation and the tooltip following the hovering from bar to bar. :)
One thing you could improve is highlighting the current day dynamically (currently it's thursday but the wednesday bar is highlighted).
Anyway you did a really good job here 🎉
@Ekene-Azubuko
Submitted
This is my solution to the challenge. Any tip that can improve my code will be appreciated please.
@PipouwPieuw
Posted
Hello there, good job completing this challenge!
I have a few suggestions that could improve your page:
.product instead of .description would fix this..container-fluid {
display: flex;
justify-content: center; // Centering horizontally
align-items: center: // Centreing vertically
min-height: 100vh; // To make the container at least as tall as the screen
}
Overall you did a good job, keep it up! :)
Marked as helpful
@yassee-ward
Submitted
i'll be happy to see your feedbacks
@PipouwPieuw
Posted
Hello Yasser, good job! your page looks good.
A few suggestions to improve your CSS / HTML:
max-width: 320px instead of width: 320px on your card, this would make it more responsive.body {
display: flex;
align-items: center; // Center content vertically
justify-content: center; // Center content horizontally
min-height: 100vh; // To make the body at least as tall as the screen
}
opacity: 90%on the card ?QR code to frontmentor.io.Goop job on this, keep it up!
@MaraVengen
Submitted
This is my solution for this challenge.
I hope I'm getting better at this.
@PipouwPieuw
Posted
This looks clean!
The hover effect on the image doesn't trigger though. Not sure why as I see you included an "overlay" element next to the image.
To center the card on your page, instead of setting a fixed padding to the body you can use :
body {
display: flex;
justify-content: center; // Center content horizontally
align-items: center; // Center content vertically
min-height: 100%; // So the body is at least as tall as the window
}
Also your images are missing an alt attribute. This is important for accessibility. :)
Good job overall, you're getting there !
@pfigz
Submitted
This is my first submission to FE Mentor. I'm a career changer who has mostly focused on backend development, so any tips/tricks/suggestions are welcome. Feedback re: usage of grid/flex and .container/.card sizing is especially appreciated.
My font-size is different than in the style guide, but it was necessary to get the correct look for this implementation.
I did not have a design doc for this, just the reference images. It's not exact, but pretty close all things considered.
@PipouwPieuw
Posted
Hey there,
You did a good job! It looks neat. :)
A few suggestions:
box-sizing: border-box on everything like so :*,
*:before
*:after {
box-sizing: border-box;
}
This makes padding and stuff included in the width / height calculations. You may have noticed that your .card is 280px wide even if you gave it a width of 250px. This is because its padding of 15px is then added to the width. border-box prevents this.
Instead of giving a fixed width: 250px to your card it would be better to give it max-width: 250px. max-width makes elements more responsive as it allows them to shrink if necessary. In this case I also had to add width: 100% so the card would size correctly, I'm not sure why. It may be because you used display: grid.
You can then get rid of the breakpoint of min-width: 300px you used, because the card will never get larger than the screen now.
I also would set min-height: 100vh instead of height: 100vh on the body. This way it would be at least as big as the screen but could get bigger if needed.
HTML landmarks are important. You should include a <main> tag around your content and use headings. For exemple the text Improve your front-end skills by building projects could be wrapped inside a <h1>.
Overall, well done! You did a good job :)
Marked as helpful
@diel-12
Submitted
This is my first project from Frontend Mentor. Feeling excited to show this to you all, and don't hesitate to give me any feedback!
So, I had some hard time making <div> be centred, I coded img width : 100% but I don't know what it means, I just googled it (haven't learnt img width 100% yet lol).
That's all! I'll be waiting for any opinions and feedback! thanks!
@PipouwPieuw
Posted
Hello!
First of all, well done for completing your first challenge. Good job!
The easiest way to center something is to use flexbox. You should definitely learn about it if you're not familiar with it :) Here you would do it like this:
body {
display: flex;
justify-content: center;
align-items: center;
flex-direction: column; // Because there are two elements inside your <body>
min-height: 100vh; // To make the body at least as tall as the screen
}
You would also need to remove the margin from your <div> and <p> which causes weird spacing here. With this, your content should be centered no matter the screen size :)
width: 100% on your image means that it will always take 100% of the horizontal space available inside its parent. This could cause your image to stretch beyond its original size so instead you may want to use max-width: 100%. Also don't forget to set height: auto to your images so they will always keep their ratio, no matter how they grow or shrink.
The alt attribute shouldn't contain the word "image" because it is redundant. It should also be a readable text. A good alt here would be QR code to frontendmentor.io.
You should also take a look at the accessibility report above to improve your HTML.
Overall you did a good job for your first challenge, well done!
Marked as helpful
@MaraVengen
Submitted
Okay...so this is my solution to this challenge.
I had little problems in making it responsive. Still trying to grab the concept of responsivity. Was able to manipulate my way around this one...lol.
Puhlease!!! Any feedback or comment on how to make this solution better will be very much appreciated. Thanks in advance.
@PipouwPieuw
Posted
Hey 👋
I opened your page on my mobile phone and it displayed fine, so I guess you managed to make it responsive 🎉
You set a fixed height on your .row, you may want to remove this. Letting the elements adapt as their content changes is very important.
Also, I may be wrong but I don't think it's necessary to use flex-direction: row here as it is already the default value.
Overall, well done for completing this challenge! 🥳
