SeanTankGarvey / f1-3-c2p1-colmar-academy

1 stars 0 forks source link

wrapping images #5

Closed rimmesbe closed 7 years ago

rimmesbe commented 7 years ago

It's almost always a good idea to wrap an image in a div for sizing it. I would wrap these images with divs, give the images: width: 100%; so they don't leak out of their container, and then size the div as you like. This should fix the vertical stretch of these images.

https://github.com/SeanTankGarvey/f1-3-c2p1-colmar-academy/blob/master/ColmarAcademy-Capstone/index.html#L68

SeanTankGarvey commented 7 years ago

That's what I meant by my comment in the CSS about the height being preferred at 35% making them perfect squares. But okay, I now included them within figure tags with sizing instead. Wait quick question, were you (just) referring to the images on the right side column, or did you mean the main image, because if you notice that that image resizes two different ways, on purpose, because I thought it looked better that way then simply resizing proportionately, so if you resize the screen slowly you'll see the image starts by resizing both vertically and horizontally then at a certain height/width, stops resizing vertically and only resizes horizontally, until the media query kicks in and the image changes height and width again. That was intentional. I was trying to keep the banner section looking more even/proportionate throughout the various sizings. Are you recommending always wrapping all images in divs/containers? Even when they are flex items?

rimmesbe commented 7 years ago

I was seeing the images stretched vertically on my browser.

SeanTankGarvey commented 7 years ago

All of them or which ones?

rimmesbe commented 7 years ago
screen shot 2017-09-14 at 2 26 34 pm
SeanTankGarvey commented 7 years ago

Yeah because I followed the specsheet and set the height to auto instead of 35% as I commented in the CSS. But wrapping them is another option. Initially my images were squared but based on the specsheet, I thought that's what they wanted. :-)

SeanTankGarvey commented 7 years ago

I went back and updated the html and css based on the code review. Would you mind just taking one quick look (at the website itself if nothing else) to see if the main changes you suggested were made?

SeanTankGarvey commented 7 years ago

I did everything you suggested, I think, except for adding transitions.

SeanTankGarvey commented 7 years ago

Thank you again.

rimmesbe commented 7 years ago

No problem, you had a good project. I'll look into your other questions a later time.

On Thu, Sep 14, 2017 at 2:40 PM, Sean Tank Garvey notifications@github.com wrote:

Thank you again.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/SeanTankGarvey/f1-3-c2p1-colmar-academy/issues/5#issuecomment-329572671, or mute the thread https://github.com/notifications/unsubscribe-auth/AJfw_I1cj4ylbbJAkmhbg7_Z81ej8ZkIks5siXMCgaJpZM4PU51i .