Fix image and audio result focus rings (#4959)

* Fix image and audio result focus rings

* Make tests wait for rendering

* Update the focus-bold-filled snapshot

* Update image snapshots

frontend/.storybook/decorators/with-theme.ts

@@ -20,11 +20,21 @@ const setElementTheme = (el: HTMLElement, cssClass: ThemeCssClass) => {
   if (cssClass === "dark-mode") {
     el.classList.add("dark-mode")
     el.classList.remove("light-mode")
+    document.documentElement.style.setProperty(
+      "--color-bg-curr-page",
+      "#0d0d0d"
+    )
   } else {
     el.classList.add("light-mode")
     el.classList.remove("dark-mode")
+
+    document.documentElement.style.setProperty(
+      "--color-bg-curr-page",
+      "#ffffff"
+    )
   }
 }
+
 const themeState = reactive<{ value: EffectiveColorMode }>({ value: "light" })
 
 /**

frontend/src/components/VAudioTrack/VAudioTrack.vue

@@ -430,7 +430,7 @@ const layoutBasedProps = computed(() =>
         class: [
           "cursor-pointer",
           {
-            "focus-bold-filled": props.layout === "box",
+            "focus-visible:focus-bold-filled": props.layout === "box",
             "focus-slim-tx": props.layout === "row",
           },
         ],
@@ -497,7 +497,8 @@ const handleWaveformBlur = () => {
   <Component
     :is="isComposite ? VLink : 'div'"
     v-bind="containerAttributes"
-    class="audio-track group block overflow-hidden rounded-sm ring-pink-8 hover:no-underline"
+    class="audio-track group block overflow-hidden rounded-sm hover:no-underline"
+    :class="{ 'audio-link': isComposite && layout === 'box' }"
     :aria-label="ariaLabel"
     :role="isComposite ? 'application' : undefined"
     @keydown.shift.tab.exact="$emit('shift-tab', $event)"

frontend/src/components/VImageCell/VImageCell.vue

@@ -136,13 +136,22 @@ const { isHidden: shouldBlur } = useSensitiveMedia(props.image)
   <li
     :style="styles"
     class="container w-full max-w-full"
-    :class="isSquare ? 'square' : 'intrinsic'"
+    :class="
+      isSquare
+        ? 'square'
+        : 'intrinsic sm:w-[--container-width] sm:flex-grow-[--container-grow]'
+    "
   >
     <VLink
       itemprop="contentUrl"
       :title="contextSensitiveTitle"
       :href="imageLink"
-      class="group relative block w-full overflow-hidden rounded-sm text-gray-2 hover:no-underline focus-visible:outline-3 focus-visible:outline-offset-4"
+      class="image-link group relative block w-full overflow-hidden rounded-sm text-gray-2 hover:no-underline"
+      :class="
+        isSquare
+          ? 'focus-visible:focus-bold-filled'
+          : 'focus-slim-tx focus-visible:-m-2 focus-visible:w-[calc(100%+theme(spacing.4))] focus-visible:p-2 sm:focus-visible:m-0 sm:focus-visible:w-full sm:focus-visible:p-0 focus-visible:sm:focus-bold-filled'
+      "
       :aria-label="contextSensitiveLabel"
       @mousedown="sendSelectSearchResultEvent"
     >
@@ -156,7 +165,9 @@ const { isHidden: shouldBlur } = useSensitiveMedia(props.image)
         <img
           loading="lazy"
           class="image col-span-full row-span-full block w-full overflow-hidden rounded-sm object-cover"
-          :class="[isSquare ? 'h-full' : 'margin-auto']"
+          :class="[
+            isSquare ? 'h-full' : 'margin-auto sm:aspect-[--img-aspect-ratio]',
+          ]"
           :alt="
             shouldBlur ? `${$t('sensitiveContent.title.image')}` : image.title
           "
@@ -176,7 +187,7 @@ const { isHidden: shouldBlur } = useSensitiveMedia(props.image)
           <VIcon name="eye-closed" />
         </span>
         <figcaption
-          class="z-10 col-span-full my-2 self-end justify-self-start rounded-sm text-default group-hover:visible group-focus-visible:visible"
+          class="z-10 col-span-full mt-2 self-end justify-self-start rounded-sm text-default group-hover:visible group-focus-visible:visible sm:mb-2"
           :class="[
             isSquare
               ? 'invisible row-span-full p-2'
@@ -203,15 +214,3 @@ const { isHidden: shouldBlur } = useSensitiveMedia(props.image)
     </VLink>
   </li>
 </template>
-
-<style scoped>
-@screen sm {
-  .intrinsic.container {
-    flex-grow: var(--container-grow);
-    width: var(--container-width);
-  }
-  .intrinsic .image {
-    aspect-ratio: var(--img-aspect-ratio);
-  }
-}
-</style>

frontend/src/components/VImageCell/meta/VImageCell.stories.ts

@@ -19,7 +19,7 @@ export const Default: Story = {
     components: { VImageCell },
     setup() {
       return () =>
-        h("div", { class: "p-2 image-wrapper max-w-80" }, [
+        h("div", { class: "p-4 image-wrapper max-w-80" }, [
           h("ol", { class: "flex flex-wrap gap-4" }, [h(VImageCell, args)]),
         ])
     },

frontend/src/components/VSearchResultsGrid/VImageCollection.vue

@@ -14,7 +14,10 @@ withDefaults(defineProps<CollectionComponentProps<"image">>(), {
 </script>
 
 <template>
-  <ol class="image-grid flex flex-wrap gap-4" :aria-label="collectionLabel">
+  <ol
+    class="image-grid flex flex-wrap gap-6 sm:gap-4"
+    :aria-label="collectionLabel"
+  >
     <VImageCell
       v-for="image in results"
       :key="image.id"

frontend/src/components/meta/Focus.stories.js

@@ -54,6 +54,6 @@ export const BoldFilled = {
   name: "Bold filled",
 
   args: {
-    classNames: ["focus-bold-filled"],
+    classNames: ["focus-visible:focus-bold-filled"],
   },
 }

frontend/src/constants/z-indices.ts

@@ -11,7 +11,8 @@ export const Z_INDICES = Object.freeze({
   50: "50",
   // Named indices
   popover: "50",
-  snackbar: "10",
+  "focus-ring": "10",
+  snackbar: "20",
   "global-audio": "20",
 } as const)
 

frontend/src/pages/search.vue

@@ -50,7 +50,6 @@ const {
   searchTerm,
   searchType,
   apiSearchQueryParams: query,
-  searchTypeIsSupported: supported,
 } = storeToRefs(searchStore)
 
 const { fetchState } = storeToRefs(mediaStore)
@@ -167,16 +166,15 @@ await useAsyncData(
       :fetching-error="fetchingError"
       class="w-full py-10"
     />
-    <section v-else>
+    <div v-else>
       <NuxtPage
         :page-key="$route.path"
         :results="searchResults"
         :is-fetching="isFetching"
         :search-term="searchTerm"
-        :supported="supported"
         :handle-load-more="handleLoadMore"
         data-testid="search-results"
       />
-    </section>
+    </div>
   </div>
 </template>

frontend/src/styles/tailwind.css

@@ -537,35 +537,30 @@ Time                - 11px - xs semibold (default leading-tight)
     @apply focus-visible:hover:border-tx; /* focus prevails over hover */
   }
 
