Can we get rid of height/width auto in oneboxes?

Question for @awesomerobot at the moment our oneboxes have height / width set to auto in the images. EG:

https://github.com/Supermathie/bratwurst

This cause pages to Jiggle when the image finally loads, despite us having the exact dimensions.

image

I was wondering if you can think of some workaround here where we can avoid this? Are we going to need to amend something server side to size these images down.

(keep in mind we need to account for mobile here as well)

6 Likes

I think the problem is wildly different aspect ratios. But if we know the size we should use it. Could be this is new info we only have after @vinothkannans improvement to download oneboxed images properly.

Yes, we now correctly know the aspect ratio, trouble is I am not sure if you can tell CSS just to size it down to aspect giving it max-height and width without specifying some percentage.

But maybe there is a CSS way, I am not sure, barring that we can run a post processor that takes care of the resizing in JS and then give it a special class to say it was already sized down so don’t do 100% width / height thing.

3 Likes

I guess we can get rid of this problem by removing below css lines

https://github.com/discourse/discourse/blob/master/app/assets/stylesheets/common/base/onebox.scss#L137-L138

Here width: auto; is added to fix xkcd oneboxes. For that adding max-height: 100% !important; in below is enough I think.

https://github.com/discourse/discourse/blob/master/app/assets/stylesheets/common/base/onebox.scss#L370-L375

2 Likes

Possibly, @awesomerobot should weigh in on this.

The bouncing of one boxes due to variable size is quite bad so it would be nice to improve this.

4 Likes

Probably not as an image, but as a background image, there is a setting to fit the image neatly inside the element as large as possible without cropping. You can consider turning this into a background image, but then you’ll lose some of the benefits of the img tag.

1 Like

Just tried it and it looks like you can remove height/width: auto and things work out? How are we setting height/width? Are we resizing the images proportionately when we download them? As long as we have static dimensions within our max height/width we shouldn’t need anything else.

Yes, CSS-defined background-image is a little nicer in this regard because you can set the container to fixed dimensions and then say background-size: contain; (will fit entire image to container proportionately, which may leave some white space on 2 sides).

Ultimately though, you’re not really doing much different here than if we put the images a placeholder container with static height/width. For a tall/narrow image you’d end up with whitespace on the sides of the image. We’d be losing the current functionality of reflowing the text around the image without leaving extra whitespace.

Right, accommodating wildly different aspect ratios is tough. To stay proportionate we can only define width OR height, but one property has to be set to auto. We let one dimension hit the max-width/height, and auto-define the other. Without applying auto to the other dimension, we run into proportion issues (for example, we don’t seem to specify auto on flickr embeds? see the stretched dog here)

So if I understand what you’re saying: the problem with CSS is that if you give an image a width of 120px and set a max-height of 200px — once you toss a 120px by 500px image into that box, it’ll be squished vertically.

If you know the aspect ratio of the original image you could do the math using calc() in css? You’d need some logic to determine if you should be defining or scaling height/width? You’d also be essentially supplying all the numbers to specify a percentage anyway? Maybe I’m overthinking it.

1 Like

At the moment the information we have that we want to apply is:

aside.onebox .onebox-body img {
  max-height: 170px;
  max-width: 20%;
  height: auto;
  width: auto;
}

<img src="https://d11a6trkgmumsb.cloudfront.net/original/3X/9/d/9d950db38b4b6d07b62c6a5a4b2ac882e17c68fa.png" width="103" height="103" class="thumbnail">

What we want to do in psuedocode is:

aside.onebox .onebox-box img[has-width AND has-height] {
   height: some magic calc that uses img width and height to restrict at 170px
   width: some magic calc that restricts width to 20% of container and maintains aspect
}

The 20% width thing makes stuff extra complicated.

Perhaps try a theme here that messes with it to see if we can get it to work?

4 Likes

You want to avoid the jiggle right but has no extra white space, right? That won’t avoid the jiggle. To avoid the jiggle you need to leave exactly the size of the eventual image waiting for it to load or show through a placeholder.

You can achieve that by taking a reference aspect ratio = max-width/max-height. Then check the AR of the image to see which direction you need to scale it. > and scale horizontally, < vertically etc.

Scaling is a quick one-liner and you can this make the img has the exact right width / height.

EDIT: Put the img inside a div with a background placeholder image, or put the background placeholder in the img itself, not sure if it will work.

Right, the only way I can think of doing this with CSS-only is doing something like

.wrapper {
  display: flex;
  width: 20%;
  height: 170px;
  justify-content: center;
}

img {
  max-width: 100%;
  max-height: 100%;
  align-self: center;
}

But you’re creating a container that will typically have some amount of whitespace around it unless it happens to match the 20% x 170px aspect ratio (which is variable because of the 20%). That could look something like this in situations with extremely tall or narrow images (I set the background of the wrapper to coral, to emphasize the extra whitespace)

4 Likes

I see, I guess we can just do it in the app then in JavaScript, cause there we can do whatever fancy we want and just give it a special class to bypass all the auto stuff, disadvantage though is that this will not likely be that resilient to resizes.

Accounting for:

image

Is super tricky.

Maybe we are overthinking all of this and just need to be way stricter with these onebox images, I don’t know.

We can pass the dimensions of the original image to CSS, where we can do some calculation and scale the image based on the aspect ratio of the original… Then we’d have scaled-down static dimensions in CSS based on the aspect ratio of the original image, which could resize? I might be oversimplifying.

https://codepen.io/krisaubuchon/pen/LzoXRo

5 Likes

Facebook crops their preview images to either a small square or a large rectangle, based on the image size and dimensions. If the image dimensions are way off the map they don’t display it at all. A lot of sites are setting their og:image to get it to display well on Facebook, so it might work to follow the same rules. This would probably only make sense if you wanted to make large rectangular images display at a larger size in the onebox.

vs

7 Likes

@sam can you own this next week, and tap whoever you need to assist? I definitely want oneboxes to stop bouncing on dynamic image size, it’s really bad for us.

This is now fixed per:

https://github.com/discourse/discourse/commit/9197feefb867fa1a028ae68b6ddc930119689600

The non-jittery-ness will be present on all new posts or rebaked posts as we needed to make some internal html changes. (note that the post processor that downloads images must run first which may take a minute or 2)

The non-jittery-ness works fine in all modern browsers (edge,firefox,chrome) but old behavior remains in IE.

It is implemented using serious levels of voodoo and CSS vars described here:

http://cssmojo.com/aspect-ratio-using-custom-properties-and-calc/

7 Likes