Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(web): add cover images to individual shares #9988

Merged
Merged
49 changes: 49 additions & 0 deletions web/src/lib/components/album-page/__tests__/album-cover.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import AlbumCover from '$lib/components/album-page/album-cover.svelte';
import { getAssetThumbnailUrl } from '$lib/utils';
import type { AlbumResponseDto } from '@immich/sdk';
import { render } from '@testing-library/svelte';
import { init, register, waitLocale } from 'svelte-i18n';

vi.mock('$lib/utils');

describe('AlbumCover component', () => {
beforeAll(async () => {
await init({ fallbackLocale: 'en-US' });
register('en-US', () => import('$lib/i18n/en-US.json'));
await waitLocale('en-US');
Snowknight26 marked this conversation as resolved.
Show resolved Hide resolved
});

it('renders an image when the album has a thumbnail', () => {
vi.mocked(getAssetThumbnailUrl).mockReturnValue('/asdf');
const component = render(AlbumCover, {
album: {
albumName: 'someName',
albumThumbnailAssetId: '123',
} as AlbumResponseDto,
Snowknight26 marked this conversation as resolved.
Show resolved Hide resolved
preload: false,
class: 'text',
});
const img = component.getByTestId('album-image') as HTMLImageElement;
expect(img.alt).toBe('someName');
expect(img.getAttribute('loading')).toBe('lazy');
expect(img.className).toBe('z-0 rounded-xl object-cover text');
expect(img.getAttribute('src')).toBe('/asdf');
expect(getAssetThumbnailUrl).toHaveBeenCalledWith({ id: '123' });
});

it('renders an image when the album has no thumbnail', () => {
const component = render(AlbumCover, {
album: {
albumName: '',
albumThumbnailAssetId: null,
} as AlbumResponseDto,
preload: true,
class: 'asdf',
});
const img = component.getByTestId('album-image') as HTMLImageElement;
expect(img.alt).toBe('Unnamed Album');
expect(img.getAttribute('loading')).toBe('eager');
expect(img.className).toBe('z-0 rounded-xl object-cover asdf');
expect(img.getAttribute('src')).toBeTruthy();
});
});
2 changes: 1 addition & 1 deletion web/src/lib/components/album-page/album-card.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@
</div>
{/if}

<AlbumCover {album} {preload} css="h-full w-full transition-all duration-300 hover:shadow-lg" />
<AlbumCover {album} {preload} class="h-full w-full transition-all duration-300 hover:shadow-lg" />

<div class="mt-4">
<p
Expand Down
30 changes: 9 additions & 21 deletions web/src/lib/components/album-page/album-cover.svelte
Original file line number Diff line number Diff line change
@@ -1,35 +1,23 @@
<script lang="ts">
import { getAssetThumbnailUrl } from '$lib/utils';
import { type AlbumResponseDto } from '@immich/sdk';
import NoCover from '$lib/components/sharedlinks-page/covers/no-cover.svelte';
import AssetCover from '$lib/components/sharedlinks-page/covers/asset-cover.svelte';
import { t } from 'svelte-i18n';

export let album: AlbumResponseDto | undefined;
export let album: AlbumResponseDto;
export let preload = false;
export let css = '';
let className = '';
export { className as class };

$: thumbnailUrl =
album && album.albumThumbnailAssetId ? getAssetThumbnailUrl({ id: album.albumThumbnailAssetId }) : null;
$: alt = album.albumName || $t('unnamed_album');
$: thumbnailUrl = album.albumThumbnailAssetId ? getAssetThumbnailUrl({ id: album.albumThumbnailAssetId }) : null;
</script>

