feat(web): improve alt text (#7596)

* alt text * memory lane alt text * revert sql generator change * use getAltText * oops * handle large number of people in asset * nit * add aria-label to search button * update api * fixed tests * fixed typing * fixed spacing * fix displaying null
2024-11-24 08:52:28 +02:00 · 2024-03-03 16:42:17 -05:00 · 2024-03-03 16:42:17 -05:00 · 2fa10a254c
commit 2fa10a254c
parent 07c926bb12
24 changed files with 143 additions and 51 deletions
--- a/mobile/openapi/doc/MapMarkerResponseDto.md
+++ b/mobile/openapi/doc/MapMarkerResponseDto.md
--- a/mobile/openapi/lib/model/map_marker_response_dto.dart
+++ b/mobile/openapi/lib/model/map_marker_response_dto.dart
--- a/mobile/openapi/test/map_marker_response_dto_test.dart
+++ b/mobile/openapi/test/map_marker_response_dto_test.dart
--- a/open-api/immich-openapi-specs.json
+++ b/open-api/immich-openapi-specs.json
@ -8291,6 +8291,14 @@
      },
      "MapMarkerResponseDto": {
        "properties": {
+          "city": {
+            "nullable": true,
+            "type": "string"
+          },
+          "country": {
+            "nullable": true,
+            "type": "string"
+          },
          "id": {
            "type": "string"
          },
@ -8301,12 +8309,19 @@
          "lon": {
            "format": "double",
            "type": "number"
+          },
+          "state": {
+            "nullable": true,
+            "type": "string"
          }
        },
        "required": [
+          "city",
+          "country",
          "id",
          "lat",
-          "lon"
+          "lon",
+          "state"
        ],
        "type": "object"
      },
