Design comparison
Solution retrospective
what would be the best practice to build this ?
Community feedback
- @correlucasPosted over 2 years ago
πΎHello Kounik, congratulations for your new solution!
You did good building this solution with flex, about the best practices somethings should have in mind is to try to write the minimum of code you can, you can do that removing unnecessary divs, using tags like
picture
to save time displaying different images depeing of device without need to use media queries.I think the easiest way to build this challenge is using grid, also the code is more clean, here's how I build mine.To build this component with 2 columns all you need to do is create a main block to hold all the content (you can use
<main>
to wrap), set itswidth
asmax-width: 1000px
(it's the container size) anddisplay: grid
/grid-template-column: 1fr 1fr
(this means that your component will have two columns with 50% of the container width each thats 500px).If you want to go beyond, in Google Chrome theres a tool in dev tools called
lighthouse
that gives you a fulll report saying how is the page performance and how can you improve it.π I hope this helps you and happy coding!
Marked as helpful1@Valhalla-2Posted over 2 years ago@correlucas thanks ,first time heard about the picture tag and chrome lighthouse dev tool ,,, thank you very much .,, i will try that
1 - @elaineleungPosted over 2 years ago
Hi Kounik, well done completing your second challenge, and I think you've done a good job here on the whole. About your question, I would say there are few things you can try:
-
Mobile-first approach: This mean starting your code by building out the mobile version. I see that you have the desktop version first; you can try the mobile first next time, and you may find that you won't need to write as much code for the media query!
-
Whenever possible, try to use responsive width/height properties first instead of fixed widths/heights. Right now when I resize the browser to make it smaller, the desktop view is not responsive (as in, its width is fixed can cannot be changed). You can try using
max-width
instead to make it responsive. Also, I see you usingrem
units, and I think that's great! That's also something I consider good practice ((I use it for nearly everything) as pixels are fixed butrem
can be scaled based on user preference. -
It'd be good to add some normalize/reset CSS rules at the top of the style sheet to override some of the browser's default styles. Here are some you can try:
* { box-sizing: border-box; margin: 0; padding: 0; } img { max-width: 100%; display: block // you can also just add this to the img individually instead of globally }
-
Lastly, in the text area, I see that you got margin set on every element, which can be a bit tedious to change if you do need to change the values. What you can try next time is to give the entire
content
container a padding (try 1 or 1.5rem) and then if you need to, add top or bottom margin for the elements for spacing.
I think you did well on the whole, so keep going! π
Marked as helpful0@Valhalla-2Posted over 2 years ago@elaineleung thank you for you feedback ... i will try that ,,,
1 -
Please log in to post a comment
Log in with GitHubJoin our Discord community
Join thousands of Frontend Mentor community members taking the challenges, sharing resources, helping each other, and chatting about all things front-end!
Join our Discord