Skip to content

Commit 1404420

Browse files
committed
fix(PictureThumbnail): Show spinner while images load
The connectedCallback used replaceChildren which immediately replaced the spinner with the still-loading image. Now it appends the image alongside the spinner so both are in the DOM. The existing CSS hides the image via opacity: 0 on the [loading] attribute and fades it in once the loading attribute is removed. (cherry picked from commit 42e6460)
1 parent 5f36e69 commit 1404420

File tree

2 files changed

+45
-8
lines changed

2 files changed

+45
-8
lines changed

app/javascript/alchemy_admin/components/picture_thumbnail.js

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,7 @@ export default class PictureThumbnail extends HTMLElement {
2929
}
3030

3131
connectedCallback() {
32-
if (this.image) {
33-
this.replaceChildren(this.image)
34-
}
32+
this.#setImage()
3533
}
3634

3735
disconnectedCallback() {
@@ -87,13 +85,21 @@ export default class PictureThumbnail extends HTMLElement {
8785
console.error(message, evt)
8886
}
8987

88+
#setImage() {
89+
if (this.image?.complete) {
90+
this.replaceChildren(this.image)
91+
} else if (this.image) {
92+
this.append(this.image)
93+
}
94+
}
95+
9096
set loading(value) {
9197
value ? this.load() : this.stop()
9298
}
9399

94100
set src(src) {
95101
this.start(src)
96-
this.replaceChildren(this.image)
102+
this.#setImage()
97103
}
98104

99105
get name() {

spec/javascript/alchemy_admin/components/picture_thumbnail.spec.js

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,24 +43,38 @@ describe("alchemy-picture-thumbnail", () => {
4343
})
4444

4545
describe("connectedCallback", () => {
46-
it("appends image to element", () => {
46+
it("shows spinner and appends image while loading", () => {
4747
const element = renderComponent(
4848
"alchemy-picture-thumbnail",
4949
'<alchemy-picture-thumbnail src="https://example.com/image.jpg"></alchemy-picture-thumbnail>'
5050
)
5151

52+
expect(element.querySelector("alchemy-spinner")).not.toBeNull()
5253
expect(element).toContain(element.image)
5354
})
5455

55-
it("replaces existing placeholder content with image", () => {
56+
it("appends image when already complete", () => {
57+
const element = renderComponent(
58+
"alchemy-picture-thumbnail",
59+
"<alchemy-picture-thumbnail></alchemy-picture-thumbnail>"
60+
)
61+
element.image = new Image()
62+
Object.defineProperty(element.image, "complete", { value: true })
63+
64+
element.connectedCallback()
65+
66+
expect(element).toContain(element.image)
67+
})
68+
69+
it("replaces placeholder content with spinner and image while loading", () => {
5670
const element = renderComponent(
5771
"alchemy-picture-thumbnail",
5872
'<alchemy-picture-thumbnail src="https://example.com/image.jpg"><alchemy-icon name="image"></alchemy-icon></alchemy-picture-thumbnail>'
5973
)
6074

6175
expect(element.querySelector("alchemy-icon")).toBeNull()
76+
expect(element.querySelector("alchemy-spinner")).not.toBeNull()
6277
expect(element).toContain(element.image)
63-
expect(element.childNodes.length).toBe(1)
6478
})
6579

6680
it("does nothing if image does not exist", () => {
@@ -254,6 +268,7 @@ describe("alchemy-picture-thumbnail", () => {
254268

255269
expect(stopSpy).toHaveBeenCalled()
256270
expect(element.hasAttribute("loading")).toBe(false)
271+
expect(element).toContain(element.image)
257272
})
258273

259274
it("handles error event", () => {
@@ -385,7 +400,7 @@ describe("alchemy-picture-thumbnail", () => {
385400
expect(startSpy).toHaveBeenCalledWith("https://example.com/new.jpg")
386401
})
387402

388-
it("replaces children with new image", () => {
403+
it("shows spinner while new image is loading", () => {
389404
const element = renderComponent(
390405
"alchemy-picture-thumbnail",
391406
'<alchemy-picture-thumbnail src="https://example.com/old.jpg"></alchemy-picture-thumbnail>'
@@ -395,7 +410,23 @@ describe("alchemy-picture-thumbnail", () => {
395410
element.src = "https://example.com/new.jpg"
396411

397412
expect(element.contains(oldImage)).toBe(false)
413+
expect(element.querySelector("alchemy-spinner")).not.toBeNull()
414+
})
415+
416+
it("replaces children with image when already complete", () => {
417+
const element = renderComponent(
418+
"alchemy-picture-thumbnail",
419+
"<alchemy-picture-thumbnail></alchemy-picture-thumbnail>"
420+
)
421+
const startSpy = vi.spyOn(element, "start").mockImplementation(() => {
422+
element.image = new Image()
423+
Object.defineProperty(element.image, "complete", { value: true })
424+
})
425+
426+
element.src = "https://example.com/new.jpg"
427+
398428
expect(element.contains(element.image)).toBe(true)
429+
startSpy.mockRestore()
399430
})
400431
})
401432

0 commit comments

Comments
 (0)