@abdo-kotbSubmitted about 3 years ago
Feel free to provide feedback. I will be very happy if you tell me how I can improve.
Kris Pietrzak
@krisp-devAll comments
- @krisp-devPosted about 3 years ago
Overall good job on this challenge! I have recently done this one myself and I found it quite challenging.
I have found 2 minor issues; the svg element in your hero section ::before is not stretching the entire width of the screen and looks cut off at the edges (for example on 1920px screen).
Another is, the headings and paragraphs in your feature section are misaligned due to the svg elements having mixed heights, and adding a margin-bottom to the svg icon itself will cause the misalignment. The way I approached this was to wrap the
imgelement in adivand set the max-height of the div to 88px - which is the height of the security icon which is the biggest one of them all, and then vertically align the icons in their respective divs.Great job though, looks solid!
Marked as helpful1 - @promathieuthirySubmitted over 3 years ago
Hey any feedbacks would be great. I assumed the max height was 800px based on the design.
@krisp-devPosted over 3 years agoOverall looks good but noticed 2 small issues;
-Your bg-desktop.png on widths higher than 1440px is losing its position. I think you should put this on the 'body' rather than your container since your container has a height of 800px. When I did this challenge I set the 'background-position: bottom' and 'background-size: 100vw 50vh'
-The progress bar on mobile seems to be 100% filled up.
Marked as helpful1 - @VernonDodoSubmitted over 3 years ago
This design was not that challenging once I got to know my way around it. What I particularly want to get right, is to fill a component with elements and get its size right.
Another thing, on my solution, the backgrounds are visible on the corners but I couldn't pinpoint the elements that I had to style to make that disappear; would really appreciate any help with that.
@krisp-devPosted over 3 years agoSetting 'overflow: hidden' on your .container class where you declared your border-radius should fix the issue with visible corners.
0 - @ShehanMaduwanthaSubmitted over 3 years ago
I'd like to get feedback about my coding practices etc. There are a lot to improve in my coding.
@krisp-devPosted over 3 years agoThe desktop design looks good overall, it is good practice that you have used relative units throughout your code and the code itself is pretty easy to read for a beginner like myself.
A few minor things I'd touch up on are the font-size (a little bit smaller than the original design) and I'd play around with the filter of the image (contrast and brightness) but I am being really picky here.
Also, I think you forgot to make it mobile responsive.
0
