@danielemanca1983
Posted
Hi Mohsin,
I am here providing you with as much constructive feedback as I can based on the code you wrote for your solution, it is quite good to be honest, but it takes another set of eyes, to notice any imperfections we can improve on.
I looked at your solution and the dimensions of the component do not closely match the ones from the given design, for instance you should assign the .card component a max-width for larger screens. By adjusting the max-width, the height will auto adjust itself, as it is also off course currently.
In terms of the HTML code you wrote you did pretty well, although I suggest that your .img-wrapper element be switched to a <picture> instead and also I would suggest not to hardcode the height in the <img /> inside of it.
Your CSS code is also good, however I notice that there are some CSS declarations with no code within them, if you do not need them at all, I would suggest to remove those and re commit the code without them.
In general you did pretty good, well done.
Marked as helpful
@Mohsin-93
Posted
Hi Daniele I appreciate your comment and I tried replacing '.img-wrapper' with a <picture> tag but does not size up especially without specifying the height and width. If I do get it to fill out completely there is a few pixels wide space along the lower edge which I just can't get rid of. So currently I have the opacity of <img> set to zero and the image is infact the background of the '.img-wrapper'.
@danielemanca1983
Posted
@Mohsin-93 right, however I would advise you against using images as backgrounds, for accessibility purposes using the <picture> element and using different image sizes within the srcset attribute is best. See this link for having a better idea on how to use it.
