Luke Janssen
@lukejansAll comments
- @mikhael7Submitted almost 2 years ago@lukejansPosted almost 2 years ago
**Hey, Mikhael! I Love the look of thus result but have a helpful tip that I'm sure you'll enjoy to get that content to the center of the page **
HTML:
<body> <div class="wrapper"> ... YOUR CODE ... </div> </body>
CSS:
.wrapper { flex-direction: column; justify-content: center; align-items: center; display: flex; min-height: 100vh; width: 100vw; }
you can view my solution here but please note the class i use in my code is container instead but has the same result as its just a name
If this was helpful I would greatly appreciate if you marked this as helpful. Thanks! Happy Coding, Luke!
Marked as helpful1 - @Lois39Submitted almost 2 years ago@lukejansPosted almost 2 years ago
Hey, Lois! I have a suggestion for responsive design with your images that I think you'll appreciate!
use the picture tag with a source and query to change the image depending on screen size so you get the smaller image when the product page collapses to a single column.
<picture> <source srcset="./images/image-product-mobile.jpg" media="(max-width: 628px)" /> <img src="./images/image-product-desktop.jpg" alt="Channel Perfume" class="image" /> </picture>
you can visit my github repo for reference!
If you found this helpful please consider marking this comment as helpful! Thanks and HAPPY CODING!
Marked as helpful0 - @Aikaykalu17Submitted almost 2 years ago
Feedbacks are welcomed
@lukejansPosted almost 2 years agoHey, Ikechukwu Kalu Amogu! I love the result of your code, I have a few tips to help you get closer to design and clean your code. First off I would love to inform you that you can create a visble line break using the
<hr>
tag. Other then that it looks great and only thing i would do is practice more semantic html. you can look at my solution for reference github.Happy Coding, Luke!
Marked as helpful0 - @Souheib-AlouiSubmitted almost 2 years ago
- I tried my best to make the text align as the design as much as possible so i tried a bunch of values .
- For the life of me i couldn't tell fonts from each other and the picture change from desktop version to mobile depending on viewport doesn't seem to be quite right. Any help or advice would be appreciated , got stuck on this one for a while wanting it to be perfect .
@lukejansPosted almost 2 years agoHey! I'm still looking at the code but one thing I would note is that your stylesheet seems to not be loading in properly
<link rel="stylesheet" href="/style.css">
. one thing you can try to fix this would be to remove the backslash/
. I've also noticed that your repo doesn't have an images folder which is why when you try to pull in an image from the images directory nothing happens `<img src="./images/image-header-mobile.jpg" alt="">.Happy Coding, Luke!
1 - @jcovington16Submitted almost 2 years ago
Is it bad practice to separate majority of the elements on the page within their own div?
What's the best way to keep elements leveled amongst each other?
@lukejansPosted almost 2 years agoI would practice semantic html. like using footer instead of
<div class="attribution">
. for your<div class="main">
I would highly recommend using the main tag instead as it helps for SEO and is meant to signify the main part of the document. as for the other divs i would use the sections tag instead to better describe to the browser what things are. Don't forget that html is built as a lang to describe what things are to the browser, everything is actually just a<div> (block level)
or a<span> (inline)
but with more significant meaning.1 - @thalesbsbSubmitted almost 2 years ago@lukejansPosted almost 2 years ago
Hey, Thales! You got the design down on this one but i have some recommendations for you if you're interested in getting as close to design as possible. There are a couple tips i have for you but the 2 big ones would be responsiveness and following identical design layout.
- Your cards take up all the screen space which is less than ideal. here are some tips to change that. first you can add a wrapper to you page so you can center the component you built to do this you can add a container and or wrapper around all of your code the first level after the body then nest your code inside it and apply this css rule.
.wrapper { display: flex; justify-content: center; flex-direction: column; align-items: center; min-height: 100vh; }
than you can set a width on your left center and right card classes or define it in the above class.
- to make your webpage responsive on small screens you will need to use
@media
queries. this post will get a little lengthy so ill just link my repo for this challenge so you can figure out how to integrate it into your code. 3-coulmn-component.
Marked as helpful0 - @TxreqSubmitted almost 2 years ago@lukejansPosted almost 2 years ago
something to note would be that your border radii are not properly symmetrical which can make your QR Code look a little odd. To fix this you should apply a smaller border radius to the child as the radius will effect the two depending on their size.
here is a discussion on github following this odd but understandable problem link
0 - @TYTAN01Submitted almost 2 years ago@lukejansPosted almost 2 years ago
another thing to note would be that your border radii are not properly symmetrical which can make your QR Code look a little odd. To fix this you should apply a smaller border radius to the child as the radius will effect the two depending on their size.
1 - @TYTAN01Submitted almost 2 years ago@lukejansPosted almost 2 years ago
Looks great!
One thing I've learned that I think you'll thank me for in the near future is to change the way you set your font. You should never be setting your font size in pixels as this is considered bad practice and not as accessible. When setting font size in pixels it does not allow the user to change the font in their browser settings making it difficult for ppl who need bigger font to read.
Instead try:
font-size: 1rem;
which is 16px. You can use this tool to convert rem to pxhopefully this helps and happy coding!
1 - @szentmikSubmitted almost 2 years ago@lukejansPosted almost 2 years ago
Looks great!
One thing I've learned that I think you'll thank me for in the near future is to change the way you set your font. You should never be setting your font size in pixels as this is considered bad practice and not as accessible. When setting font size in pixels it does not allow the user to change the font in their browser settings making it difficult for ppl who need bigger font to read.
Instead try:
font-size: 1rem;
which is 16px. You can use this tool to convert rem to pxhopefully this helps and happy coding!
0 - @sanchezmiltonSubmitted about 2 years ago@lukejansPosted almost 2 years ago
Ouuu this looks very clean and followed the design! I see you got the img to change src depending on page size and im just wondering how you got that to work as i cannot see your css files and only the tailwindcss config file. I would love to hear how you got the images working properly if you want to share. Great work sir!
1