Improve image grid (#2891)

* Improve image grid

* Update tests

* Update snapshots

* Fix image cell storybook test

frontend/src/components/VImageCell/VImageCell.vue

@@ -1,33 +1,33 @@
 <template>
-  <li :style="styles.container">
+  <li
+    :style="styles"
+    class="container w-full max-w-full"
+    :class="isSquare ? 'square' : 'intrinsic'"
+  >
     <VLink
       itemprop="contentUrl"
       :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"
+      class="group relative block w-full overflow-hidden rounded-sm text-dark-charcoal-10 focus-visible:outline-3 focus-visible:outline-offset-4"
       :aria-label="contextSensitiveTitle"
       @mousedown="sendSelectSearchResultEvent"
     >
       <figure
         itemprop="image"
         itemscope
         itemtype="https://schema.org/ImageObject"
-        class="absolute w-full rounded-sm"
-        :class="{ 'relative aspect-square': isSquare }"
-        :style="styles.figure"
+        class="grid w-full rounded-sm"
+        :class="{ 'aspect-square': isSquare }"
       >
         <img
-          ref="img"
           loading="lazy"
-          class="block w-full rounded-sm object-cover duration-200 motion-safe:transition-[filter,transform]"
+          class="image col-span-full row-span-full block w-full overflow-hidden rounded-sm object-cover duration-200 motion-safe:transition-[filter,transform]"
           :class="[
             isSquare ? 'h-full' : 'margin-auto',
             { 'scale-150 blur-image': shouldBlur },
           ]"
           :alt="
-            shouldBlur
-              ? $t('sensitiveContent.title.image').toString()
-              : image.title
+            shouldBlur ? `${$t('sensitiveContent.title.image')}` : image.title
           "
           :src="imageUrl"
           :width="imgWidth"
@@ -37,19 +37,25 @@
           @error="onImageLoadError($event)"
         />
         <figcaption
-          class="invisible absolute bottom-2 left-2 rounded-sm bg-white p-2 text-dark-charcoal group-hover:visible group-focus:visible"
+          class="col-span-full self-end justify-self-start rounded-sm bg-white text-dark-charcoal group-hover:visible group-focus-visible:visible"
+          :class="
+            isSquare
+              ? 'invisible row-span-full m-2 p-2'
+              : 'my-2 sm:invisible sm:row-span-full sm:m-2 sm:p-2'
+          "
         >
           <h2 class="sr-only">
             {{
-              shouldBlur
-                ? $t("sensitiveContent.title.image").toString()
-                : image.title
+              shouldBlur ? `${$t("sensitiveContent.title.image")}` : image.title
             }}
           </h2>
-          <VLicense :license="image.license" :hide-name="true" />
+          <VLicense
+            :license="image.license"
+            :hide-name="true"
+            class="text-dark-charcoal-70 group-hover:text-dark-charcoal group-focus-visible:text-dark-charcoal sm:text-dark-charcoal"
+          />
         </figcaption>
       </figure>
-      <i v-if="!isSquare" :style="styles.iPadding" class="block" aria-hidden />
     </VLink>
   </li>
 </template>