-  [class*="focus-slim-borderless-filled"],
-  [class*="focus-bold-filled"] {
-    @apply relative;
-    @apply after:pointer-events-none after:absolute after:inset-0 after:-z-10 after:rounded-inherit;
-
-    @apply focus-visible:outline-none; /* UA styles: none */
-    @apply focus-visible:after:z-10; /* inner-stroke: pseudo-element */
-  }
-  [class*="focus-slim-borderless-filled"] {
-    @apply focus-visible:after:shadow-slim-filled;
-    @apply focus-visible:ring; /* outer-stroke: box-shadow */
-  }
   [class*="focus-slim-filled"] {
     @apply focus-visible:relative focus-visible:after:pointer-events-none;
     /* colored outline */
     @apply focus-visible:outline focus-visible:outline-1.5 focus-visible:outline-offset-0;
     /* absolutely positioned overlapping pseudo element with a 1.5px border */
     @apply focus-visible:after:absolute focus-visible:after:z-10 focus-visible:after:rounded-inherit;
-    @apply focus-visible:after:border-1.5 focus-visible:after:border-bg-ring;
+    @apply focus-visible:after:border-1.5;
     @apply focus-visible:after:h-full-with-border focus-visible:after:w-full-with-border;
     @apply focus-visible:after:-left-px focus-visible:after:-top-px;
+
+    @apply focus-visible:after:border-bg-ring;
+    @apply focus-visible:outline-[--tw-outline-color];
+    /* @apply ring-offset-bg-default; */
   }
-  [class*="focus-slim-filled"]:focus-visible {
-    outline-color: var(--tw-outline-color);
-  }
 
