Improve giphy UI, use alt text if available

davidje13 · Dec 20, 2024 · 8649a6a · 8649a6a
1 parent d2d1c97
commit 8649a6a
Show file tree

Hide file tree

Showing 12 changed files with 204 additions and 126 deletions.
diff --git a/backend/src/api-tests/giphy.test.ts b/backend/src/api-tests/giphy.test.ts
@@ -2,30 +2,40 @@ import request from 'superwstest';
 import { WebSocketExpress } from 'websocket-express';
 import { testConfig } from './testConfig';
 import { addressToString, testServerRunner } from './testServerRunner';
+import type { GiphyResponse } from '../services/GiphyService';
 import { appFactory } from '../app';
 
 describe('API giphy', () => {
   const MOCK_GIPHY = testServerRunner(async () => {
     const giphyApp = new WebSocketExpress();
-    giphyApp.use(WebSocketExpress.urlencoded({ extended: false }));
-    giphyApp.get('/gifs/search', (_, res) => {
-      res.json({
-        status: 200,
-        data: [
-          {
-            images: {
-              original: { url: 'original.gif?extra' },
-              fixed_height: { url: 'medium.gif?extra' },
-              fixed_height_small: { url: 'small.gif?extra' },
+    const response: GiphyResponse = {
+      meta: { status: 200 },
+      data: [
+        {
+          alt_text: 'An image',
+          images: {
+            original: {
+              url: 'http://example.com/original.gif?extra',
+              webp: 'http://example.com/original.webp?extra',
             },
-          },
-          {
-            images: {
-              original: { url: 'original2.gif' },
+            fixed_height: {
+              url: 'http://example.com/medium.gif?extra',
+              webp: 'http://example.com/medium.webp?extra',
             },
+            fixed_height_small: { url: 'http://example.com/small.gif?extra' },
           },
-        ],
-      });
+        },
+        {
+          images: {
+            original: { url: 'http://example.com/original2.gif' },
+          },
+        },
+      ],
+      pagination: {},
+    };
+    giphyApp.use(WebSocketExpress.urlencoded({ extended: false }));
+    giphyApp.get('/gifs/search', (_, res) => {
+      res.json(response);
     });
 
     return { run: giphyApp.createServer() };
@@ -55,8 +65,15 @@ describe('API giphy', () => {
 
       expect(response.body).toEqual({
         gifs: [
-          { small: 'small.gif', medium: 'medium.gif' },
-          { small: 'original2.gif', medium: 'original2.gif' },
+          {
+            small: 'http://example.com/small.gif',
+            medium: 'http://example.com/medium.webp',
+            alt: 'An image',
+          },
+          {
+            small: 'http://example.com/original2.gif',
+            medium: 'http://example.com/original2.gif',
+          },
         ],
       });
     });

diff --git a/backend/src/export/RetroJsonExport.ts b/backend/src/export/RetroJsonExport.ts
@@ -12,6 +12,7 @@ type MaybeAsyncIterable<T> = Iterable<T> | AsyncIterable<T>;
 export interface RetroItemAttachmentJsonExport {
   type: string;
   url: string;
+  alt?: string | undefined;
 }
 
 export interface RetroItemJsonExport {
@@ -57,6 +58,7 @@ function exportRetroItemAttachment(
   return {
     type: attachment.type,
     url: attachment.url,
+    alt: attachment.alt,
   };
 }
 
@@ -66,6 +68,7 @@ function importRetroItemAttachment(
   return {
     type: attachment.type,
     url: attachment.url,
+    alt: attachment.alt,
   };
 }
 

diff --git a/backend/src/helpers/exportedJsonParsers.ts b/backend/src/helpers/exportedJsonParsers.ts
@@ -28,6 +28,7 @@ export const extractExportedRetroItem = json.object<RetroItemJsonExport>({
     json.object<RetroItemAttachmentJsonExport>({
       type: json.string,
       url: json.string,
+      alt: json.optional(json.string),
     }),
   ),
 });

diff --git a/backend/src/helpers/jsonParsers.ts b/backend/src/helpers/jsonParsers.ts
@@ -15,6 +15,7 @@ export const extractRetroItem = json.exactObject<RetroItem>({
     json.exactObject<RetroItemAttachment>({
       type: json.string,
       url: json.string,
+      alt: json.optional(json.string),
     }),
   ),
   votes: json.number,

diff --git a/backend/src/routers/ApiGiphyRouter.ts b/backend/src/routers/ApiGiphyRouter.ts
@@ -10,20 +10,20 @@ export class ApiGiphyRouter extends Router {
     this.get(
       '/search',
       safe(async (req, res) => {
-        const { q, lang = 'en' } = req.query;
+        const { q, lang } = req.query;
 
         if (typeof q !== 'string' || !q) {
           res.status(400).json({ error: 'Bad request' });
           return;
         }
 
-        if (typeof lang !== 'string') {
+        if (typeof lang !== 'string' && lang !== undefined) {
           res.status(400).json({ error: 'Bad request' });
           return;
         }
 
         try {
-          const gifs = await service.search(q, 10, lang);
+          const gifs = await service.search(q, 0, 50, lang);
 
           res.json({ gifs });
         } catch (err) {

diff --git a/backend/src/services/GiphyService.ts b/backend/src/services/GiphyService.ts
@@ -8,32 +8,40 @@ interface Config {
 interface GifInfo {
   small: string;
   medium: string;
+  alt: string | undefined;
 }
 
 interface GiphyResponseResource {
   url?: string;
+  webp?: string;
 }
 
 interface GiphyResponseGif {
+  alt_text?: string;
   images: {
     original?: GiphyResponseResource;
     fixed_height?: GiphyResponseResource;
     fixed_height_small?: GiphyResponseResource;
+    fixed_height_downsampled?: GiphyResponseResource;
+    fixed_width?: GiphyResponseResource;
+    fixed_width_small?: GiphyResponseResource;
+    fixed_width_downsampled?: GiphyResponseResource;
   };
 }
 
-interface GiphyResponse {
-  status: number;
-  data: ReadonlyArray<GiphyResponseGif>;
+export interface GiphyResponse {
+  meta: { status: number };
+  data: GiphyResponseGif[];
+  pagination: { total_count?: number };
 }
 
 export class GiphyService {
   private readonly baseUrl: string;
 
   private readonly apiKey: string;
 
-  // memory used = ~120 bytes per gif entry * max limit * max cache size
-  private readonly searchCache = new LruCache<string, GifInfo[]>(1024);
+  // memory used = ~200 bytes per gif entry * max limit * cache size
+  private readonly searchCache = new LruCache<string, GifInfo[]>(256);
 
   public constructor(config: Config) {
     this.baseUrl = config.baseUrl;
@@ -42,49 +50,68 @@ export class GiphyService {
 
   public async search(
     query: string,
+    offset: number,
     limit: number,
-    lang = 'en',
+    lang?: string | undefined,
   ): Promise<GifInfo[]> {
-    if (!this.apiKey || !query || limit <= 0) {
+    if (!this.apiKey || !query || offset < 0 || limit <= 0) {
       return [];
     }
 
-    const cached = await this.searchCache.cachedAsync(
-      `${lang}:${query}`,
-      async (): Promise<GifInfo[]> => {
-        const params = new URLSearchParams();
-        params.append('api_key', this.apiKey);
-        params.append('q', query);
-        params.append('limit', String(limit));
-        params.append('rating', 'g');
-        params.append('lang', lang);
-        const result = await fetch(
-          `${this.baseUrl}/gifs/search?${params.toString()}`,
-        );
-        const resultJson = (await result.json()) as GiphyResponse;
-
-        if (resultJson.status === 400) {
-          throw new Error('Giphy API returned Bad Request');
-        } else if (resultJson.status === 403) {
-          throw new Error('Giphy API key rejected');
-        } else if (resultJson.status === 429) {
-          throw new Error('Giphy API rate limit reached');
-        } else if (resultJson.status >= 400) {
-          throw new Error(`Unknown Giphy API response: ${resultJson.status}`);
-        }
-
-        return resultJson.data.map((gif) => {
-          const original = gif.images.original?.url ?? '';
-          const fixed = gif.images.fixed_height?.url ?? original;
-          const small = gif.images.fixed_height_small?.url ?? fixed;
-          return {
-            small: small.split('?')[0] ?? '',
-            medium: fixed.split('?')[0] ?? '',
-          };
-        });
-      },
-      (c) => c.length >= limit,
-    );
-    return cached.slice(0, limit);
+    const cacheKey = `${lang}:${query}:${offset}:${limit}`;
+
+    return this.searchCache.cachedAsync(cacheKey, async () => {
+      const params = new URLSearchParams({
+        api_key: this.apiKey,
+        q: query,
+        offset: String(offset),
+        limit: String(limit),
+        rating: 'g',
+        bundle: 'messaging_non_clips',
+      });
+      if (lang) {
+        params.set('lang', lang);
+      }
+      const result = await fetch(`${this.baseUrl}/gifs/search?${params}`);
+      const resultJson = (await result.json()) as GiphyResponse;
+      const status = resultJson.meta?.status || result.status;
+
+      if (status === 400) {
+        throw new Error('Giphy API returned Bad Request');
+      } else if (status === 403) {
+        throw new Error('Giphy API key rejected');
+      } else if (status === 429) {
+        throw new Error('Giphy API rate limit reached');
+      } else if (status >= 400) {
+        throw new Error(`Unknown Giphy API response: ${status}`);
+      }
+
+      return resultJson.data.map((gif) => {
+        const original = getResourceURL(gif.images.original);
+        const medium = getResourceURL(gif.images.fixed_height);
+        const small = getResourceURL(gif.images.fixed_height_small);
+        return {
+          small: trimQuery(small ?? medium ?? original) ?? '',
+          medium: trimQuery(medium ?? original ?? small) ?? '',
+          alt: gif.alt_text || undefined,
+        };
+      });
+    });
+  }
+}
+
+const getResourceURL = (image: GiphyResponseResource | undefined) =>
+  image?.webp ?? image?.url;
+
+function trimQuery(href: string | undefined) {
+  if (!href) {
+    return undefined;
+  }
+  try {
+    const url = new URL(href);
+    url.search = '';
+    return url.toString();
+  } catch {
+    return href.split('?')[0] ?? '';
   }
 }
diff --git a/backend/src/shared/api-entities.ts b/backend/src/shared/api-entities.ts
@@ -19,6 +19,7 @@ export interface ClientConfig {
 export interface RetroItemAttachment {
   type: string;
   url: string;
+  alt?: string | undefined;
 }
 
 export interface UserProvidedRetroItemDetails {

diff --git a/frontend/src/api/GiphyService.ts b/frontend/src/api/GiphyService.ts
@@ -3,20 +3,28 @@ import { jsonFetch } from './jsonFetch';
 export interface GifInfo {
   small: string;
   medium: string;
+  alt?: string;
 }
 
 export class GiphyService {
   public constructor(private readonly apiBase: string) {}
 
-  public async search(query: string, signal: AbortSignal): Promise<GifInfo[]> {
+  public async search(
+    query: string,
+    lang: string | undefined,
+    signal: AbortSignal,
+  ): Promise<GifInfo[]> {
     const normedQuery = query.trim();
     if (!normedQuery) {
       return [];
     }
 
     const params = new URLSearchParams({ q: normedQuery });
+    if (lang) {
+      params.set('lang', lang);
+    }
     const body = await jsonFetch<{ gifs: GifInfo[] }>(
-      `${this.apiBase}/giphy/search?${params.toString()}`,
+      `${this.apiBase}/giphy/search?${params}`,
       { signal },
     );
     return body.gifs;

diff --git a/frontend/src/components/attachments/giphy/GiphyAttachment.tsx b/frontend/src/components/attachments/giphy/GiphyAttachment.tsx
@@ -5,11 +5,12 @@ interface PropsT {
   attachment: RetroItemAttachment;
 }
 
-export const GiphyAttachment = memo(({ attachment: { url } }: PropsT) => (
+export const GiphyAttachment = memo(({ attachment: { url, alt } }: PropsT) => (
   <figure>
     <img
       src={url}
-      alt="Attachment"
+      alt={alt}
+      title={alt}
       crossOrigin="anonymous"
       referrerPolicy="no-referrer"
     />