@@ -116,6 +122,7 @@ export default defineComponent({
 
     const imageUrl = computed(() => {
       // TODO: check if we have blurry panorama thumbnails
+      // Use the main image file and not the thumbnails for panorama images to
       // fix for blurry panorama thumbnails, introduced in
       // https://github.com/cc-archive/cccatalog-frontend/commit/4c9bdac5
       if (isPanorama.value) {
@@ -199,3 +206,15 @@ export default defineComponent({
   },
 })
 </script>
+
+<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.mdx

@@ -17,7 +17,7 @@ export const Template = (args, { argTypes }) => ({
   template: `
     <ol class="flex flex-wrap gap-4">
       <VImageCell
-        :image="{ ...image, url: base64Image }"
+        :image="image"
         searchTerm="test"
         :aspect-ratio="aspectRatio"
         relatedTo="fake-uuid"
@@ -38,10 +38,6 @@ export const Template = (args, { argTypes }) => ({
     parameters={{
       viewport: { defaultViewport: "sm" },
     }}
-    args={{
-      aspectRatio: "intrinsic",
-      image: image,
-    }}
     argTypes={{
       aspectRatio: {
         options: ["square", "intrinsic"],
@@ -51,6 +47,10 @@ export const Template = (args, { argTypes }) => ({
         control: { type: "object" },
       },
     }}
+    args={{
+      aspectRatio: "intrinsic",
+      image: { ...image, url: base64Image },
+    }}
   >
     {Template.bind({})}
   </Story>

frontend/src/components/VSearchResultsGrid/VImageGrid.vue

@@ -1,5 +1,5 @@
 <template>
-  <section>
+  <section class="pt-2 sm:pt-0">
     <VGridSkeleton
       v-if="images && !images.length && !fetchState.isFinished"
       is-for-tab="image"

frontend/src/composables/use-image-cell-size.ts

@@ -5,12 +5,12 @@ import type { ImageDimensions } from "~/types/media"
 const minAspect = 3 / 4
 const maxAspect = 16 / 9
 const panoramaAspect = 21 / 9
-const minRowWidth = 450
+const minRowWidth = 250
 const widthBasis = minRowWidth / maxAspect
 const squareSize = 250
 const defaultImgSize = 100
 
-const getContainerAspect = (aspect: number) => {
+const getImgAspectRatio = (aspect: number) => {
   if (aspect > maxAspect) return maxAspect
   if (aspect < minAspect) return minAspect
   return aspect
@@ -30,46 +30,30 @@ export const useImageCellSize = ({
     isSquare.value ? squareSize : imageSize.width || defaultImgSize
   )
 
-  const imageAspect = computed(() => imgWidth.value / imgHeight.value)
-  const isPanorama = computed(() => imageAspect.value > panoramaAspect)
+  const naturalAspectRatio = computed(() => imgWidth.value / imgHeight.value)
+  const isPanorama = computed(() => naturalAspectRatio.value > panoramaAspect)
 
   /**
-   * Returns the style declarations for container, figure and i padding.
+   * Returns the style declarations for the container width, aspect ratio and flex-grow value.
    * For the square cell, the styles are empty.
    */
   const styles = computed(() => {
-    const aspect = imageAspect.value
-
-    if (isSquare.value)
-      return {
-        container: "",
-        figure: "",
-        iPadding: "",
-      }
-
-    const containerAspect = getContainerAspect(aspect)
-    const containerWidth = containerAspect * widthBasis
-
-    let imageWidth, imageLeft, imageTop
-
-    if (aspect < maxAspect) {
-      imageWidth = 100
-      imageLeft = 0
-    } else {
-      imageWidth = (aspect / maxAspect) * 100
-      imageLeft = ((aspect - maxAspect) / maxAspect) * -50
+    const styles = {}
+    if (isSquare.value) {
+      return styles
     }
 
-    if (aspect > minAspect) {
-      imageTop = 0
-    } else {
-      imageTop = ((minAspect - aspect) / (aspect * minAspect * minAspect)) * -50
-    }
+    const aspect = naturalAspectRatio.value
+    /**
+     * The aspect-ratio for `img` is clamped between the min and max aspect ratios.
+     */
+    const imgAspectRatio = getImgAspectRatio(aspect)
+    const containerWidth = Math.round(imgAspectRatio * widthBasis)
 
     return {
-      container: `width: ${containerWidth}px;flex-grow: ${containerWidth}`,
-      figure: `width: ${imageWidth}%; top: ${imageTop}%; left:${imageLeft}%;`,
-      iPadding: `padding-bottom:${(1 / containerAspect) * 100}%`,
+      "--container-width": `${containerWidth}px`,
+      "--container-grow": containerWidth,
+      "--img-aspect-ratio": imgAspectRatio,
     }
   })
 

frontend/tailwind.config.ts

@@ -229,6 +229,7 @@ export default {
       },
       outlineWidth: {
         1.5: "1.5px",
+        3: "3px",
       },
       typography: (theme: PluginAPI["theme"]) => ({
         DEFAULT: {

frontend/test/playwright/utils/breakpoints.ts

@@ -155,6 +155,7 @@ const describeEachMobileWithoutMd = describeEachBreakpoint(
   mobileBreakpoints.filter((b) => b !== "md")
 )
 const describeMobileAndDesktop = describeEachBreakpoint(["sm", "xl"])
+const describeMobileXsAndDesktop = describeEachBreakpoint(["xs", "xl"])
 
 export default {
   ...breakpointTests,
@@ -168,4 +169,6 @@ export default {
   // For testing functionality in e2e tests, we need to test mobile and desktop screens.
   // Having two breakpoints should be enough and should save testing time.
   describeMobileAndDesktop,
+  // All tests
+  describeMobileXsAndDesktop,
 }

frontend/test/storybook/visual-regression/v-image-cell.spec.ts

@@ -2,6 +2,8 @@ import { expect, test } from "@playwright/test"
 
 import { makeGotoWithArgs } from "~~/test/storybook/utils/args"
 
+import breakpoints from "~~/test/playwright/utils/breakpoints"
+
 import type { AspectRatio } from "~/types/media"
 
 const imageCell = "a[itemprop='contentUrl']"
@@ -11,45 +13,51 @@ const screenshotEl = ".sb-main-padded"
 
 test.describe.configure({ mode: "parallel" })
 
-test.describe("VImageCell", () => {
-  const gotoWithArgs = makeGotoWithArgs("components-vimagecell--v-image-cell")
-  const aspectRatios: AspectRatio[] = ["square", "intrinsic"]
-
-  for (const ratio of aspectRatios) {
-    test(`${ratio} loaded`, async ({ page }) => {
-      await gotoWithArgs(page, { aspectRatio: ratio })
-      const mainEl = page.locator(imageCell)
-      await expect(mainEl).toBeVisible()
-      expect(await page.locator(screenshotEl).screenshot()).toMatchSnapshot({
-        name: `v-image-cell-${ratio}-loaded.png`,
+breakpoints.describeMobileXsAndDesktop(({ expectSnapshot }) => {
+  test.describe("VImageCell", () => {
+    const gotoWithArgs = makeGotoWithArgs("components-vimagecell--v-image-cell")
+    const aspectRatios: AspectRatio[] = ["square", "intrinsic"]
+
+    for (const ratio of aspectRatios) {
+      test(`${ratio} loaded`, async ({ page }) => {
+        await gotoWithArgs(page, { aspectRatio: ratio })
+        const mainEl = page.locator(imageCell)
+        await expect(mainEl).toBeVisible()
+        await expectSnapshot(
+          `v-image-cell-${ratio}-loaded`,
+          page.locator(screenshotEl)
+        )
       })
-    })
-
-    test(`${ratio} focused`, async ({ page }) => {
-      await gotoWithArgs(page, { aspectRatio: ratio })
-      await page.focus(imageCell)
-      await page.locator(imageCell).click()
-      expect(await page.locator(screenshotEl).screenshot()).toMatchSnapshot({
-        name: `v-image-cell-${ratio}-focused.png`,
+
+      test(`${ratio} focused`, async ({ page }) => {
+        await gotoWithArgs(page, { aspectRatio: ratio })
+        await page.focus(imageCell)
+        await page.locator(imageCell).click()
+        await expectSnapshot(
+          `v-image-cell-${ratio}-focused`,
+          page.locator(screenshotEl)
+        )
       })
-    })
 
-    test(`${ratio} hovered`, async ({ page }) => {
-      await gotoWithArgs(page, { aspectRatio: ratio })
-      await page.hover(imageCell)
-      expect(await page.locator(screenshotEl).screenshot()).toMatchSnapshot({
-        name: `v-image-cell-${ratio}-hovered.png`,
+      test(`${ratio} hovered`, async ({ page }) => {
+        await gotoWithArgs(page, { aspectRatio: ratio })
+        await page.hover(imageCell)
+        await expectSnapshot(
+          `v-image-cell-${ratio}-hovered`,
+          page.locator(screenshotEl)
+        )
       })
-    })
-
-    test(`${ratio} focused hovered`, async ({ page }) => {
-      await gotoWithArgs(page, { aspectRatio: ratio })
-      await page.focus(imageCell)
-      await page.hover(imageCell)
-      await page.locator(imageCell).click()
-      expect(await page.locator(screenshotEl).screenshot()).toMatchSnapshot({
-        name: `v-image-cell-${ratio}-focused-hovered.png`,
+
+      test(`${ratio} focused hovered`, async ({ page }) => {
+        await gotoWithArgs(page, { aspectRatio: ratio })
+        await page.focus(imageCell)
+        await page.hover(imageCell)
+        await page.locator(imageCell).click()
+        await expectSnapshot(
+          `v-image-cell-${ratio}-focused-hovered`,
+          page.locator(screenshotEl)
+        )
       })
-    })
-  }
+    }
+  })
 })

frontend/test/unit/specs/composables/use-image-cell-size.spec.js

@@ -12,7 +12,7 @@ describe("useImageCellSize", () => {
     expect(imgHeight.value).toEqual(250)
     expect(imgWidth.value).toEqual(250)
     expect(isPanorama.value).toEqual(false)
-    expect(styles.value).toEqual({ container: "", figure: "", iPadding: "" })
+    expect(styles.value).toEqual({})
   })
 
   it("Should return correct values for intrinsic panorama image", () => {
@@ -27,9 +27,9 @@ describe("useImageCellSize", () => {
     expect(imgWidth.value).toEqual(WIDTH)
     expect(isPanorama.value).toEqual(true)
     expect(styles.value).toEqual({
-      container: "width: 450px;flex-grow: 450",
-      figure: "width: 675%; top: 0%; left:-287.5%;",
-      iPadding: "padding-bottom:56.25%",
+      "--container-grow": 250,
+      "--container-width": "250px",
+      "--img-aspect-ratio": 1.7777777777777777,
     })
   })
 
@@ -45,9 +45,9 @@ describe("useImageCellSize", () => {
     expect(imgWidth.value).toEqual(WIDTH)
     expect(isPanorama.value).toEqual(false)
     expect(styles.value).toEqual({
-      container: "width: 189.84375px;flex-grow: 189.84375",
-      figure: "width: 100%; top: -711.1111111111111%; left:0%;",
-      iPadding: "padding-bottom:133.33333333333331%",
+      "--container-grow": 105,
+      "--container-width": "105px",
+      "--img-aspect-ratio": 0.75,
     })
   })
 
@@ -63,9 +63,9 @@ describe("useImageCellSize", () => {
     expect(imgWidth.value).toEqual(WIDTH)
     expect(isPanorama.value).toEqual(false)
     expect(styles.value).toEqual({
-      container: "width: 253.125px;flex-grow: 253.125",
-      figure: "width: 100%; top: 0%; left:0%;",
-      iPadding: "padding-bottom:100%",
+      "--container-grow": 141,
+      "--container-width": "141px",
+      "--img-aspect-ratio": 1,
     })
   })
 })