From 4b49d3a85d7cafd91bf752c7386e30a54c53ca8d Mon Sep 17 00:00:00 2001 From: Min Idzelis Date: Fri, 7 Jun 2024 14:22:46 -0400 Subject: [PATCH] feat: photo-viewer; use instead of blob urls, simplify/refactor, avoid window.events (#9883) * Photoviewer * make copyImage/zoomToggle optional * Add e2e test * lint * Accept bo0tzz suggestion Co-authored-by: bo0tzz * Bad merge and review comments * unused import --------- Co-authored-by: bo0tzz Co-authored-by: Alex --- e2e/src/utils.ts | 34 ++++ e2e/src/web/specs/photo-viewer.e2e-spec.ts | 57 ++++++ web/src/lib/actions/zoom-image.ts | 31 ++++ .../asset-viewer/asset-viewer-nav-bar.svelte | 17 +- .../asset-viewer/asset-viewer.svelte | 17 +- .../asset-viewer/photo-viewer.spec.ts | 83 --------- .../asset-viewer/photo-viewer.svelte | 162 +++++++----------- web/src/lib/stores/zoom-image.store.ts | 1 + 8 files changed, 200 insertions(+), 202 deletions(-) create mode 100644 e2e/src/web/specs/photo-viewer.e2e-spec.ts create mode 100644 web/src/lib/actions/zoom-image.ts delete mode 100644 web/src/lib/components/asset-viewer/photo-viewer.spec.ts diff --git a/e2e/src/utils.ts b/e2e/src/utils.ts index d246293d94..2b26537677 100644 --- a/e2e/src/utils.ts +++ b/e2e/src/utils.ts @@ -323,6 +323,40 @@ export const utils = { return body as AssetMediaResponseDto; }, + replaceAsset: async ( + accessToken: string, + assetId: string, + dto?: Partial> & { assetData?: AssetData }, + ) => { + const _dto = { + deviceAssetId: 'test-1', + deviceId: 'test', + fileCreatedAt: new Date().toISOString(), + fileModifiedAt: new Date().toISOString(), + ...dto, + }; + + const assetData = dto?.assetData?.bytes || makeRandomImage(); + const filename = dto?.assetData?.filename || 'example.png'; + + if (dto?.assetData?.bytes) { + console.log(`Uploading ${filename}`); + } + + const builder = request(app) + .put(`/assets/${assetId}/original`) + .attach('assetData', assetData, filename) + .set('Authorization', `Bearer ${accessToken}`); + + for (const [key, value] of Object.entries(_dto)) { + void builder.field(key, String(value)); + } + + const { body } = await builder; + + return body as AssetMediaResponseDto; + }, + createImageFile: (path: string) => { if (!existsSync(dirname(path))) { mkdirSync(dirname(path), { recursive: true }); diff --git a/e2e/src/web/specs/photo-viewer.e2e-spec.ts b/e2e/src/web/specs/photo-viewer.e2e-spec.ts new file mode 100644 index 0000000000..f825b10315 --- /dev/null +++ b/e2e/src/web/specs/photo-viewer.e2e-spec.ts @@ -0,0 +1,57 @@ +import { AssetMediaResponseDto, LoginResponseDto } from '@immich/sdk'; +import { Page, expect, test } from '@playwright/test'; +import { utils } from 'src/utils'; + +function imageLocator(page: Page) { + return page.getByAltText('Image taken on').locator('visible=true'); +} +test.describe('Photo Viewer', () => { + let admin: LoginResponseDto; + let asset: AssetMediaResponseDto; + + test.beforeAll(async () => { + utils.initSdk(); + await utils.resetDatabase(); + admin = await utils.adminSetup(); + asset = await utils.createAsset(admin.accessToken); + }); + + test.beforeEach(async ({ context, page }) => { + // before each test, login as user + await utils.setAuthCookies(context, admin.accessToken); + await page.goto('/photos'); + await page.waitForLoadState('networkidle'); + }); + + test('initially shows a loading spinner', async ({ page }) => { + await page.route(`/api/assets/${asset.id}/thumbnail**`, async (route) => { + // slow down the request for thumbnail, so spiner has chance to show up + await new Promise((f) => setTimeout(f, 2000)); + await route.continue(); + }); + await page.goto(`/photos/${asset.id}`); + await page.waitForLoadState('load'); + // this is the spinner + await page.waitForSelector('svg[role=status]'); + await expect(page.getByRole('status')).toBeVisible(); + }); + + test('loads high resolution photo when zoomed', async ({ page }) => { + await page.goto(`/photos/${asset.id}`); + await expect.poll(async () => await imageLocator(page).getAttribute('src')).toContain('thumbnail'); + const box = await imageLocator(page).boundingBox(); + expect(box).toBeTruthy; + const { x, y, width, height } = box!; + await page.mouse.move(x + width / 2, y + height / 2); + await page.mouse.wheel(0, -1); + await expect.poll(async () => await imageLocator(page).getAttribute('src')).toContain('original'); + }); + + test('reloads photo when checksum changes', async ({ page }) => { + await page.goto(`/photos/${asset.id}`); + await expect.poll(async () => await imageLocator(page).getAttribute('src')).toContain('thumbnail'); + const initialSrc = await imageLocator(page).getAttribute('src'); + await utils.replaceAsset(admin.accessToken, asset.id); + await expect.poll(async () => await imageLocator(page).getAttribute('src')).not.toBe(initialSrc); + }); +}); diff --git a/web/src/lib/actions/zoom-image.ts b/web/src/lib/actions/zoom-image.ts new file mode 100644 index 0000000000..24a1a25ac1 --- /dev/null +++ b/web/src/lib/actions/zoom-image.ts @@ -0,0 +1,31 @@ +import { photoZoomState, zoomed } from '$lib/stores/zoom-image.store'; +import { useZoomImageWheel } from '@zoom-image/svelte'; +import { get } from 'svelte/store'; + +export { zoomed } from '$lib/stores/zoom-image.store'; + +export const zoomImageAction = (node: HTMLElement) => { + const { createZoomImage, zoomImageState, setZoomImageState } = useZoomImageWheel(); + + createZoomImage(node, { + maxZoom: 10, + wheelZoomRatio: 0.2, + }); + + const state = get(photoZoomState); + if (state) { + setZoomImageState(state); + } + + const unsubscribes = [ + zoomed.subscribe((state) => setZoomImageState({ currentZoom: state ? 2 : 1 })), + zoomImageState.subscribe((state) => photoZoomState.set(state)), + ]; + return { + destroy() { + for (const unsubscribe of unsubscribes) { + unsubscribe(); + } + }, + }; +}; diff --git a/web/src/lib/components/asset-viewer/asset-viewer-nav-bar.svelte b/web/src/lib/components/asset-viewer/asset-viewer-nav-bar.svelte index a8b48993c9..2dfb841e93 100644 --- a/web/src/lib/components/asset-viewer/asset-viewer-nav-bar.svelte +++ b/web/src/lib/components/asset-viewer/asset-viewer-nav-bar.svelte @@ -51,6 +51,8 @@ export let showShareButton: boolean; export let showSlideshow = false; export let hasStackChildren = false; + export let onZoomImage: () => void; + export let onCopyImage: () => void; $: isOwner = $user && asset.ownerId === $user?.id; @@ -144,22 +146,11 @@ hideMobile={true} icon={$photoZoomState && $photoZoomState.currentZoom > 1 ? mdiMagnifyMinusOutline : mdiMagnifyPlusOutline} title={$t('zoom_image')} - on:click={() => { - const zoomImage = new CustomEvent('zoomImage'); - window.dispatchEvent(zoomImage); - }} + on:click={onZoomImage} /> {/if} {#if showCopyButton} - { - const copyEvent = new CustomEvent('copyImage'); - window.dispatchEvent(copyEvent); - }} - /> + {/if} {#if !isOwner && showDownloadButton} diff --git a/web/src/lib/components/asset-viewer/asset-viewer.svelte b/web/src/lib/components/asset-viewer/asset-viewer.svelte index b7da50e69a..30e6222f28 100644 --- a/web/src/lib/components/asset-viewer/asset-viewer.svelte +++ b/web/src/lib/components/asset-viewer/asset-viewer.svelte @@ -59,6 +59,7 @@ import VideoViewer from './video-wrapper-viewer.svelte'; import { navigate } from '$lib/utils/navigation'; import { websocketEvents } from '$lib/stores/websocket'; + import { canCopyImagesToClipboard } from 'copy-image-clipboard'; import { t } from 'svelte-i18n'; export let assetStore: AssetStore | null = null; @@ -98,7 +99,6 @@ let shouldShowDownloadButton = sharedLink ? sharedLink.allowDownload : !asset.isOffline; let enableDetailPanel = asset.hasMetadata; let shouldShowShareModal = !asset.isTrashed; - let canCopyImagesToClipboard: boolean; let slideshowStateUnsubscribe: () => void; let shuffleSlideshowUnsubscribe: () => void; let previewStackedAsset: AssetResponseDto | undefined; @@ -107,6 +107,8 @@ let numberOfComments: number; let fullscreenElement: Element; let unsubscribe: () => void; + let zoomToggle = () => void 0; + let copyImage: () => Promise; $: isFullScreen = fullscreenElement !== null; $: { @@ -227,11 +229,6 @@ await handleGetAllAlbums(); } - // Import hack :( see https://github.com/vadimkorr/svelte-carousel/issues/27#issuecomment-851022295 - // TODO: Move to regular import once the package correctly supports ESM. - const module = await import('copy-image-clipboard'); - canCopyImagesToClipboard = module.canCopyImagesToClipboard(); - if (asset.stackCount && asset.stack) { $stackAssetsStore = asset.stack; $stackAssetsStore = [...$stackAssetsStore, asset].sort( @@ -568,7 +565,7 @@ {asset} {album} isMotionPhotoPlaying={shouldPlayMotionPhoto} - showCopyButton={canCopyImagesToClipboard && asset.type === AssetTypeEnum.Image} + showCopyButton={canCopyImagesToClipboard() && asset.type === AssetTypeEnum.Image} showZoomButton={asset.type === AssetTypeEnum.Image} showMotionPlayButton={!!asset.livePhotoVideoId} showDownloadButton={shouldShowDownloadButton} @@ -576,6 +573,8 @@ showSlideshow={!!assetStore} hasStackChildren={$stackAssetsStore.length > 0} showShareButton={shouldShowShareModal} + onZoomImage={zoomToggle} + onCopyImage={copyImage} on:back={closeViewer} on:showDetail={showDetailInfoHandler} on:download={() => downloadFile(asset)} @@ -623,6 +622,8 @@ {#key previewStackedAsset.id} {#if previewStackedAsset.type === AssetTypeEnum.Image} {:else} - + {/if} {:else} { - const meta = await originalImport(); - return { - ...meta, - downloadRequest: vi.fn(), - }; -}); - -describe('PhotoViewer component', () => { - let downloadRequestMock: MockInstance; - let createObjectURLMock: Mock<[obj: Blob], string>; - let asset: AssetResponseDto; - - beforeAll(() => { - downloadRequestMock = vi.spyOn(utils, 'downloadRequest').mockResolvedValue({ - data: new Blob(), - status: 200, - }); - createObjectURLMock = vi.fn(); - window.URL.createObjectURL = createObjectURLMock; - asset = assetFactory.build({ originalPath: 'image.png' }); - }); - - afterAll(() => { - vi.resetAllMocks(); - }); - - it('initially shows a loading spinner', () => { - render(PhotoViewer, { asset }); - expect(screen.getByRole('status')).toBeInTheDocument(); - }); - - it('loads and shows a photo', async () => { - createObjectURLMock.mockReturnValueOnce('url-one'); - render(PhotoViewer, { asset }); - - expect(downloadRequestMock).toBeCalledWith( - expect.objectContaining({ - url: `/api/assets/${asset.id}/thumbnail?size=preview&c=${asset.checksum}`, - }), - ); - await waitFor(() => expect(screen.getByRole('img')).toBeInTheDocument()); - expect(screen.getByRole('img')).toHaveAttribute('src', 'url-one'); - }); - - it('loads high resolution photo when zoomed', async () => { - createObjectURLMock.mockReturnValueOnce('url-one'); - render(PhotoViewer, { asset }); - createObjectURLMock.mockReturnValueOnce('url-two'); - - await waitFor(() => expect(screen.getByRole('img')).toBeInTheDocument()); - await fireEvent(window, new CustomEvent('zoomImage')); - await waitFor(() => expect(screen.getByRole('img')).toHaveAttribute('src', 'url-two')); - expect(downloadRequestMock).toBeCalledWith( - expect.objectContaining({ - url: `/api/assets/${asset.id}/original?c=${asset.checksum}`, - }), - ); - }); - - it('reloads photo when checksum changes', async () => { - const { component } = render(PhotoViewer, { asset }); - createObjectURLMock.mockReturnValueOnce('url-two'); - - await waitFor(() => expect(screen.getByRole('img')).toBeInTheDocument()); - component.$set({ asset: { ...asset, checksum: 'new-checksum' } }); - - await waitFor(() => expect(screen.getByRole('img')).toHaveAttribute('src', 'url-two')); - expect(downloadRequestMock).toBeCalledWith( - expect.objectContaining({ - url: `/api/assets/${asset.id}/thumbnail?size=preview&c=new-checksum`, - }), - ); - }); -}); diff --git a/web/src/lib/components/asset-viewer/photo-viewer.svelte b/web/src/lib/components/asset-viewer/photo-viewer.svelte index f531a3ee4c..4ce8c77d30 100644 --- a/web/src/lib/components/asset-viewer/photo-viewer.svelte +++ b/web/src/lib/components/asset-viewer/photo-viewer.svelte @@ -1,103 +1,83 @@ - -
+{#if imageError} +
Error loading image
+{/if} +
+ {getAltText(asset)} ((imageLoaded = true), (assetFileUrl = imageLoaderUrl))} + on:error={() => (imageError = imageLoaded = true)} + /> {#if !imageLoaded}
- {:else} -
+ {:else if !imageError} +
{#if $slideshowState !== SlideshowState.None && $slideshowLook === SlideshowLook.BlurredBackground}