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

news: Make images embedded in posts responsive #93

Closed
wants to merge 1 commit into from

Conversation

Holzhaus
Copy link
Member

No description provided.

@Be-ing
Copy link
Contributor

Be-ing commented May 19, 2020

This is helpful, but it only works for images that are wider than the text column at 1x scale factor. Other images have to have max-height specified in px, which I don't think can be done in Markdown.

@Holzhaus
Copy link
Member Author

This is helpful, but it only works for images that are wider than the text column at 1x scale factor. Other images have to have max-height specified in px, which I don't think can be done in Markdown.

What do you mean? Why would you want to specify max-height? The aspect ratio is kept automatically. And if you have images that are not as wide as the text they will stay at their original size.

@Be-ing
Copy link
Contributor

Be-ing commented May 19, 2020

image
The first image does not have max-height set, the second does.

@Be-ing
Copy link
Contributor

Be-ing commented May 19, 2020

@radusuciu @randombyte-developer any input on how we should handle this?

@Holzhaus
Copy link
Member Author

Is the upper image enlarged? If the image is actually that big, we should just downscale it to the appropriate height. Downscaling it via HTML just creates unnecessary traffic.

@Be-ing
Copy link
Contributor

Be-ing commented May 19, 2020

The really proper way to handle these images for different scale factors is with the srcset attribute for img. I don't think Markdown has any way to do that.

That's also quite a bit more work to scale all those images. For the main website, I think the work is worthwhile; for a blog post with a few images, probably more trouble than it's worth.

@Holzhaus
Copy link
Member Author

That's also quite a bit more work to scale all those images. For the main website, I think the work is worthwhile; for a blog post with a few images, probably more trouble than it's worth.

If you have ImageMagick installed, you can resize all images in the current working directory to 300 px height using this:

$ mogrify -resize x300 *.jpg

@Holzhaus
Copy link
Member Author

@Be-ing as a workaround we can also do this:

-{% filter markdown %}
+{% filter markdown:"extra" %}

 Some text.

-![my caption](path/to/image.png)
+![my caption](path/to/image.png){: style="max-height: 256px" }

@Holzhaus
Copy link
Member Author

Holzhaus commented Jun 1, 2020

Merge? This doesn't break anything, just makes sure that images bigger than the viewport are resized.

@Holzhaus Holzhaus closed this Jun 3, 2020
@Holzhaus Holzhaus mentioned this pull request Jun 7, 2020
@Be-ing
Copy link
Contributor

Be-ing commented Jul 8, 2020

So how should we handle this? I am okay with merging this PR, but you closed it?

@Holzhaus
Copy link
Member Author

Holzhaus commented Jul 8, 2020

Already fixed with the new theme. This should work: #93 (comment)

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

Successfully merging this pull request may close these issues.

None yet

2 participants