Petra• 10
@PetraU11
Submitted
This is my first so-called project therefore be kind. Could you please check the positioning and the margin/borders/padding etc.? I still feel a way lost :)
Antoine
@Antoine-Flo
All comments
- Antoine• 220
@Antoine-Flo
Posted
Hi Petra 👋 Great job it looks nice 😀 As Yellow-May said, you have to center this card, which isn't easy.
You have few solutions for that :
- display the container as grid or flexbox, then center the content with justify-content and align-items.
- apply
margin : 0 auto;on your element (but you know that) - use absolute positioning then =>
left: 50%;(withtransform: translateX(-50%))
All of this techniques has quirks and conditions and you'll have to learn by experimenting 👍 happy coding !
0 - Onyekwere Precious• 755
@Yellow-May
Submitted
Anything is fine, Thank you.
Antoine• 220@Antoine-Flo
Posted
Hi Onyekwere Precious 👋
Good job it looks really nice 👍 but why did you use a min-width on your body 🤔 try to remove it and your layout will response nicely when you reduce the screen size.
Apart from that, it looks really nice, keep coding 😀
0 @hassanahmadp
Submitted
Please give me feedback
Antoine• 220@Antoine-Flo
Posted
Hi Muhammad ! 👋
Great job, it looks really nice ! If I had a suggestion, try to avoid using a fix height for the grid raw for the mobile layout. From your breakpoint at 940px, because of the fixed height there is a huge empty space in the Daniel Clifford card.
And I'm not sure you have to specify
grid-column: auto; grid-row: auto;at the end of your css, it should be the default behavior (but maybe I'm mistaken).Anyway nice job again 👍
1- TheMihir• 85
@TheMihirSensei
Submitted
Feedback plz
Antoine• 220@Antoine-Flo
Posted
Hi TheMihir 👋 good job, your design comparison looks nice.
Anyway, your layout isn't responsive right now, you have to find ways to fix your design so it doesn't move everywhere when the screen size changes.
- Float properties are nice but you should also use flexbox, to help you organize your elements with more control.
- Apply a
max-widthto your container so it doesn't stretch on big screens - I wouldn't use the
bg-pattern-desktopas a background-image, but as animgin your html, like the woman (with absolute positioning). - Notice that the 'woman' picture changes when in mobile size. You don't have to use the box anymore but the other illustration in you images folder.
Keep coding, layouts are challenging at first but it gets easier 👍
2 - Yemisrach• 1,430
@Yemisrach15
Submitted
Any kind of feedback would be appreciated!
Antoine• 220@Antoine-Flo
Posted
Hi Yemisrach15 👋 Great job, the design comparison is like pixel perfect 👌 it's impressive.
If I had a suggestion, you should apply a
max-widthto your div wrapper, so your layout doesn't stretch too much on big screen.Apart from that, your CSS style sheet is way cleaner than mine 😅 Good luck and happy coding !
2 - Ashwin Sasikumar• 235
@ashwin366
Submitted
Any valuable feedback is appreciated..
Antoine• 220@Antoine-Flo
Posted
Hi Ashwin 🖐 nice job !
A suggestion :
- You should wrap all your layout inside a div 'container', and apply a max-width so your design doesn't stretch in a weird way on larger screen.
It looks good, keep coding 👍
0 - PBianca Procopio• 150
@biancahpp
Submitted
I'm satisfied with how it turned out but not satisfied with the orange box. I had to put it to position: absolute in order to make it work. I would really appreciate some suggestions on how to improve that (and everything else too).
I'm still working on the mobile version so disregard that.
=)
Antoine• 220@Antoine-Flo
Posted
Hi Bianca ! 👋 Yeah this box is tricky, it took me a while to figure it out as well.
My suggestion :
- Right now each time we open a question it stays open, even if we open a new one. Eventually if you open all the questions it extend out of the card. Try to find a way to close the previous question each time a new one is opened (I have the same issue in my design 😅).
Great job 👍 and good luck !
0
