@Dinesh141197
Submitted
Please provide harsh and truthful feedback as much as possible. I want to improve myself significantly.
@IamArshadAli
@Dinesh141197
Submitted
Please provide harsh and truthful feedback as much as possible. I want to improve myself significantly.
@IamArshadAli
Posted
Hello There! 👋
Congratulations on completing this challenge 🎉
I like your spirit of learning from mistakes. I've nothing harsh but some insightful thoughts that might improve your code. 💡
1. Center Your Content
There are many ways to center any content; for now, we'll go with a straightforward grid
:
body {
display: grid;
place-items: center;
}
2. Add some space
Let's give your content some space to breathe using padding
:
.product__content {
padding: 20px; // tweak the value as per your need
...
}
3. Order Your Image
I noticed you are using a desktop-first approach and grid
, you can order your image like this:
.product-image {
order: 2; // as the image is next to the content on desktop mode
...
}
@media (max-width: 649px) {
.product-image{
order: 1; // reset the image position when on mobile
}
}
4. Add an Overlay
The challenge has some purple tint on the image which can be achieved by using mix-blend-mode: overlay;
and ::before
pseudo element.
.product-image {
position: relative; // to make sure the overlay does not overflows
}
.product-image::before {
content: '';
position: absolute;
top: 0;
left: 0;
width: 100%;
height: 100%;
background-color: var(--clr-accent);
z-index: 1; // bring the overlay on top of the image
mix-blend-mode: multiply; // apply the tint effect
}
5. Flex your Stats
Using flex
can be more convenient than grid
for .stat
.stat {
display: flex;
gap: 15px; // specify gap between each item
...
}
// make your flex items vertical on mobile
@media (max-width: 649px) {
.stat {
flex-direction: column;
}
}
Above all your solution looks great! ✨
As I said, you already did an excellent job, but these details will improve it even further. 🚀
I hope this helps 👍
Happy Coding 🤓
Marked as helpful
@Dinesh141197
Submitted
Please provide harsh and truthful feedback as much as possible. I want to improve myself significantly.
@IamArshadAli
Posted
Hello There! 👋
Congratulations on completing this challenge 🎉
I like your spirit of learning from mistakes. I've nothing harsh but some insightful thoughts that might improve your code. 💡
1. Center Your Content
There are many ways to center any content; for now, we'll go with a straightforward grid
:
body {
display: grid;
place-items: center;
}
2. Add some space
Let's give your content some space to breathe using padding
:
.product__content {
padding: 20px; // tweak the value as per your need
...
}
3. Order Your Image
I noticed you are using a desktop-first approach and grid
, you can order your image like this:
.product-image {
order: 2; // as the image is next to the content on desktop mode
...
}
@media (max-width: 649px) {
.product-image{
order: 1; // reset the image position when on mobile
}
}
4. Add an Overlay
The challenge has some purple tint on the image which can be achieved by using mix-blend-mode: overlay;
and ::before
pseudo element.
.product-image {
position: relative; // to make sure the overlay does not overflows
}
.product-image::before {
content: '';
position: absolute;
top: 0;
left: 0;
width: 100%;
height: 100%;
background-color: var(--clr-accent);
z-index: 1; // bring the overlay on top of the image
mix-blend-mode: multiply; // apply the tint effect
}
5. Flex your Stats
Using flex
can be more convenient than grid
for .stat
.stat {
display: flex;
gap: 15px; // specify gap between each item
...
}
// make your flex items vertical on mobile
@media (max-width: 649px) {
.stat {
flex-direction: column;
}
}
Above all your solution looks great! ✨
As I said, you already did an excellent job, but these details will improve it even further. 🚀
I hope this helps 👍
Happy Coding 🤓
@abbigailmerrill
Submitted
Im unsure why my content is overflowing off of the page!
@IamArshadAli
Posted
Hello There! 👋
Congratulations on completing this challenge 🎉
I do have some suggestions that might interest you. 💡
Overflow:
Your content is overflowing because of two reasons:
margin-left
to your .container > *
width
of the .listItem
& .inputContainer
to 100%
Overflowing problems can be fixed by
.container > * {
margin: auto 20px; // set horizontal margin to your .container children
}
.listItem {
width: --webkit-fill-available; // acquires the available space to prevent overflowing
...
}
As there is a list of features, wrap them in a <ul class="list"><li class="listItem">...</li>...</ul>
instead of <span class="listItem">
One more area I found that can be improved in your code is your submit button:
Button:
Instead of explicitly setting width
and height
and then specifying flex
to center the text, you can give padding
to the button and align the text to the center like this:
<button class="button" type="submit">...</button>
.button {
text-align: center:
padding: 10px 25px;
width: 100%; // gets the full width from the parent flex container (.inputContainer)
}
Above all your solution looks great! ✨
As I said, you already did an excellent job, but these details will improve it even further. 🚀
I hope this helps 👍
Happy Coding 🤓
Marked as helpful
@CairoGarb
Submitted
6th Project Done!
I'm having difficulty in positioning the main content in the center of page in smaller screens. I'm currently using a extension that emulates a mobile screen, but in the browser's inspect shows different from the extension. I'll appreciate any tip, thanks!
@IamArshadAli
Posted
Hello There! 👋
Congratulations on completing this challenge 🎉
You can center your main content using flexbox like this:
body {
width: 100vh;
min-height: 100vh;
display: flex;
justify-content: center;
align-items: center;
}
Apart from this, you did an excellent job. ✨
Happy Coding 🤓
Marked as helpful
@HananeEL-2023
Submitted
I've completed my project and deployed it on Vercel, but I'm encountering an issue where the background images aren't displaying. Could you please assist me in identifying and resolving the problem?
@IamArshadAli
Posted
Hello there! 👋
Congratulations on completing this challenge🎉
I also faced the same issue with my projects. Here we know that our react application gets built during deployment to any server(i.e. Vercel, Netlify, Github, etc).
After a lot of trial & error, some research, and some observation, I found that when we build our react applications, the image path gets hashed e.g. ./image/bg-header.svg
gets converted to ./image/bg-header-2f4ag3h.svg
(same applies for each image type)
Tailwind classes (even if you use arbitrary values bg-[url(...)]
or customize your theme) do not allow the image paths to get hashed and it remains the same even after the app is built. Hence, our production version doesn't know about the image path that Tailwind provides.
The solution that I found to deal with this problem is, we need to specify our background image URL from either inline-css
or from CSS file like this:
// method 1
<div className="..." style={{
backgroundImage: `url("${image}")`, // remember to use quotes when using svg as a background image
}}>...</div>
// method 2
.header-bg {
background-image: URL(...); // relative path to your image
}
Please let me know if you find anything insightful on this issue. 💡
I hope this helps. 👍
Happy Coding🤓
Marked as helpful
Hi everyone
I have changed some colours in my project as the text was difficult to read, the contrast was poor
Some changes:
Edit after submitting solution: I notice that in the screenshot comparison, my design looks completely different than the one that I tested in my localhost and live browser. I am not sure why it is behaving in this way. Can somebody help?
Any other feedback is appreciated!
Thank you Priyanka
@IamArshadAli
Posted
Greetings @PriyankaRC16
The challenge's screenshot provided to us are captured at a width of 1440px
for desktop and 375px
for mobile screens. So, it's important to pay special attention to these screen sizes.
You might have a display with a screen resolution of 1920x1080
or something else, which is why the design looks perfect on your machine and breaks in the screenshot, as the screenshot mechanism looks for our design at 1440px
width.
You might want to consider simulating your screen using the developer tools of your browser. Most of the browsers follow the same path to access the developer tools i.e. options > more tools > developer tools
or you can simply use the following key combination Ctrl (or Cmd) + Shift + I
.
Then set the dimensions of the screen to the desired size and you are good to go.
Hope this helps. Thank you🤓
@gosiast
Submitted
UPDATE: thank you for the help, It fixed the issues and answered my questions :)
// I struggled to use the border: radius to the top of the 'card' element. How can I fix that?
I learned that I need to pay attention to the parent-child element.
Can someone help me how to insert a screenshot into the README file?
Thanks for the feedback! :)
@IamArshadAli
Posted
Hello There! 👋
Congratulations on completing this challenge. 🎉
I've some suggestions that might interest you. 💡
use overflow: hidden;
on your .card
, it will take care of the overflowing image, and the specified border-radius
will be applied to the card.
you can add a screenshot into the readme like this ![alt text](./src/images/screenshot.jpg)
or this <img src"./src/images/screenshot.jpg" />
Hope this helps you. 👍
Happy Coding 🤓
Marked as helpful
@JulienLach
Submitted
Hello !
Still got some isues with the navbar dropdown. I dont know ow to but some padding to the menu, so i stick the div to the link to dont let any space between. How to do so ?
@IamArshadAli
Posted
Hello there! 👋
Congratulations on completing this Challenge. 🎉
I do have some suggestions that might interest you. 💡
.dropdown-menu {
...
min-width: 9rem; // dynamic width to your dropdown
position: absolute; // to ensure absolute positioning to a relative parent
margin-top: 2rem; // adds some space between the dropdown and the link
padding: 2rem;
...
}
absolute
but it doesn't have any ancestor, due to which the menu will use the document body, and move along with page scrolling. Fix it by specifying the nearest relative
parent like this:.navbar .container .main-menu ul li {
position: relative;
}
.features-menu {
right: 0;
}
CSS variables
to reduce redundantly writing implicit values. For example::root {
--clr-almost-white: hsl(0, 0%, 98%);
--clr-medium-gray: hsl(0, 0%, 41%);
--clr-almost-black: hsl(0, 0%, 8%);
}
.navbar .main-menu ul a {
color: var(--clr-medium-gray);
...
}
Now you can use these variables anywhere in your CSS file and you'll only need to change the values at one place and it will affect all other places in the code.
Hope this helps to solve your problems. 🚀
Happy Coding! 🤓
@Macury
Submitted
Any ideas on how to make the main image-tint match?
@IamArshadAli
Posted
Hello there! 👋
Congratulations on completing your first challenge on Frontend Mentor. 🎉
As for your question, I suggest the following change:
mix-blend-mode: multiply
on .img
to get the desired tintI hope this resolves your question.
Good Luck with upcoming Challenges. 👍
Happy Coding! 🤓
Marked as helpful
@JAYSONRK
Submitted
Any feedbacks are appreciated.
@IamArshadAli
Posted
Hello there! 👋
Congratulations on completing this challenge. 🎉
I do have some suggestions that might interest you. 💡
organized folder structure
that will make your project flexible.project
-src
--assets
----images
------img.png
--components
----Header.jsx
--App.js
--README.md
React Components
. You can do the same with importing images and svg's .--assets
----images
------index.js
// assets/images/index.js
import gamingGrowth from "./image-gaming-growth.png";
import retroPCs from "./image-retro-pcs.png";
import topLaptops from "./image-top-laptops.png";
export { gamingGrowth, retroPCs, topLaptops }
// components/Extras.jsx
import { gamingGrowth, retroPCs, topLaptops } from "../assets/images"
Instead of using <h1>
for the logo, use logo.svg
that was provided with the starter files.
@media (min-width: 768px) {...}
is usually used to style your elements for medium size devices like tablets, use @media (min-width: 640px) {...}
to style for mobile
Above all your solution is great and I'm inspired by how well you've utilized semantics HTML
to structure your code. ✌️
Good luck with the upcoming challenges. 👍
Hope you found this feedback useful. 🚀
Happy Coding. 🤓
@dselimovic02
Submitted
I would like some feedback on my CSS solutions from some Seniors with few minutes to spare. Thank you :D
@IamArshadAli
Posted
Hello there, 👋
Congratulations on completing this challenge. 🎉
I do have some suggestions to enhance your CSS by a bit. 📈
Change the .container
to have min-height: 100vh
and keep align-items: center;
. This will ensure that your .container
takes full vertical space available on all devices and places your stats card in the center.
Also, add some horizontal and vertical padding to the .container
so that there will be some space left for your stats card to breathe on smaller devices.
Bonus: Please refer to this article on Semantic HTML
which teaches us how to structure our websites using semantics.
Good Luck with upcoming Challenges. 👍
Happy Coding! 🤓
Marked as helpful
@Philip-Droubi
Submitted
Hey everyone 🙋♂️, this is my solution for faq-accordion-card-main
challenge.
Please take a look and give me some feedback. Thanks :)
@IamArshadAli
Posted
Amazing Solution with Clean Code 🚀
Out of curiosity, your clean and accurate solution led me to check the lighthouse report and congratulations it is a clear 98% 🎉
You may need to add a <meta name="description" content="...">
to your <head>
and specify explicit width
and height
to your arrow buttons to get a full score of 100%🔥
Also, after reviewing your code, I learned some tricks about relatively positioning multiple images which I can use to improve my version of this challenge. ✌️
One more thing to add: You can try doing the same challenge without using JS. You just need to implement the same thing using:
<details>
<summary>...</summary>
<p></p>
</details>
Learn more about <details>
and <summary>
elements from here.
Thanks for writing such an amazing solution. 😉
Happy Coding! 🤓
Marked as helpful