Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

<picture> with img-thumbnail class #24176

Closed
t5k6 opened this issue Sep 29, 2017 · 9 comments
Closed

<picture> with img-thumbnail class #24176

t5k6 opened this issue Sep 29, 2017 · 9 comments
Labels

Comments

@t5k6
Copy link

t5k6 commented Sep 29, 2017

I am trying to implement <picture> tag to selectively serve pictures; webp format for Chrome based browsers and jpeg fallback for other browsers. However using Bootstrap's img-thumbnail class is not working properly, it creates handlebar like artifact on border sides.

image

Sample code:

<picture class="img-thumbnail img-fluid">
  <source srcset="/path/to/image.webp" type="image/webp">
  <img class="img-thumbnail img-fluid" src="/path/to/image.jpg" alt="">
</picture>

Bootstrap version: 4 beta

@andresgalante
Copy link
Collaborator

andresgalante commented Sep 30, 2017

I've just tested it. @t5k6 you are right, the way we setup images and figures don't support picture.

This is fixed by display: inline-block; the picture element. A possible solution is to follow what we do with the figure element with the class .figure.

I'll follow the same approach and send a PR.

@andresgalante
Copy link
Collaborator

@mdo @XhmikosR Do you think we should put this on reboot or under figures with a new class of .picture ?

@XhmikosR
Copy link
Member

.figure is being set to inline-block in _images.scss. Not sure if we should add this in reboot or not. It's @mdo's final choice.

@andresgalante
Copy link
Collaborator

@XhmikosR Yeap. Sorry, I meant _image.scss or _reboot.scss

If it goes to reboot we don't document it. if it goes to images.scss then we would have to add a new section to the docs with picture or put it under figure.

@andresgalante
Copy link
Collaborator

Here is a live example of what this issue is describing with the inline-block solution:

https://codepen.io/andresgalante/pen/EwXJdj

@XhmikosR
Copy link
Member

XhmikosR commented Oct 1, 2017

It's @mdo's final choice. Personally I wouldn't add it to reboot.

@mdo
Copy link
Member

mdo commented Oct 2, 2017

Rather than add a generic .picture class, couldn't we encourage folks to use .d-inline-block? And does this issue happen to all <picture>s, or only ones that have our particular thumbnail styles?

@andresgalante
Copy link
Collaborator

@t5k6 After testing it again I realized I was wrong, you don't need to change the display of the picture tag unless you want to add a border around it.

Use img-* classes only on the img tag and not on the picture tag and you'll be ok. Your markup should look like this:

<picture>
  <source srcset="/path/to/image.webp" type="image/webp">
  <img class="img-thumbnail img-fluid" src="/path/to/image.jpg" alt="">
</picture>

The picture will handle where the source of that image comes from.

See an example here: https://codepen.io/andresgalante/pen/EwXJdj

@mdo There is nothing wrong with the way we handle images. Do you you I think it has value to document this on our docs under images?

@mdo
Copy link
Member

mdo commented Oct 3, 2017

@mdo There is nothing wrong with the way we handle images. Do you you I think it has value to document this on our docs under images?

Yeah, good idea—I think it'd be helpful to have a code example and snippet in the Image docs page.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants