From 2041c5df1e9d5d1f9f37e7c177f2e70f61ea5dba Mon Sep 17 00:00:00 2001
From: sarayourfriend <24264157+sarayourfriend@users.noreply.github.com>
Date: Thu, 18 May 2023 05:07:45 +1000
Subject: [PATCH] Add media type context to start of result link titles (#1920)

Co-authored-by: Olga Bulat <obulat@gmail.com>
Co-authored-by: Zack Krida <zackkrida@pm.me>
---
 frontend/docker-compose.playwright.yml        |  1 +
 frontend/package.json                         |  4 ++-
 .../components/VAudioTrack/VAudioTrack.vue    |  2 +-
 .../components/VContentLink/VContentLink.vue  |  2 +-
 .../VSearchResultsGrid/VImageCell.vue         | 13 ++++++--
 frontend/src/locales/scripts/en.json5         |  8 +++--
 .../e2e/all-results-keyboard.spec.ts          | 17 +++++-----
 frontend/test/playwright/playwright.config.ts | 31 +++++++++----------
 8 files changed, 45 insertions(+), 33 deletions(-)

diff --git a/frontend/docker-compose.playwright.yml b/frontend/docker-compose.playwright.yml
index 01e1f3cf33b..0ed5435abad 100644
--- a/frontend/docker-compose.playwright.yml
+++ b/frontend/docker-compose.playwright.yml
@@ -18,4 +18,5 @@ services:
       - DEBUG=pw:webserver
       - DISABLE_SENTRY=true
       - UPDATE_TAPES=${UPDATE_TAPES:-false}
+      - FASTSTART=${FASTSTART:-false}
     cpus: 0.000
diff --git a/frontend/package.json b/frontend/package.json
index e2c031b9e9f..a59e595f2f2 100644
--- a/frontend/package.json
+++ b/frontend/package.json
@@ -16,6 +16,7 @@
     "docker:run": "docker run --rm -it -p 127.0.0.1:8443:8443/tcp openverse-frontend:latest",
     "generate": "nuxt generate",
     "start": "nuxt start",
+    "start:playwright": "pnpm i18n:copy-test-locales && pnpm start",
     "prod": "pnpm build:only && pnpm start",
     "prod:playwright": "pnpm i18n:copy-test-locales && pnpm prod",
     "storybook": "nuxt storybook --port=54000",
@@ -29,11 +30,12 @@
     "test:unit": "jest",
     "test:unit:watch": "pnpm test:unit --collectCoverage=false --watch",
     "test:playwright": "./bin/playwright.sh",
-    "test:playwright:local": "DISABLE_SENTRY=true playwright test -c test/playwright",
+    "test:playwright:local": "DISABLE_SENTRY=true pnpm exec playwright test -c test/playwright",
     "test:playwright:debug": "PWDEBUG=1 pnpm test:playwright:local",
     "test:playwright:recreate-tapes": "rimraf test/tapes && pnpm test:playwright:update-tapes",
     "test:playwright:update-tapes": "UPDATE_TAPES=true pnpm test:playwright",
     "test:playwright:gen": "playwright codegen localhost:8443/",
+    "test:playwright:faststart": "FASTSTART=true pnpm test:playwright",
     "test:storybook": "TEST_COMMAND=test:storybook:local ./bin/playwright.sh",
     "test:storybook:local": "playwright test -c test/storybook",
     "test:storybook:debug": "PWDEBUG=1 pnpm test:storybook:local",
diff --git a/frontend/src/components/VAudioTrack/VAudioTrack.vue b/frontend/src/components/VAudioTrack/VAudioTrack.vue
index 290bca51d9d..a56fec87690 100644
--- a/frontend/src/components/VAudioTrack/VAudioTrack.vue
+++ b/frontend/src/components/VAudioTrack/VAudioTrack.vue
@@ -470,7 +470,7 @@ export default defineComponent({
     )
     const ariaLabel = computed(() =>
       isComposite.value
-        ? i18n.t("audio-track.aria-label-interactive", {
+        ? i18n.t("audio-track.aria-label-interactive-seekable", {
             title: props.audio.title,
           })
         : i18n.t("audio-track.aria-label", { title: props.audio.title })
diff --git a/frontend/src/components/VContentLink/VContentLink.vue b/frontend/src/components/VContentLink/VContentLink.vue
index 600bc337eaf..e800d60c3de 100644
--- a/frontend/src/components/VContentLink/VContentLink.vue
+++ b/frontend/src/components/VContentLink/VContentLink.vue
@@ -1,5 +1,5 @@
 <template>
-  <!-- We 'disable' the link when there are 0 results by removing the href and setting aria-disabled. -->
+  <!-- VLink handles rendering a disabled element when `href` is undefined. -->
   <VLink
     :href="hasResults ? to : undefined"
     class="flex w-full flex-col items-start overflow-hidden rounded-sm border border-dark-charcoal/20 bg-white py-4 pe-12 ps-4 md:flex-row md:items-center md:justify-between md:p-6"
diff --git a/frontend/src/components/VSearchResultsGrid/VImageCell.vue b/frontend/src/components/VSearchResultsGrid/VImageCell.vue
index 9d0c86c59ad..7ba7b0f28fe 100644
--- a/frontend/src/components/VSearchResultsGrid/VImageCell.vue
+++ b/frontend/src/components/VSearchResultsGrid/VImageCell.vue
@@ -1,10 +1,10 @@
 <template>
   <VLink
     itemprop="contentUrl"
-    :title="image.title"
+    :title="contextSensitiveTitle"
     :href="imageLink"
     class="group relative block w-full overflow-hidden rounded-sm bg-dark-charcoal-10 text-dark-charcoal-10 focus-bold-filled"
-    :aria-label="image.title"
+    :aria-label="contextSensitiveTitle"
     :style="styles.container"
   >
     <Component :is="as">
@@ -46,6 +46,7 @@ import { computed, defineComponent, PropType } from "vue"
 
 import type { AspectRatio, ImageDetail } from "~/types/media"
 import { useImageCellSize } from "~/composables/use-image-cell-size"
+import { useI18n } from "~/composables/use-i18n"
 
 import VLicense from "~/components/VLicense/VLicense.vue"
 import VLink from "~/components/VLink.vue"
@@ -93,6 +94,7 @@ export default defineComponent({
       imageSize: { width: props.image.width, height: props.image.height },
       isSquare,
     })
+    const i18n = useI18n()
 
     const imageUrl = computed(() => {
       // TODO: check if we have blurry panorama thumbnails
@@ -136,12 +138,19 @@ export default defineComponent({
       imgWidth.value = element.naturalWidth
     }
 
+    const contextSensitiveTitle = computed(() => {
+      return i18n.t("browse-page.aria.image-title", {
+        title: props.image.title,
+      })
+    })
+
     return {
       styles,
       imgWidth,
       imgHeight,
       imageUrl,
       imageLink,
+      contextSensitiveTitle,
 
       getImageForeignUrl,
       onImageLoadError,
diff --git a/frontend/src/locales/scripts/en.json5 b/frontend/src/locales/scripts/en.json5
index 8c44eb51e0b..8969b201108 100644
--- a/frontend/src/locales/scripts/en.json5
+++ b/frontend/src/locales/scripts/en.json5
@@ -421,6 +421,8 @@
       "remove-filter": "remove filter",
       "license-explanation": "license explanation",
       creator: "search by creator",
+      "image-title": "Image: {title}",
+      "audio-title": "Audio: {title}",
       results: "Search results for {query}.",
     },
   },
@@ -596,9 +598,9 @@
     alt: 'Cover art for "{title}" by {creator}',
   },
   "audio-track": {
-    "aria-label": "{title} - Audio Player",
-    "aria-label-interactive": "{title} - audio player - press the spacebar to play and pause a preview of the audio",
-    "aria-label-interactive-seekable": "{title} - audio player - press the spacebar to play and pause a preview of the audio; use the left and right arrow keys to seek through the track.",
+    "aria-label": "Audio: {title}",
+    "aria-label-interactive": "Audio: {title} - interactive player - press the space bar to play and pause a preview of the audio",
+    "aria-label-interactive-seekable": "Audio: {title} - interactive player - press the space bar to play and pause a preview of the audio; use the left and right arrow keys to seek through the track.",
     messages: {
       err_aborted: "You aborted playback.",
       err_network: "A network error occurred.",
diff --git a/frontend/test/playwright/e2e/all-results-keyboard.spec.ts b/frontend/test/playwright/e2e/all-results-keyboard.spec.ts
index 8413f21ad53..c248d8c0a30 100644
--- a/frontend/test/playwright/e2e/all-results-keyboard.spec.ts
+++ b/frontend/test/playwright/e2e/all-results-keyboard.spec.ts
@@ -7,13 +7,12 @@ import { keycodes } from "~/constants/key-codes"
 const walkToNextOfType = async (type: "image" | "audio", page: Page) => {
   const isActiveElementOfType = () => {
     return page.evaluate(
-      ([contextType]) =>
-        new RegExp(
-          `/${contextType}/[0-9A-F]{8}-[0-9A-F]{4}-4[0-9A-F]{3}-[89AB][0-9A-F]{3}-[0-9A-F]{12}\\?q=birds$`,
-          "i"
-        ).test(
-          (document.activeElement as HTMLAnchorElement | null)?.href ?? ""
-        ),
+      ([contextType]) => {
+        const regex = new RegExp(`^${contextType}:`, "i")
+        const element = document.activeElement as HTMLAnchorElement | null
+        const toTest = element?.title || element?.ariaLabel || ""
+        return regex.test(toTest)
+      },
       [type]
     )
   }
@@ -46,7 +45,7 @@ test.describe.configure({ mode: "parallel" })
 
 test.describe("all results grid keyboard accessibility test", () => {
   test.beforeEach(async ({ page }) => {
-    await page.goto("/search?q=birds")
+    await page.goto("/search/?q=birds")
   })
 
   test("should open image results as links", async ({ page }) => {
@@ -60,7 +59,7 @@ test.describe("all results grid keyboard accessibility test", () => {
     )
   })
 
-  test("should open audio results as links", async ({ page }) => {
+  test.only("should open audio results as links", async ({ page }) => {
     await walkToType("audio", page)
     await page.keyboard.press("Enter")
     await page.waitForURL(
diff --git a/frontend/test/playwright/playwright.config.ts b/frontend/test/playwright/playwright.config.ts
index 8b0d4377097..d91b7cc63bc 100644
--- a/frontend/test/playwright/playwright.config.ts
+++ b/frontend/test/playwright/playwright.config.ts
@@ -11,24 +11,23 @@ addAliases({
 
 const UPDATE_TAPES = process.env.UPDATE_TAPES || "false"
 
+/**
+ * Enabling `FASTSTART` allows you to bypass running the nuxt build
+ * when rapidly debugging tests locally within the container. If you
+ * already have the Docker image, this is an easy way to iterate on
+ * tests and do log-based debugging without needing to download the
+ * enormous browser binaries (on top of the already huge Docker image
+ * download).
+ *
+ * Note that visual-regression tests can be quite flaky when using
+ * `FASTSTART`.
+ */
+const pwCommand =
+  process.env.FASTSTART !== "false" ? "start:playwright" : "prod:playwright"
+
 const config: PlaywrightTestConfig = {
   webServer: {
-    /**
-     * Note: You can speed up local testing by switching `prod` out for `start`
-     * to skip the build, but be aware that then talkback and the Nuxt server
-     * will be racing to warm up and the tests could start before talkback is
-     * ready to start serving API responses. Playwright only lets you specify
-     * one port to wait for a ready response from so there's no way around it.
-     * Normally the build introduces more than enough time for talkback to
-     * be ready by the time the Nuxt server is actually started.
-     *
-     * This doesn't mean _don't_ use `start`, it just means that once you've
-     * debugged everything else, make sure to inspect the trace output for any
-     * ConsoleMessage strings that refer to incorrect responses. Once you've
-     * verified that there is indeed a tape saved for the response in question,
-     * switch this back to `prod` and see if your tests pass.
-     */
-    command: "./node_modules/.bin/npm-run-all -p -r talkback prod:playwright",
+    command: `./node_modules/.bin/npm-run-all -p -r talkback ${pwCommand}`,
     cwd: "/app",
     timeout: process.env.CI ? 60_000 * 5 : 60_000 * 10, // 5 minutes in CI, 10 in other envs
     port: 8443,