search

cloudflare / embed-box

Universal install guide for embed codes and CMS plugins.
http://embedbox.io
MIT License
78 stars 12 forks source link

Preserve HTML screenshot aspect ratios #73

Closed adamschwartz closed 8 years ago

adamschwartz commented 8 years ago

(Particularly the HTML screenshots in the Drupal target.)

adamschwartz commented 8 years ago

@TeffenEllis Here’s a quick implementation: https://jsfiddle.net/adamschwartz/7y1vjn5w/show/light/

adamschwartz commented 8 years ago

I don’t believe this is quite working.

image

This screenshot illuminates just one of a few issues with the current implementation. But I think the main issue is that afaict many iframes are incorrectly having a width of 1000px width set.

image

In this example, the intrinsic ratio is 2:1 (0.5) and yet the aspect ratio here is closer to 7:2. With a correct implementation, this is a situation which should never occur.

Furthermore, there is never a situation in which whitespace (in this case gray-space) should be visible. We know the width of the code contents is ~680px so we shouldn’t be setting 1000px width on this iframe. Making this change allows the scale to be much larger resulting in a much more readable as well as a better looking “screenshot”.

image

I ultimately decided to make this a separate issue rather than reopening this one. Please see: https://github.com/EagerIO/EmbedBox/issues/84 for more information.