refactor: thumbnail components #26379
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| import Image from '$lib/components/Image.svelte'; | ||
| import { cancelImageUrl } from '$lib/utils/sw-messaging'; | ||
| import { fireEvent, render } from '@testing-library/svelte'; | ||
|
|
||
| vi.mock('$lib/utils/sw-messaging', () => ({ | ||
| cancelImageUrl: vi.fn(), | ||
| })); | ||
|
|
||
| describe('Image component', () => { | ||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| it('renders an img element when src is provided', () => { | ||
| const { baseElement } = render(Image, { src: '/test.jpg', alt: 'test' }); | ||
| const img = baseElement.querySelector('img'); | ||
| expect(img).not.toBeNull(); | ||
| expect(img!.getAttribute('src')).toBe('/test.jpg'); | ||
| }); | ||
|
|
||
| it('does not render an img element when src is undefined', () => { | ||
| const { baseElement } = render(Image, { src: undefined }); | ||
| const img = baseElement.querySelector('img'); | ||
| expect(img).toBeNull(); | ||
| }); | ||
|
|
||
| it('calls onStart when src is set', () => { | ||
| const onStart = vi.fn(); | ||
| render(Image, { src: '/test.jpg', onStart }); | ||
| expect(onStart).toHaveBeenCalledOnce(); | ||
| }); | ||
|
|
||
| it('calls onLoad when image loads', async () => { | ||
| const onLoad = vi.fn(); | ||
| const { baseElement } = render(Image, { src: '/test.jpg', onLoad }); | ||
| const img = baseElement.querySelector('img')!; | ||
| await fireEvent.load(img); | ||
| expect(onLoad).toHaveBeenCalledOnce(); | ||
| }); | ||
|
|
||
| it('calls onError when image fails to load', async () => { | ||
| const onError = vi.fn(); | ||
| const { baseElement } = render(Image, { src: '/test.jpg', onError }); | ||
| const img = baseElement.querySelector('img')!; | ||
| await fireEvent.error(img); | ||
| expect(onError).toHaveBeenCalledOnce(); | ||
| expect(onError).toHaveBeenCalledWith(expect.any(Error)); | ||
| expect(onError.mock.calls[0][0].message).toBe('Failed to load image: /test.jpg'); | ||
| }); | ||
|
|
||
| it('calls cancelImageUrl on unmount', () => { | ||
| const { unmount } = render(Image, { src: '/test.jpg' }); | ||
| expect(cancelImageUrl).not.toHaveBeenCalled(); | ||
| unmount(); | ||
| expect(cancelImageUrl).toHaveBeenCalledWith('/test.jpg'); | ||
| }); | ||
|
|
||
| it('does not call onLoad after unmount', async () => { | ||
| const onLoad = vi.fn(); | ||
| const { baseElement, unmount } = render(Image, { src: '/test.jpg', onLoad }); | ||
| const img = baseElement.querySelector('img')!; | ||
| unmount(); | ||
| await fireEvent.load(img); | ||
| expect(onLoad).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('does not call onError after unmount', async () => { | ||
| const onError = vi.fn(); | ||
| const { baseElement, unmount } = render(Image, { src: '/test.jpg', onError }); | ||
| const img = baseElement.querySelector('img')!; | ||
| unmount(); | ||
| await fireEvent.error(img); | ||
| expect(onError).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('passes through additional HTML attributes', () => { | ||
| const { baseElement } = render(Image, { | ||
| src: '/test.jpg', | ||
| alt: 'test alt', | ||
| class: 'my-class', | ||
| draggable: false, | ||
| }); | ||
| const img = baseElement.querySelector('img')!; | ||
| expect(img.getAttribute('alt')).toBe('test alt'); | ||
| expect(img.getAttribute('draggable')).toBe('false'); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| <script lang="ts"> | ||
| import { imageManager } from '$lib/managers/ImageManager.svelte'; | ||
| import { onDestroy, untrack } from 'svelte'; | ||
| import type { HTMLImgAttributes } from 'svelte/elements'; | ||
|
|
||
| type Props = Omit<HTMLImgAttributes, 'onload' | 'onerror'> & { | ||
| src: string | undefined; | ||
| onStart?: () => void; | ||
| onLoad?: () => void; | ||
| onError?: (error: Error) => void; | ||
| ref?: HTMLImageElement; | ||
| }; | ||
|
|
||
| let { src, onStart, onLoad, onError, ref = $bindable(), ...rest }: Props = $props(); | ||
|
|
||
| let capturedSource: string | undefined = $state(); | ||
| let destroyed = false; | ||
|
|
||
| $effect(() => { | ||
| if (src !== undefined && capturedSource === undefined) { | ||
| capturedSource = src; | ||
| untrack(() => { | ||
| onStart?.(); | ||
| }); | ||
| } | ||
| }); | ||
|
|
||
| onDestroy(() => { | ||
| destroyed = true; | ||
| if (capturedSource !== undefined) { | ||
| imageManager.cancelPreloadUrl(capturedSource); | ||
| } | ||
| }); | ||
|
|
||
| const handleLoad = () => { | ||
| if (destroyed || !src) { | ||
| return; | ||
| } | ||
| onLoad?.(); | ||
| }; | ||
|
|
||
| const handleError = () => { | ||
| if (destroyed || !src) { | ||
| return; | ||
| } | ||
| onError?.(new Error(`Failed to load image: ${src}`)); | ||
| }; | ||
| </script> | ||
|
|
||
| {#if capturedSource} | ||
| {#key capturedSource} | ||
|
Member
There was a problem hiding this comment. You don't need the |
||
| <img bind:this={ref} src={capturedSource} {...rest} onload={handleLoad} onerror={handleError} /> | ||
| {/key} | ||
| {/if} | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,89 @@ | ||
| import ImageThumbnail from '$lib/components/assets/thumbnail/image-thumbnail.svelte'; | ||
| import { fireEvent, render } from '@testing-library/svelte'; | ||
|
|
||
| vi.mock('$lib/utils/sw-messaging', () => ({ | ||
| cancelImageUrl: vi.fn(), | ||
| })); | ||
|
|
||
| describe('ImageThumbnail component', () => { | ||
| beforeEach(() => { | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| it('renders an img element with correct attributes', () => { | ||
| const { baseElement } = render(ImageThumbnail, { | ||
| url: '/test-thumbnail.jpg', | ||
| altText: 'Test image', | ||
| widthStyle: '200px', | ||
| }); | ||
| const img = baseElement.querySelector('img'); | ||
| expect(img).not.toBeNull(); | ||
| expect(img!.getAttribute('src')).toBe('/test-thumbnail.jpg'); | ||
| expect(img!.getAttribute('alt')).toBe(''); | ||
| }); | ||
|
|
||
| it('shows BrokenAsset on error', async () => { | ||
| const { baseElement } = render(ImageThumbnail, { | ||
| url: '/test-thumbnail.jpg', | ||
| altText: 'Test image', | ||
| widthStyle: '200px', | ||
| }); | ||
| const img = baseElement.querySelector('img')!; | ||
| await fireEvent.error(img); | ||
|
|
||
| expect(baseElement.querySelector('img')).toBeNull(); | ||
| expect(baseElement.querySelector('span')?.textContent).toEqual('error_loading_image'); | ||
| }); | ||
|
|
||
| it('calls onComplete with false on successful load', async () => { | ||
| const onComplete = vi.fn(); | ||
| const { baseElement } = render(ImageThumbnail, { | ||
| url: '/test-thumbnail.jpg', | ||
| altText: 'Test image', | ||
| widthStyle: '200px', | ||
| onComplete, | ||
| }); | ||
| const img = baseElement.querySelector('img')!; | ||
| await fireEvent.load(img); | ||
| expect(onComplete).toHaveBeenCalledWith(false); | ||
| }); | ||
|
|
||
| it('calls onComplete with true on error', async () => { | ||
| const onComplete = vi.fn(); | ||
| const { baseElement } = render(ImageThumbnail, { | ||
| url: '/test-thumbnail.jpg', | ||
| altText: 'Test image', | ||
| widthStyle: '200px', | ||
| onComplete, | ||
| }); | ||
| const img = baseElement.querySelector('img')!; | ||
| await fireEvent.error(img); | ||
| expect(onComplete).toHaveBeenCalledWith(true); | ||
| }); | ||
|
|
||
| it('applies hidden styles when hidden is true', () => { | ||
| const { baseElement } = render(ImageThumbnail, { | ||
| url: '/test-thumbnail.jpg', | ||
| altText: 'Test image', | ||
| widthStyle: '200px', | ||
| hidden: true, | ||
| }); | ||
| const img = baseElement.querySelector('img')!; | ||
| const style = img.getAttribute('style') ?? ''; | ||
| expect(style).toContain('grayscale'); | ||
| expect(style).toContain('opacity'); | ||
| }); | ||
|
|
||
| it('sets alt text after loading', async () => { | ||
| const { baseElement } = render(ImageThumbnail, { | ||
| url: '/test-thumbnail.jpg', | ||
| altText: 'Test image', | ||
| widthStyle: '200px', | ||
| }); | ||
| const img = baseElement.querySelector('img')!; | ||
| expect(img.getAttribute('alt')).toBe(''); | ||
|
|
||
| await fireEvent.load(img); | ||
| expect(img.getAttribute('alt')).toBe('Test image'); | ||
| }); | ||
| }); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@michelheusschen Hopefully the variable naming now makes it more clear that we only intend to operate on a single non-undefined src value. If you want this to call listeners and update multiple times based on updates to the
srcprop - surround this with a{#key}- or just use native<img>