-  [class*="focus-bold-filled"] {
-    @apply after:shadow-bold-filled;
-    @apply focus-visible:ring-bold; /* outer-stroke: box-shadow */
+  .focus-bold-filled {
+    @apply outline-style-none; /* UA styles: none */
+    @apply ring-bold; /* outer-stroke: box-shadow */
+    @apply relative;
+
+    @apply after:z-10; /* inner-stroke: pseudo-element */
+
+    @apply after:pointer-events-none after:absolute after:inset-0 after:rounded-inherit;
+    @apply after:z-10 after:shadow-bold-filled; /* inner-stroke: pseudo-element */
   }
 
   /**

frontend/tailwind.config.ts

@@ -98,6 +98,12 @@ export default {
       pink: shadeScale("pink"),
       yellow: shadeScale("yellow"),
 
+      // Focus ring colors
+      focus: {
+        DEFAULT: "var(--color-border-focus)",
+        yellow: "var(--color-yellow-3)",
+      },
+
       // Special keywords
       tx: "transparent",
       curr: "currentColor",
@@ -166,6 +172,8 @@ export default {
       0: "0px",
       1: "1px",
       2: "2px",
+      4: "4px",
+      8: "8px",
     },
     fontSize: {
       // Deprecated
@@ -294,10 +302,10 @@ export default {
       },
       boxShadow: {
         ring: "inset 0 0 0 1px white",
-        "ring-1.5": "inset 0 0 0 1.5px white",
+        "ring-1.5": "inset 0 0 0 1.5px var(--color-bg-curr-page)",
         "el-2": "0 0.125rem 0.25rem rgba(0, 0, 0, 0.1)",
-        "slim-filled": "inset 0 0 0 1.5px white",
-        "bold-filled": "inset 0 0 0 3px white",
+        "slim-filled": "inset 0 0 0 1.5px var(--color-bg-curr-page)",
+        "bold-filled": "inset 0 0 0 3px var(--color-bg-curr-page)",
       },
       borderRadius: {
         inherit: "inherit",
@@ -352,7 +360,7 @@ export default {
           ])
         ),
         {
-          values: { ...theme("colors"), DEFAULT: theme("borderColor.focus") },
+          values: { ...theme("colors.focus") },
         }
       )
     }),

frontend/test/playwright/visual-regression/components/filters.spec.ts

@@ -1,4 +1,4 @@
-import { test } from "@playwright/test"
+import { expect, test, type Page } from "@playwright/test"
 
 import {
   filters,
@@ -7,7 +7,11 @@ import {
 } from "~~/test/playwright/utils/navigation"
 import breakpoints from "~~/test/playwright/utils/breakpoints"
 
-import { languageDirections } from "~~/test/playwright/utils/i18n"
+import {
+  type LanguageDirection,
+  languageDirections,
+  t,
+} from "~~/test/playwright/utils/i18n"
 
 import type { Breakpoint } from "~/constants/screens"
 
@@ -16,6 +20,20 @@ test.describe.configure({ mode: "parallel" })
 const getFiltersName = (breakpoint: Breakpoint) =>
   breakpoint === "lg" ? "filters-sidebar" : "filters-modal"
 
+const checkModalRendered = async (
+  page: Page,
+  isDesktop: boolean,
+  dir: LanguageDirection
+) => {
+  if (isDesktop) {
+    return true
+  }
+  const seeResultsButton = page.getByRole("button", {
+    name: t("header.seeResults", dir),
+  })
+  await expect(seeResultsButton).toBeEnabled()
+}
+
 for (const dir of languageDirections) {
   breakpoints.describeEachBreakpoint(["xs", "sm", "md", "lg"])(
     ({ breakpoint, expectSnapshot }) => {
@@ -25,7 +43,9 @@ for (const dir of languageDirections) {
         await goToSearchTerm(page, "birds", { dir })
         await filters.open(page, dir)
       })
-      test(`filters modal none selected - ${dir}`, async ({ page }) => {
+      test(`filters none selected - ${dir}`, async ({ page }) => {
+        await checkModalRendered(page, isDesktop, dir)
+
         await expectSnapshot(
           page,
           getFiltersName(breakpoint),
@@ -37,9 +57,10 @@ for (const dir of languageDirections) {
         )
       })
 
-      test(`filters modal 1 filter selected - ${dir}`, async ({ page }) => {
+      test(`filters 1 filter selected - ${dir}`, async ({ page }) => {
         const firstFilter = page.getByRole("checkbox").first()
         await firstFilter.check()
+        await checkModalRendered(page, isDesktop, dir)
 
         const snapshotName = `${getFiltersName(breakpoint)}-checked`
 

frontend/test/playwright/visual-regression/pages/pages-single-result.spec.ts

@@ -1,4 +1,4 @@
-import { test } from "@playwright/test"
+import { expect, test } from "@playwright/test"
 
 import breakpoints from "~~/test/playwright/utils/breakpoints"
 import {
@@ -9,7 +9,7 @@ import {
   sleep,
 } from "~~/test/playwright/utils/navigation"
 
-import { languageDirections } from "~~/test/playwright/utils/i18n"
+import { languageDirections, t } from "~~/test/playwright/utils/i18n"
 
 import { supportedMediaTypes } from "~/constants/media"
 
@@ -41,6 +41,13 @@ for (const mediaType of supportedMediaTypes) {
 
         // This will include the "Back to results" link.
         await openFirstResult(page, mediaType, dir)
+        // Wait for the rendering to finish.
+        await expect(
+          page.getByRole("link", {
+            name: t(`${mediaType}Details.weblink`, dir),
+          })
+        ).toBeEnabled()
+
         await expectSnapshot(
           page,
           `${mediaType}-from-search-results-with-additional-search-views`,