@uzowaySubmitted over 3 years ago
I'd be incredibly grateful if the community can give me feedback on the responsiveness of the page and also on the HTML markup after review of my code.
TrakaMeitene
@TrakaMeiteneAll comments
- @TrakaMeitenePosted over 3 years ago
At the second section starting with egg, make the image as - background image, background-size:cover and no repeat( to all images in the containers), so it fits the container. ON my screen (bigger than 1440px) it is smaller than the view width.
You've done a great work, keep coding :)
1 - @shivam5750Submitted over 3 years ago
Any further improvement. Please do feedback
@TrakaMeitenePosted over 3 years agoAs I noticed, at the footer you've made list elements on hover, so it makes a white margin. Before the image place an anchor <a> , href="#", and then at css .social li a:hover . It should work better.
Very fast working. Keep doing :)
0 - @JulianIfesiokwuSubmitted over 3 years ago
Any help is welcome. I learnt alot on this project especially background position, background size and multiple backgrounds.
@TrakaMeitenePosted over 3 years agoYou should see the hrefs to the images. Background does not work. The buttons on mobile width: 414px spaces around. I suggest you to use just one button, arrow images inside of the button, and then you should position images inside of it. Here you can see how I've done it in my solution : https://trakameitene.github.io/Slider/
After all, nice job :) You have made a lot of effort on JS . Keep doing !
1 - @c0dz-wqSubmitted over 3 years ago
I had some issues with positioning background-image for Desktop layout. If you have any advice on that or anything else, I would appreciate it.
@TrakaMeitenePosted over 3 years agoIt will help with your background image positioning.
background-repeat: no-repeat; background-size: cover;
I'm new to Js. Maybe you had a good resource to learn and could share ?
0 - @monicamclaughlanSubmitted over 3 years ago
Any and all feedback is welcome, getting back into the process.
@TrakaMeitenePosted over 3 years agoAt the *(..) box-sizing:border-box; will be correct. I would rather use border box only *{ box-sizing:border-box;} cause margins affect the design. div. container height is too much. I would write height :100vh; But I'm not sure if it is the best practice. As the button tag has a border default, I would suggest to take it off : border:none; I would use @media so you could define margins for mobile view.
Clean code, it was easy to understand your work :)
1
