Skip to content

Conversation

skaji
Copy link
Contributor

@skaji skaji commented May 5, 2017

Now style attributes for <img> tags are not allowed (cf #1844).

Then <img> can be bigger than the parent element.
See, for example, https://metacpan.org/pod/release/SKAJI/Parallel-Pipes-0.002/lib/Parallel/Pipes.pm

This pull request adds class "img-responsive" to <img> tag in pod so that images are made responsive-friendly.
See also http://getbootstrap.com/css/#images

@coveralls
Copy link

coveralls commented May 5, 2017

Coverage Status

Coverage remained the same at 69.5% when pulling 20bc1a6 on skaji:pod-img into 8daecc2 on metacpan:master.

@oalders
Copy link
Member

oalders commented May 5, 2017

Thanks @skaji!

@oalders oalders merged commit cc11b67 into metacpan:master May 5, 2017
@haarg
Copy link
Member

haarg commented May 5, 2017

This breaks some uses of images: https://metacpan.org/pod/Acme::CPANAuthors::Nonhuman

@skaji
Copy link
Contributor Author

skaji commented May 5, 2017

@haarg thanks for pointing out that.

before

screen shot 2017-05-05 at 14 16 01

after

screen shot 2017-05-05 at 14 16 27

what to do

If we keep previous behavior, then I think we should set class="img-responsive" only if <img> does not have the width attribute.

@haarg What do you think?

@haarg
Copy link
Member

haarg commented May 7, 2017

@skaji We should probably investigate the current uses of inline images more thoroughly. Having the width and height set seems a little too incidental to use for this purpose.

@karenetheridge
Copy link
Contributor

:(

@Grinnz
Copy link
Contributor

Grinnz commented May 8, 2017

Since according to bootstrap docs the img-responsive class adds the following styles: max-width: 100%;, height: auto; display: block; then instead of setting that class, the first two styles could be set, excluding display: block. That seems to work for scaling the Parallel::Pipes image, and images like in Acme::CPANAuthors::Nonhuman don't each go on their own line.

@skaji
Copy link
Contributor Author

skaji commented May 8, 2017

My original intent for this PR is below.

Since #1844, style attributes for <img> tags have been not allowed.
I used to use <img style="max-width:100%;" ...>, but it does not work anymore.
So I want to add style="max-width:100%;" to <img> tags somehow.

Yes, I missed the inline usage of <img> tags, sorry.

I'm totally okay with reverting this PR for a while.

@Grinnz Thanks. Sounds good.

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

Successfully merging this pull request may close these issues.

6 participants