mirror of
https://github.com/discourse/discourse.git
synced 2025-03-09 14:34:35 +00:00
FIX: Ensure images do not change height when loading is complete (#16368)
Browsers automatically calculate an aspect ratio based on the width/height attributes of an `<img`. HOWEVER that aspect ratio only applies while the image is loading. Once loaded, it'll use the image's actual dimensions. This can cause things to jump around after loading. For example: - if a user deliberately inserts false width/height - the image fails to load (404) - an optimised image is a few pixels different, due to a rounding when resizing This decorator explicitly sets the `aspect-ratio` property so that things are consistent throughout the lifetime of all `<img` elements.
This commit is contained in:
parent
7179fbab77
commit
7edc941843
@ -0,0 +1,60 @@
|
||||
import { withPluginApi } from "discourse/lib/plugin-api";
|
||||
|
||||
// Browsers automatically calculate an aspect ratio based on the width/height attributes of an `<img`.
|
||||
// HOWEVER that aspect ratio only applies while the image is loading. Once loaded, it'll use the
|
||||
// image's actual dimensions. This can cause things to jump around after loading. For example:
|
||||
// - if a user deliberately inserts false width/height
|
||||
// - the image fails to load (404)
|
||||
// - an optimised image is a few pixels different, due to a rounding when resizing
|
||||
//
|
||||
// This decorator explicitly sets the `aspect-ratio` property so that things are consistent throughout
|
||||
// the lifetime of all `<img` elements.
|
||||
|
||||
export default {
|
||||
name: "image-aspect-ratio",
|
||||
|
||||
initWithApi(api) {
|
||||
const supportsAspectRatio = CSS.supports("aspect-ratio: 1");
|
||||
|
||||
api.decorateCookedElement(
|
||||
(element) => {
|
||||
element.querySelectorAll("img").forEach((img) => {
|
||||
const declaredHeight = parseFloat(img.getAttribute("height"));
|
||||
const declaredWidth = parseFloat(img.getAttribute("width"));
|
||||
|
||||
if (
|
||||
isNaN(declaredHeight) ||
|
||||
isNaN(declaredWidth) ||
|
||||
img.style.aspectRatio
|
||||
) {
|
||||
return;
|
||||
}
|
||||
|
||||
if (supportsAspectRatio) {
|
||||
img.style.setProperty(
|
||||
"aspect-ratio",
|
||||
`${declaredWidth} / ${declaredHeight}`
|
||||
);
|
||||
} else {
|
||||
// For older browsers (e.g. iOS < 15), we need to apply the aspect ratio manually.
|
||||
// It's not perfect, because it won't recompute on browser resize.
|
||||
// This property is consumed in `topic-post.scss` for responsive images only.
|
||||
// It's a no-op for non-responsive images.
|
||||
const calculatedHeight =
|
||||
img.width / (declaredWidth / declaredHeight);
|
||||
|
||||
img.style.setProperty(
|
||||
"--calculated-height",
|
||||
`${calculatedHeight}px`
|
||||
);
|
||||
}
|
||||
});
|
||||
},
|
||||
{ id: "image-aspect-ratio" }
|
||||
);
|
||||
},
|
||||
|
||||
initialize() {
|
||||
withPluginApi("1.2.0", this.initWithApi);
|
||||
},
|
||||
};
|
@ -1,4 +1,8 @@
|
||||
import { acceptance, queryAll } from "discourse/tests/helpers/qunit-helpers";
|
||||
import {
|
||||
acceptance,
|
||||
normalizeHtml,
|
||||
queryAll,
|
||||
} from "discourse/tests/helpers/qunit-helpers";
|
||||
import { click, fillIn, visit } from "@ember/test-helpers";
|
||||
import { test } from "qunit";
|
||||
import { IMAGE_VERSION as v } from "pretty-text/emoji/version";
|
||||
@ -12,8 +16,10 @@ acceptance("Emoji", function (needs) {
|
||||
|
||||
await fillIn(".d-editor-input", "this is an emoji :blonde_woman:");
|
||||
assert.strictEqual(
|
||||
queryAll(".d-editor-preview:visible").html().trim(),
|
||||
`<p>this is an emoji <img src="/images/emoji/google_classic/blonde_woman.png?v=${v}" title=":blonde_woman:" class="emoji" alt=":blonde_woman:" loading="lazy" width="20" height="20"></p>`
|
||||
normalizeHtml(queryAll(".d-editor-preview:visible").html().trim()),
|
||||
normalizeHtml(
|
||||
`<p>this is an emoji <img src="/images/emoji/google_classic/blonde_woman.png?v=${v}" title=":blonde_woman:" class="emoji" alt=":blonde_woman:" loading="lazy" width="20" height="20" style="aspect-ratio: 20 / 20;"></p>`
|
||||
)
|
||||
);
|
||||
});
|
||||
|
||||
@ -22,9 +28,12 @@ acceptance("Emoji", function (needs) {
|
||||
await click("#topic-footer-buttons .btn.create");
|
||||
|
||||
await fillIn(".d-editor-input", "this is an emoji :blonde_woman:t5:");
|
||||
|
||||
assert.strictEqual(
|
||||
queryAll(".d-editor-preview:visible").html().trim(),
|
||||
`<p>this is an emoji <img src="/images/emoji/google_classic/blonde_woman/5.png?v=${v}" title=":blonde_woman:t5:" class="emoji" alt=":blonde_woman:t5:" loading="lazy" width="20" height="20"></p>`
|
||||
normalizeHtml(queryAll(".d-editor-preview:visible").html().trim()),
|
||||
normalizeHtml(
|
||||
`<p>this is an emoji <img src="/images/emoji/google_classic/blonde_woman/5.png?v=${v}" title=":blonde_woman:t5:" class="emoji" alt=":blonde_woman:t5:" loading="lazy" width="20" height="20" style="aspect-ratio: 20 / 20;"></p>`
|
||||
)
|
||||
);
|
||||
});
|
||||
});
|
||||
|
@ -0,0 +1,12 @@
|
||||
import { acceptance, query } from "discourse/tests/helpers/qunit-helpers";
|
||||
import { visit } from "@ember/test-helpers";
|
||||
import { test } from "qunit";
|
||||
|
||||
acceptance("Image aspect ratio", function () {
|
||||
test("it applies the aspect ratio", async function (assert) {
|
||||
await visit("/t/2480");
|
||||
const image = query("#post_3 img[src='/assets/logo.png']");
|
||||
|
||||
assert.strictEqual(image.style.aspectRatio, "690 / 388");
|
||||
});
|
||||
});
|
@ -2,6 +2,7 @@ import {
|
||||
acceptance,
|
||||
count,
|
||||
exists,
|
||||
normalizeHtml,
|
||||
query,
|
||||
queryAll,
|
||||
visible,
|
||||
@ -54,8 +55,13 @@ acceptance("User Drafts", function (needs) {
|
||||
"meta"
|
||||
);
|
||||
assert.strictEqual(
|
||||
query(".user-stream-item:nth-child(3) .excerpt").innerHTML.trim(),
|
||||
`here goes a reply to a PM <img src="/images/emoji/google_classic/slight_smile.png?v=${IMAGE_VERSION}" title=":slight_smile:" class="emoji" alt=":slight_smile:" loading="lazy" width="20" height="20">`
|
||||
normalizeHtml(
|
||||
query(".user-stream-item:nth-child(3) .excerpt").innerHTML.trim()
|
||||
),
|
||||
normalizeHtml(
|
||||
`here goes a reply to a PM <img src="/images/emoji/google_classic/slight_smile.png?v=${IMAGE_VERSION}" title=":slight_smile:" class="emoji" alt=":slight_smile:" loading="lazy" width="20" height="20" style="aspect-ratio: 20 / 20;">`
|
||||
),
|
||||
"shows the excerpt"
|
||||
);
|
||||
});
|
||||
});
|
||||
|
@ -574,3 +574,12 @@ export async function paste(element, text, otherClipboardData = {}) {
|
||||
await settled();
|
||||
return e;
|
||||
}
|
||||
|
||||
// The order of attributes can vary in diffferent browsers. When comparing
|
||||
// HTML strings from the DOM, this function helps to normalize them to make
|
||||
// comparison work cross-browser
|
||||
export function normalizeHtml(html) {
|
||||
const resultElement = document.createElement("template");
|
||||
resultElement.innerHTML = html;
|
||||
return resultElement.innerHTML;
|
||||
}
|
||||
|
@ -192,6 +192,11 @@ $quote-share-maxwidth: 150px;
|
||||
img:not(.thumbnail):not(.ytp-thumbnail-image):not(.emoji) {
|
||||
max-width: 100%;
|
||||
height: auto;
|
||||
|
||||
@supports not (aspect-ratio: 1) {
|
||||
// (see javascripts/discourse/app/initializers/image-aspect-ratio.js)
|
||||
height: var(--calculated-height);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
Loading…
x
Reference in New Issue
Block a user