@the-coding-beekeeper
@SonEfAdam
Submitted
I realised that my CSS, especially flexbox and css grid, is not really that good. How can I use them in a better way? I always find myself guessing random pixel numbers for the font sizes or grid sizes.
The bottom of my image element is not really touching the box that I made. I am not quite sure how to fix this.
I am still very new to HTML and CSS. I hope you guys can provide good feedback for me. Thank you!
@the-coding-beekeeper
Posted
Hi SonEfAdam!
Congratulations for finishing this project! As you are new to HTML and CSS there are revealing some issues. Let me offer you some suggestions to improve the code:
Give your body a width:
body {
width: 100vw;
Size your grid-container correctly (overflow: hidden should be needless):
.block {
width: 42%;
grid-template-columns: 50% 50%;
/* overflow:hidden; */
Give your image a width instead of a height and set it`s display to block, so that it fits into the grid-container:
.dp-img {
/* height: 600px; */
width: 100%;
display: block;
}
Then reduce the font-sizes and delete all the hard-coded px-margins. Replace the margins where needed with relative values like rem or em.
in your html the button could be coded easier like following:
<button class="button">
<img src="./images/icon-cart.svg">Add to Cart
</button>
The .price element could be solved easier as following:
.price {
display: flex;
align-items: center;
/* height: 100px; */
}
These suggestions will not lead to a perfect solution, it is only intended to give you some food for thought.
Happy coding and greetings from the-coding-beekeeper.
Marked as helpful
@rezis-work
Submitted
responsive, grids, clean code
@the-coding-beekeeper
Posted
Hi rezis-work!
Congratulations for finishing this project! Your solution is very close to the given template, really nice.
I have a suggestion to improve your code:
To improve the responsiveness of ypour code, replace the hard-coded, absolute px-values (width, height, padding, margin) with relative values like rem or em.
Happy coding and greetings! The-coding-beekeeper
Marked as helpful
@AnAvilableUsername
Submitted
i think i screwed up the mobile layout
@the-coding-beekeeper
Posted
Hi AnAvilableUsername!
Congratulations for finishing this project! Your solution is quite close to the given template, very nice shot.
I have some suggestions to improve your code: It seems that you use the <h> tags for styling-purposes, but this is not good practice. The <h> tags should represent the structure of the page. Imagine the structure of your <h> tags like the summary of a book (one title <h1>, chapters <h2>, ...).
To make your code better readable and better structured, try to replace some <div> tags with semantic html like <main>, <section>, ...etc.
Replace the hard-coded, absolute px-values of your divs with relative values like rem or em. That is very important to make your code responsive.
To make your code responsive, you should add a mediaquery. To write down all the necessary changes in your code would go too far. But as a guideline: most of the time the mobile-first-approach is the better way.
I hope my feedback helps you to a little.
Happy coding and greetings! The-coding-beekeeper
@gabymarchingo
Submitted
background shows on my editor but doesn't on here as GitHub doesn't like leading /'s it's also centered properly on my editor/browser
@the-coding-beekeeper
Posted
Hello gabymarchingo!
Congratulation for finishing the project! There are some issues that should be fixed. My suggestion to get closer to the design-template:
body {
width: 100vw;
min-height: 100vh;
display: flex;
flex-direction: column;
align-items: center;
justify-content: center;
background-image: url(/images/pattern-background-desktop.svg);
background-repeat: no-repeat;
background-size: contain;
background-color: hsl(225, 100%, 94%);
}
.card {
/* margin-top: 130px;
margin-left: 580px;*/
display: flex;
flex-direction: column;
}
Give the body a width and a height and center the content; Fix the background issues by a working file path and some background properties; delete the hard coded margins at your .card and add flex-direction: column.
I hope that will fix some issues. Happy coding!
Greetings from the-coding-beekeeper
@Zainabnofiu
Submitted
Hey , This is my solution for the challenge. I'll be glad to have your feedback.
@the-coding-beekeeper
Posted
Hi Zainabnofiu!
Congratulations for finishing this project! Your solution is quite close to the template. Only the backgroud is not in the correct shape. To improve this, you have to adjust the background-positions. I suggest code as following:
background: url(./images/bg-pattern-top.svg), url(./images/bg-pattern-bottom.svg);
background-position: -63% -85%, 145% 240%;
background-repeat: no-repeat;
Happy coding!
Marked as helpful
@Josephmungai99
Submitted
How could I have made it even better?
Any tips will be highly appreciated.
@the-coding-beekeeper
Posted
Hi Joseph,
congratulations for finishing the project! I have one suggestion to improve your code: the <center> tag is outdated. To center text or other items try this instead in your css: display: flex; flex-direction: column; align-items: center; text-align: center;
Greetings! the-coding-beekeeper
Marked as helpful