--- a/open-api/typescript-sdk/src/fetch-client.ts
+++ b/open-api/typescript-sdk/src/fetch-client.ts
@ -260,9 +260,12 @@ export type AssetJobsDto = {
    name: AssetJobName;
 };
 export type MapMarkerResponseDto = {
+    city: string | null;
+    country: string | null;
    id: string;
    lat: number;
    lon: number;
+    state: string | null;
 };
 export type MemoryLaneResponseDto = {
    assets: AssetResponseDto[];
--- a/server/src/domain/asset/asset.service.spec.ts
+++ b/server/src/domain/asset/asset.service.spec.ts
@ -286,27 +286,22 @@ describe(AssetService.name, () => {

  describe('getMapMarkers', () => {
    it('should get geo information of assets', async () => {
+      const asset = assetStub.withLocation;
+      const marker = {
+        id: asset.id,
+        lat: asset.exifInfo!.latitude!,
+        lon: asset.exifInfo!.longitude!,
+        city: asset.exifInfo!.city,
+        state: asset.exifInfo!.state,
+        country: asset.exifInfo!.country,
+      };
      partnerMock.getAll.mockResolvedValue([]);
-      assetMock.getMapMarkers.mockResolvedValue(
-        [assetStub.withLocation].map((asset) => ({
-          id: asset.id,
-
-          /* eslint-disable-next-line @typescript-eslint/no-non-null-assertion */
-          lat: asset.exifInfo!.latitude!,
-
-          /* eslint-disable-next-line @typescript-eslint/no-non-null-assertion */
-          lon: asset.exifInfo!.longitude!,
-        })),
-      );
+      assetMock.getMapMarkers.mockResolvedValue([marker]);

      const markers = await sut.getMapMarkers(authStub.user1, {});

      expect(markers).toHaveLength(1);
-      expect(markers[0]).toEqual({
-        id: assetStub.withLocation.id,
-        lat: 100,
-        lon: 100,
-      });
+      expect(markers[0]).toEqual(marker);
    });
  });

--- a/server/src/domain/asset/response-dto/map-marker-response.dto.ts
+++ b/server/src/domain/asset/response-dto/map-marker-response.dto.ts
@ -9,4 +9,13 @@ export class MapMarkerResponseDto {

  @ApiProperty({ format: 'double' })
  lon!: number;
+
+  @ApiProperty()
+  city!: string | null;
+
+  @ApiProperty()
+  state!: string | null;
+
+  @ApiProperty()
+  country!: string | null;
 }
--- a/server/src/domain/repositories/asset.repository.ts
+++ b/server/src/domain/repositories/asset.repository.ts
@ -1,4 +1,9 @@
-import { AssetSearchOneToOneRelationOptions, AssetSearchOptions, SearchExploreItem } from '@app/domain';
+import {
+  AssetSearchOneToOneRelationOptions,
+  AssetSearchOptions,
+  ReverseGeocodeResult,
+  SearchExploreItem,
+} from '@app/domain';
 import { AssetEntity, AssetJobStatusEntity, AssetType, ExifEntity } from '@app/infra/entities';
 import { FindOptionsRelations, FindOptionsSelect } from 'typeorm';
 import { Paginated, PaginationOptions } from '../domain.util';
@ -25,7 +30,7 @@ export interface MapMarkerSearchOptions {
  fileCreatedAfter?: Date;
 }

-export interface MapMarker {
+export interface MapMarker extends ReverseGeocodeResult {
  id: string;
  lat: number;
  lon: number;
--- a/server/src/infra/repositories/asset.repository.ts
+++ b/server/src/infra/repositories/asset.repository.ts
@ -507,6 +507,9 @@ export class AssetRepository implements IAssetRepository {
      select: {
        id: true,
        exifInfo: {
+          city: true,
+          state: true,
+          country: true,
          latitude: true,
          longitude: true,
        },
@ -532,12 +535,11 @@ export class AssetRepository implements IAssetRepository {

    return assets.map((asset) => ({
      id: asset.id,
-
-      /* eslint-disable-next-line @typescript-eslint/no-non-null-assertion */
      lat: asset.exifInfo!.latitude!,
-
-      /* eslint-disable-next-line @typescript-eslint/no-non-null-assertion */
      lon: asset.exifInfo!.longitude!,
+      city: asset.exifInfo!.city,
+      state: asset.exifInfo!.state,
+      country: asset.exifInfo!.country,
    }));
  }

--- a/server/test/fixtures/asset.stub.ts
+++ b/server/test/fixtures/asset.stub.ts
@ -482,6 +482,9 @@ export const assetStub = {
      latitude: 100,
      longitude: 100,
      fileSizeInByte: 23_456,
+      city: 'test-city',
+      state: 'test-state',
+      country: 'test-country',
    } as ExifEntity,
    deletedAt: null,
  }),
--- a/web/src/lib/components/album-page/tests/album-card.spec.ts
+++ b/web/src/lib/components/album-page/tests/album-card.spec.ts
@ -47,11 +47,11 @@ describe('AlbumCard component', () => {
    const detailsText = `${count} items` + (shared ? ' . Shared' : '');

    expect(albumImgElement).toHaveAttribute('src');
-    expect(albumImgElement).toHaveAttribute('alt', album.id);
+    expect(albumImgElement).toHaveAttribute('alt', album.albumName);

    await waitFor(() => expect(albumImgElement).toHaveAttribute('src'));

-    expect(albumImgElement).toHaveAttribute('alt', album.id);
+    expect(albumImgElement).toHaveAttribute('alt', album.albumName);
    expect(sdkMock.getAssetThumbnail).not.toHaveBeenCalled();

    expect(albumNameElement).toHaveTextContent(album.albumName);
@ -74,11 +74,11 @@ describe('AlbumCard component', () => {
    const albumImgElement = sut.getByTestId('album-image');
    const albumNameElement = sut.getByTestId('album-name');
    const albumDetailsElement = sut.getByTestId('album-details');
-    expect(albumImgElement).toHaveAttribute('alt', album.id);
+    expect(albumImgElement).toHaveAttribute('alt', album.albumName);

    await waitFor(() => expect(albumImgElement).toHaveAttribute('src', thumbnailUrl));

-    expect(albumImgElement).toHaveAttribute('alt', album.id);
+    expect(albumImgElement).toHaveAttribute('alt', album.albumName);
    expect(sdkMock.getAssetThumbnail).toHaveBeenCalledTimes(1);
    expect(sdkMock.getAssetThumbnail).toHaveBeenCalledWith({
      id: 'thumbnailIdOne',
--- a/web/src/lib/components/album-page/album-card.svelte
+++ b/web/src/lib/components/album-page/album-card.svelte
@ -72,7 +72,7 @@
      <img
        loading={preload ? 'eager' : 'lazy'}
        src={imageData}
-        alt={album.id}
+        alt={album.albumName}
        class="z-0 h-full w-full rounded-xl object-cover transition-all duration-300 hover:shadow-lg"
        data-testid="album-image"
        draggable="false"
@ -82,7 +82,7 @@
        loading={preload ? 'eager' : 'lazy'}
        src="$lib/assets/no-thumbnail.png"
        sizes="min(271px,186px)"
-        alt={album.id}
+        alt={album.albumName}
        class="z-0 h-full w-full rounded-xl object-cover transition-all duration-300 hover:shadow-lg"
        data-testid="album-image"
        draggable="false"
--- a/web/src/lib/components/asset-viewer/activity-viewer.svelte
+++ b/web/src/lib/components/asset-viewer/activity-viewer.svelte
@ -195,7 +195,7 @@
                  <img
                    class="rounded-lg w-[75px] h-[75px] object-cover"
                    src={getAssetThumbnailUrl(reaction.assetId, ThumbnailFormat.Webp)}
-                    alt="comment-thumbnail"
+                    alt="Profile picture of {reaction.user.name}, who commented on this asset"
                  />
                </div>
              {/if}
@ -241,7 +241,7 @@
                    <img
                      class="rounded-lg w-[75px] h-[75px] object-cover"
                      src={getAssetThumbnailUrl(reaction.assetId, ThumbnailFormat.Webp)}
-                      alt="like-thumbnail"
+                      alt="Profile picture of {reaction.user.name}, who liked this asset"
                    />
                  </div>
                {/if}
--- a/web/src/lib/components/asset-viewer/detail-panel.svelte
+++ b/web/src/lib/components/asset-viewer/detail-panel.svelte
@ -616,7 +616,16 @@
    {:then component}
      <svelte:component
        this={component.default}
-        mapMarkers={[{ lat: latlng.lat, lon: latlng.lng, id: asset.id }]}
+        mapMarkers={[
+          {
+            lat: latlng.lat,
+            lon: latlng.lng,
+            id: asset.id,
+            city: asset.exifInfo?.city ?? null,
+            state: asset.exifInfo?.state ?? null,
+            country: asset.exifInfo?.country ?? null,
+          },
+        ]}
        center={latlng}
        zoom={15}
        simplified
--- a/web/src/lib/components/asset-viewer/photo-viewer.svelte
+++ b/web/src/lib/components/asset-viewer/photo-viewer.svelte
@ -13,6 +13,7 @@
  import { fade } from 'svelte/transition';
  import LoadingSpinner from '../shared-components/loading-spinner.svelte';
  import { NotificationType, notificationController } from '../shared-components/notification/notification';
+  import { getAltText } from '$lib/utils/thumbnail-util';

  export let asset: AssetResponseDto;
  export let element: HTMLDivElement | undefined = undefined;
@ -133,7 +134,7 @@
        bind:this={$photoViewer}
        transition:fade={{ duration: haveFadeTransition ? 150 : 0 }}
        src={assetData}
-        alt={asset.id}
+        alt={getAltText(asset)}
        class="h-full w-full object-contain"
        draggable="false"
      />
--- a/web/src/lib/components/assets/thumbnail/image-thumbnail.svelte
+++ b/web/src/lib/components/assets/thumbnail/image-thumbnail.svelte
@ -8,7 +8,7 @@
  import Icon from '$lib/components/elements/icon.svelte';

  export let url: string;
-  export let altText: string;
+  export let altText: string | undefined;
  export let title: string | null = null;
  export let heightStyle: string | undefined = undefined;
  export let widthStyle: string;
--- a/web/src/lib/components/assets/thumbnail/thumbnail.svelte
+++ b/web/src/lib/components/assets/thumbnail/thumbnail.svelte
@ -3,6 +3,7 @@
  import Icon from '$lib/components/elements/icon.svelte';
  import { ProjectionType } from '$lib/constants';
  import { getAssetFileUrl, getAssetThumbnailUrl, isSharedLink } from '$lib/utils';
+  import { getAltText } from '$lib/utils/thumbnail-util';
  import { timeToSeconds } from '$lib/utils/date-time';
  import { AssetTypeEnum, ThumbnailFormat, type AssetResponseDto } from '@immich/sdk';
  import {
@ -177,7 +178,7 @@
        {#if asset.resized}
          <ImageThumbnail
            url={getAssetThumbnailUrl(asset.id, format)}
-            altText={asset.originalFileName}
+            altText={getAltText(asset)}
            widthStyle="{width}px"
            heightStyle="{height}px"
            thumbhash={asset.thumbhash}
--- a/web/src/lib/components/faces-page/person-side-panel.svelte
+++ b/web/src/lib/components/faces-page/person-side-panel.svelte
@ -223,14 +223,14 @@
                    url={selectedPersonToCreate[index] ||
                      getPeopleThumbnailUrl(selectedPersonToReassign[index]?.id || face.person.id)}
                    altText={selectedPersonToReassign[index]
-                      ? selectedPersonToReassign[index]?.name || ''
+                      ? selectedPersonToReassign[index]?.name
                      : selectedPersonToCreate[index]
-                        ? 'new person'
+                        ? 'New person'
                        : getPersonNameWithHiddenValue(face.person?.name, face.person?.isHidden)}
                    title={selectedPersonToReassign[index]
-                      ? selectedPersonToReassign[index]?.name || ''
+                      ? selectedPersonToReassign[index]?.name
                      : selectedPersonToCreate[index]
-                        ? 'new person'
+                        ? 'New person'
                        : getPersonNameWithHiddenValue(face.person?.name, face.person?.isHidden)}
                    widthStyle="90px"
                    heightStyle="90px"
--- a/web/src/lib/components/memory-page/memory-viewer.svelte
+++ b/web/src/lib/components/memory-page/memory-viewer.svelte
@ -171,7 +171,7 @@
              <img
                class="h-full w-full rounded-2xl object-cover"
                src={getAssetThumbnailUrl(previousMemory.assets[0].id, ThumbnailFormat.Jpeg)}
-                alt=""
+                alt="Previous memory"
                draggable="false"
              />
            {:else}
@ -179,7 +179,7 @@
                class="h-full w-full rounded-2xl object-cover"
                src="$lib/assets/no-thumbnail.png"
                sizes="min(271px,186px)"
-                alt=""
+                alt="Previous memory"
                draggable="false"
              />
            {/if}
@ -203,7 +203,7 @@
                transition:fade
                class="h-full w-full rounded-2xl object-contain transition-all"
                src={getAssetThumbnailUrl(currentAsset.id, ThumbnailFormat.Jpeg)}
-                alt=""
+                alt={currentAsset.exifInfo?.description}
                draggable="false"
              />
            {/key}
@ -244,7 +244,7 @@
              <img
                class="h-full w-full rounded-2xl object-cover"
                src={getAssetThumbnailUrl(nextMemory.assets[0].id, ThumbnailFormat.Jpeg)}
-                alt=""
+                alt="Next memory"
                draggable="false"
              />
            {:else}
@ -252,7 +252,7 @@
                class="h-full w-full rounded-2xl object-cover"
                src="$lib/assets/no-thumbnail.png"
                sizes="min(271px,186px)"
-                alt=""
+                alt="Next memory"
                draggable="false"
              />
            {/if}
--- a/web/src/lib/components/photos-page/memory-lane.svelte
+++ b/web/src/lib/components/photos-page/memory-lane.svelte
@ -4,6 +4,7 @@
  import { AppRoute, QueryParameter } from '$lib/constants';
  import { memoryStore } from '$lib/stores/memory.store';
  import { getAssetThumbnailUrl } from '$lib/utils';
+  import { getAltText } from '$lib/utils/thumbnail-util';
  import { ThumbnailFormat, getMemoryLane } from '@immich/sdk';
  import { mdiChevronLeft, mdiChevronRight } from '@mdi/js';
  import { onMount } from 'svelte';
@ -64,7 +65,6 @@
        {/if}
      </div>
    {/if}
-
    <div class="inline-block" bind:offsetWidth={innerWidth}>
      {#each $memoryStore as memory, index (memory.title)}
        <button
@ -74,7 +74,7 @@
          <img
            class="h-full w-full rounded-xl object-cover"
            src={getAssetThumbnailUrl(memory.assets[0].id, ThumbnailFormat.Jpeg)}
-            alt={memory.title}
+            alt={`Memory Lane ${getAltText(memory.assets[0])}`}
            draggable="false"
          />
          <p class="absolute bottom-2 left-4 z-10 text-lg text-white">{memory.title}</p>
--- a/web/src/lib/components/shared-components/change-location.svelte
+++ b/web/src/lib/components/shared-components/change-location.svelte
@ -192,7 +192,18 @@
      {:then component}
        <svelte:component
          this={component.default}
-          mapMarkers={lat && lng && asset ? [{ id: asset.id, lat, lon: lng }] : []}
+          mapMarkers={lat && lng && asset
+            ? [
+                {
+                  id: asset.id,
+                  lat,
+                  lon: lng,
+                  city: asset.exifInfo?.city ?? null,
+                  state: asset.exifInfo?.state ?? null,
+                  country: asset.exifInfo?.country ?? null,
+                },
+              ]
+            : []}
          {zoom}
          bind:addClipMapMarker
          center={lat && lng ? { lat, lng } : undefined}
--- a/web/src/lib/components/shared-components/map/map.svelte
+++ b/web/src/lib/components/shared-components/map/map.svelte
@ -88,7 +88,7 @@
    }
  }

-  type FeaturePoint = Feature<Point, { id: string }>;
+  type FeaturePoint = Feature<Point, { id: string; city: string | null; state: string | null; country: string | null }>;

  const asFeature = (marker: MapMarkerResponseDto): FeaturePoint => {
    return {
@ -96,6 +96,9 @@
      geometry: { type: 'Point', coordinates: [marker.lon, marker.lat] },
      properties: {
        id: marker.id,
+        city: marker.city,
+        state: marker.state,
+        country: marker.country,
      },
    };
  };
@ -107,6 +110,9 @@
      lat: coords.lat,
      lon: coords.lng,
      id: featurePoint.properties.id,
+      city: featurePoint.properties.city,
+      state: featurePoint.properties.state,
+      country: featurePoint.properties.country,
    };
  };
 </script>
@ -178,7 +184,9 @@
          <img
            src={getAssetThumbnailUrl(feature.properties?.id, undefined)}
            class="rounded-full w-[60px] h-[60px] border-2 border-immich-primary shadow-lg hover:border-immich-dark-primary transition-all duration-200 hover:scale-150 object-cover bg-immich-primary"
-            alt={`Image with id ${feature.properties?.id}`}
+            alt={feature.properties?.city && feature.properties.country
+              ? `Map marker for images taken in ${feature.properties.city}, ${feature.properties.country}`
+              : 'Map marker with image'}
          />
        {/if}
        {#if $$slots.popup}
--- a/web/src/lib/components/shared-components/search-bar/search-bar.svelte
+++ b/web/src/lib/components/shared-components/search-bar/search-bar.svelte
@ -97,7 +97,7 @@
      <div class="absolute inset-y-0 left-0 flex items-center pl-6">
        <div class="dark:text-immich-dark-fg/75">
          <button class="flex items-center">
-            <Icon path={mdiMagnify} size="1.5em" />
+            <Icon ariaLabel="search" path={mdiMagnify} size="1.5em" />
          </button>
        </div>
      </div>
@ -134,7 +134,7 @@
          type="reset"
          class="rounded-full p-2 hover:bg-immich-primary/5 active:bg-immich-primary/10 dark:text-immich-dark-fg/75 dark:hover:bg-immich-dark-primary/25 dark:active:bg-immich-dark-primary/[.35]"
        >
-          <Icon path={mdiClose} size="1.5em" />
+          <Icon ariaLabel="clear" path={mdiClose} size="1.5em" />
        </button>
      </div>
    {/if}
--- a/web/src/lib/utils/thumbnail-util.ts
+++ b/web/src/lib/utils/thumbnail-util.ts
@ -1,3 +1,6 @@
+import type { AssetResponseDto } from '@immich/sdk';
+import { fromLocalDateTime } from './timeline-util';
+
 /**
 * Calculate thumbnail size based on number of assets and viewport width
 * @param assetCount Number of assets in the view
@ -31,3 +34,30 @@ export function getThumbnailSize(assetCount: number, viewWidth: number): number

  return 300;
 }
+
+export function getAltText(asset: AssetResponseDto) {
+  if (asset.exifInfo?.description) {
+    return asset.exifInfo.description;
+  }
+
+  let altText = 'Image taken';
+  if (asset.exifInfo?.city && asset.exifInfo.country) {
+    altText += ` in ${asset.exifInfo.city}, ${asset.exifInfo.country}`;
+  }
+
+  const names = asset.people?.filter((p) => p.name).map((p) => p.name) ?? [];
+  if (names.length == 1) {
+    altText += ` with ${names[0]}`;
+  }
+  if (names.length > 1 && names.length <= 3) {
+    altText += ` with ${names.slice(0, -1).join(', ')} and ${names.at(-1)}`;
+  }
+  if (names.length > 3) {
+    altText += ` with ${names.slice(0, 2).join(', ')}, and ${names.length - 2} others`;
+  }
+
+  const date = fromLocalDateTime(asset.localDateTime).toLocaleString({ dateStyle: 'long' });
+  altText += ` on ${date}`;
+
+  return altText;
+}