<div class="relative aspect-square">
{#if thumbnailUrl}
<img
loading={preload ? 'eager' : 'lazy'}
src={thumbnailUrl}
alt={album?.albumName ?? $t('unknown_album')}
Snowknight26 marked this conversation as resolved.
Show resolved Hide resolved
class="z-0 rounded-xl object-cover {css}"
data-testid="album-image"
draggable="false"
/>
<AssetCover {alt} class={className} src={thumbnailUrl} {preload} />
{:else}
<enhanced:img
loading={preload ? 'eager' : 'lazy'}
src="$lib/assets/no-thumbnail.png"
sizes="min(271px,186px)"
alt={album?.albumName ?? $t('empty_album')}
class="z-0 rounded-xl object-cover {css}"
data-testid="album-image"
draggable="false"
/>
<NoCover {alt} class={className} {preload} />
{/if}
</div>
2 changes: 1 addition & 1 deletion web/src/lib/components/forms/edit-album-form.svelte
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
<form on:submit|preventDefault={handleUpdateAlbumInfo} autocomplete="off" id="edit-album-form">
<div class="flex items-center">
<div class="hidden sm:flex">
<AlbumCover {album} css="h-[200px] w-[200px] m-4 shadow-lg" />
<AlbumCover {album} class="h-[200px] w-[200px] m-4 shadow-lg" />
</div>

<div class="flex-grow">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,9 +189,8 @@
<div>Let anyone with the link see the selected photo(s)</div>
{:else}
<div class="text-sm">
Individual shared | <span class="text-immich-primary dark:text-immich-dark-primary"
>{editingLink.description || ''}</span
>
{$t('individual_share')} |
<span class="text-immich-primary dark:text-immich-dark-primary">{editingLink.description || ''}</span>
</div>
{/if}
{/if}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import AssetCover from '$lib/components/sharedlinks-page/covers/asset-cover.svelte';
import { render } from '@testing-library/svelte';

describe('AssetCover component', () => {
it('renders correctly', () => {
const component = render(AssetCover, {
alt: '123',
preload: true,
src: 'wee',
class: 'asdf',
});
const img = component.getByTestId('album-image') as HTMLImageElement;
expect(img.alt).toBe('123');
expect(img.getAttribute('src')).toBe('wee');
expect(img.getAttribute('loading')).toBe('eager');
expect(img.className).toBe('z-0 rounded-xl object-cover asdf');
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import NoCover from '$lib/components/sharedlinks-page/covers/no-cover.svelte';
import { render } from '@testing-library/svelte';

describe('NoCover component', () => {
it('renders correctly', () => {
const component = render(NoCover, {
alt: '123',
preload: true,
class: 'asdf',
});
const img = component.getByTestId('album-image') as HTMLImageElement;
expect(img.alt).toBe('123');
expect(img.className).toBe('z-0 rounded-xl object-cover asdf');
expect(img.getAttribute('loading')).toBe('eager');
expect(img.src).toBeTruthy();
Snowknight26 marked this conversation as resolved.
Show resolved Hide resolved
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
import ShareCover from '$lib/components/sharedlinks-page/covers/share-cover.svelte';
import { getAssetThumbnailUrl } from '$lib/utils';
import type { AlbumResponseDto, SharedLinkResponseDto } from '@immich/sdk';
import { render } from '@testing-library/svelte';
import { init, register, waitLocale } from 'svelte-i18n';

vi.mock('$lib/utils');

describe('ShareCover component', () => {
beforeAll(async () => {
await init({ fallbackLocale: 'en-US' });
register('en-US', () => import('$lib/i18n/en-US.json'));
await waitLocale('en-US');
});

it('renders an image when the shared link is an album', () => {
const component = render(ShareCover, {
link: {
album: {
albumName: '123',
} as AlbumResponseDto,
} as SharedLinkResponseDto,
preload: false,
class: 'text',
});
const img = component.getByTestId('album-image') as HTMLImageElement;
expect(img.alt).toBe('123');
expect(img.getAttribute('loading')).toBe('lazy');
expect(img.className).toBe('z-0 rounded-xl object-cover text');
});

it('renders an image when the shared link is an individual share', () => {
vi.mocked(getAssetThumbnailUrl).mockReturnValue('/asdf');
const component = render(ShareCover, {
link: {
assets: [
{
id: 'someId',
},
],
} as SharedLinkResponseDto,
preload: false,
class: 'text',
});
const img = component.getByTestId('album-image') as HTMLImageElement;
expect(img.alt).toBe('Individual share');
expect(img.getAttribute('loading')).toBe('lazy');
expect(img.className).toBe('z-0 rounded-xl object-cover text');
expect(img.getAttribute('src')).toBe('/asdf');
expect(getAssetThumbnailUrl).toHaveBeenCalledWith('someId');
});

it('renders an image when the shared link has no album or assets', () => {
const component = render(ShareCover, {
link: {
assets: [],
} as unknown as SharedLinkResponseDto,
preload: false,
class: 'text',
});
const img = component.getByTestId('album-image') as HTMLImageElement;
expect(img.alt).toBe('Unnamed Share');
expect(img.getAttribute('loading')).toBe('lazy');
expect(img.className).toBe('z-0 rounded-xl object-cover text');
});
});
16 changes: 16 additions & 0 deletions web/src/lib/components/sharedlinks-page/covers/asset-cover.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<script lang="ts">
export let alt;
export let preload = false;
export let src: string;
let className = '';
export { className as class };
</script>

<img
{alt}
class="z-0 rounded-xl object-cover {className}"
data-testid="album-image"
draggable="false"
loading={preload ? 'eager' : 'lazy'}
{src}
/>
16 changes: 16 additions & 0 deletions web/src/lib/components/sharedlinks-page/covers/no-cover.svelte
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO this should be in the asset cover and used if src is undefined. Having an extra component in that case feels a bit weird to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the reasons I went for this approach was to deduplicate its usage. Since the 'undefined' image was referenced in two places, now only one component would have to be updated to ensure it's consistent in both places it's referenced.
I agree it's a little odd to have a component that's nothing but a wrapper for an <enhanced:img> tag.

Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<script lang="ts">
export let alt = '';
export let preload = false;
let className = '';
export { className as class };
</script>

<enhanced:img
{alt}
class="z-0 rounded-xl object-cover {className}"
data-testid="album-image"
draggable="false"
loading={preload ? 'eager' : 'lazy'}
sizes="min(271px,186px)"
src="$lib/assets/no-thumbnail.png"
/>
28 changes: 28 additions & 0 deletions web/src/lib/components/sharedlinks-page/covers/share-cover.svelte
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
<script lang="ts">
import type { SharedLinkResponseDto } from '@immich/sdk';
import AlbumCover from '$lib/components/album-page/album-cover.svelte';
import NoCover from '$lib/components/sharedlinks-page/covers/no-cover.svelte';
import AssetCover from '$lib/components/sharedlinks-page/covers/asset-cover.svelte';
import { getAssetThumbnailUrl } from '$lib/utils';
import { t } from 'svelte-i18n';

export let link: SharedLinkResponseDto;
export let preload = false;
let className = '';
export { className as class };
</script>

<div class="relative aspect-square">
{#if link?.album}
<AlbumCover album={link.album} class={className} {preload} />
{:else if link.assets[0]}
<AssetCover
alt={$t('individual_share')}
class={className}
{preload}
src={getAssetThumbnailUrl(link.assets[0].id)}
/>
{:else}
<NoCover alt={$t('unnamed_share')} class={className} {preload} />
{/if}
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import { createEventDispatcher } from 'svelte';
import CircleIconButton from '../elements/buttons/circle-icon-button.svelte';
import { locale } from '$lib/stores/preferences.store';
import AlbumCover from '$lib/components/album-page/album-cover.svelte';
import ShareCover from '$lib/components/sharedlinks-page/covers/share-cover.svelte';
import { t } from 'svelte-i18n';

export let link: SharedLinkResponseDto;
Expand Down Expand Up @@ -52,7 +52,7 @@
class="flex w-full gap-4 border-b border-gray-200 py-4 transition-all hover:border-immich-primary dark:border-gray-600 dark:text-immich-gray dark:hover:border-immich-dark-primary"
>
<div>
<AlbumCover album={link?.album} css="h-[100px] w-[100px] transition-all duration-300 hover:shadow-lg" />
<ShareCover class="h-[100px] w-[100px] transition-all duration-300 hover:shadow-lg" {link} />
</div>

<div class="flex flex-col justify-between">
Expand Down
3 changes: 2 additions & 1 deletion web/src/lib/i18n/en-US.json
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,6 @@
"editor": "Editor",
"email": "Email",
"empty": "Empty",
"empty_album": "Empty Album",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this not used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was changed to unnamed_share, but if we want the key preserved I can change the string only.

"empty_trash": "Empty trash",
"enable": "Enable",
"enabled": "Enabled",
Expand Down Expand Up @@ -742,6 +741,8 @@
"unknown_year": "Unknown Year",
"unlink_oauth": "Unlink Oauth",
"unlinked_oauth_account": "Unlinked OAuth account",
"unnamed_album": "Unnamed Album",
"unnamed_share": "Unnamed Share",
"unselect_all": "Unselect all",
"unstack": "Un-stack",
"up_next": "Up next",
Expand Down
Loading