Skip to content

Commit

Permalink
Add media type context to start of result link titles (#1920)
Browse files Browse the repository at this point in the history
Co-authored-by: Olga Bulat <[email protected]>
Co-authored-by: Zack Krida <[email protected]>
  • Loading branch information
3 people authored May 17, 2023
1 parent 67cbf85 commit 2041c5d
Show file tree
Hide file tree
Showing 8 changed files with 45 additions and 33 deletions.
1 change: 1 addition & 0 deletions frontend/docker-compose.playwright.yml
Original file line number Diff line number Diff line change
Expand Up @@ -18,4 +18,5 @@ services:
- DEBUG=pw:webserver
- DISABLE_SENTRY=true
- UPDATE_TAPES=${UPDATE_TAPES:-false}
- FASTSTART=${FASTSTART:-false}
cpus: 0.000
4 changes: 3 additions & 1 deletion frontend/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/components/VAudioTrack/VAudioTrack.vue
Original file line number Diff line number Diff line change
Expand Up @@ -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 })
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/components/VContentLink/VContentLink.vue
Original file line number Diff line number Diff line change
@@ -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"
Expand Down
13 changes: 11 additions & 2 deletions frontend/src/components/VSearchResultsGrid/VImageCell.vue
Original file line number Diff line number Diff line change
@@ -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">
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down
8 changes: 5 additions & 3 deletions frontend/src/locales/scripts/en.json5
Original file line number Diff line number Diff line change
Expand Up @@ -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}.",
},
},
Expand Down Expand Up @@ -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.",
Expand Down
17 changes: 8 additions & 9 deletions frontend/test/playwright/e2e/all-results-keyboard.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]
)
}
Expand Down Expand Up @@ -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 }) => {
Expand All @@ -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(
Expand Down
31 changes: 15 additions & 16 deletions frontend/test/playwright/playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 2041c5d

Please sign in to comment.