Skip to content

Commit

Permalink
Standardize the image and audio result components (#5272)
Browse files Browse the repository at this point in the history
* Standardize the image and audio result components

* "ar" locale started using Western numbers

* Fix image result query missing
  • Loading branch information
obulat authored Dec 15, 2024
1 parent dbc68f5 commit 87ce0b3
Show file tree
Hide file tree
Showing 42 changed files with 78 additions and 72 deletions.
19 changes: 18 additions & 1 deletion frontend/shared/utils/query-utils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { SupportedMediaType } from "#shared/constants/media"
import type { SupportedMediaType } from "#shared/constants/media"

import type { LocationQueryValue } from "vue-router"

Expand All @@ -22,3 +22,20 @@ export const validateUUID = (id: string | undefined | null) => {

export const mediaSlug = (mediaType: SupportedMediaType) =>
mediaType === "image" ? "images" : "audio"

/**
* Create a query string for a single result page, e.g. `/search?q=term&p=1`.
*/
export const singleResultQuery = (searchTerm?: string, position?: number) => {
if (!searchTerm && !position) {
return ""
}
const query = new URLSearchParams()
if (searchTerm) {
query.set("q", searchTerm)
}
if (position) {
query.set("p", position.toString())
}
return `?${query.toString()}`
}
60 changes: 25 additions & 35 deletions frontend/src/components/VAudioTrack/VAudioTrack.vue
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,14 @@ const props = defineProps<{
*/
size?: AudioSize
/**
* The search term that was used to find this track. This is used
* in the link to the track's detail page.
* The URL for the single result page. Can include search term and position
* in the search results.
*/
searchTerm?: string
href?: string
}>()
const emit = defineEmits<{
"shift-tab": [KeyboardEvent]
interacted: [Omit<AudioInteractionData, "component">]
interacted: [Pick<AudioInteractionData, "event" | "id" | "provider">]
mousedown: [AudioTrackClickEvent]
focus: [FocusEvent]
}>()
Expand Down Expand Up @@ -415,34 +415,15 @@ const handleMousedown = (event: MouseEvent) => {
emit("mousedown", { event, inWaveform })
}
/**
* These layout-conditional props and listeners allow us
* to set properties on the parent element depending on
* the layout in use.
*/
const isComposite = computed(() => ["box", "row"].includes(props.layout))
const layoutBasedProps = computed(() =>
isComposite.value
? {
href: `/audio/${props.audio.id}/${
props.searchTerm ? "?q=" + props.searchTerm : ""
}`,
class: [
"cursor-pointer",
{
"focus-visible:focus-bold-filled": props.layout === "box",
"focus-slim-tx": props.layout === "row",
},
],
}
: {}
)
const ariaLabel = computed(() =>
isComposite.value
? t("audioTrack.ariaLabelInteractiveSeekable", {
title: props.audio.title,
})
: t("audioTrack.ariaLabel", { title: props.audio.title })
t(
isComposite.value
? "audioTrack.ariaLabelInteractiveSeekable"
: "audioTrack.ariaLabel",
{ title: props.audio.title }
)
)
const togglePlayback = () => {
Expand All @@ -462,11 +443,15 @@ const handleKeydown = (event: KeyboardEvent) => {
seekable.listeners.keydown(event)
}
const containerAttributes = computed(() => ({
/**
* These layout-conditional props and listeners allow us
* to set properties on the parent element depending on
* the layout in use.
*/
const containerAttributes = computed(() => {
// ARIA slider attributes are only added when interactive
...(isComposite.value ? seekable.attributes.value : {}),
...layoutBasedProps.value,
}))
return isComposite.value ? seekable.attributes.value : {}
})
const snackbar = useAudioSnackbar()
Expand Down Expand Up @@ -496,9 +481,14 @@ const handleWaveformBlur = () => {
<!-- eslint-disable vue/use-v-on-exact -->
<Component
:is="isComposite ? VLink : 'div'"
:href="href"
v-bind="containerAttributes"
class="audio-track group block overflow-hidden rounded-sm hover:no-underline"
:class="{ 'audio-link': isComposite && layout === 'box' }"
:class="{
'audio-link cursor-pointer focus-visible:focus-bold-filled':
layout === 'box',
'cursor-pointer focus-slim-tx': layout === 'row',
}"
:aria-label="ariaLabel"
:role="isComposite ? 'application' : undefined"
@keydown.shift.tab.exact="$emit('shift-tab', $event)"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { useI18n, useNuxtApp } from "#imports"
import { computed } from "vue"
import { IMAGE } from "#shared/constants/media"
import { singleResultQuery } from "#shared/utils/query-utils"
import type { AspectRatio, ImageDetail } from "#shared/types/media"
import type { SingleResultProps } from "#shared/types/collection-component-props"
import { useSearchStore } from "~/stores/search"
Expand Down Expand Up @@ -60,9 +61,7 @@ const imageUrl = computed(() => {
})
const imageLink = computed(() => {
return `/image/${props.image.id}/${
props.searchTerm ? "?q=" + props.searchTerm : ""
}`
return `/image/${props.image.id}/${singleResultQuery(props.searchTerm)}`
})
/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,45 +2,40 @@ import { h } from "vue"

import { image } from "~~/test/unit/fixtures/image"

import VImageCell from "~/components/VImageCell/VImageCell.vue"
import VImageResult from "~/components/VImageResult/VImageResult.vue"

import type { Meta, StoryObj } from "@storybook/vue3"

const meta = {
title: "Components/VImageCell",
component: VImageCell,
} satisfies Meta<typeof VImageCell>
title: "Components/VImageResult",
component: VImageResult,
} satisfies Meta<typeof VImageResult>

export default meta
type Story = StoryObj<typeof meta>

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

parameters: {
viewport: {
defaultViewport: "sm",
},
viewport: { defaultViewport: "sm" },
},

argTypes: {
aspectRatio: {
options: ["square", "intrinsic"],
control: { type: "radio" },
},

image: {
control: { type: "object" },
control: { type: "select" },
},
image: { control: { type: "object" } },
},

args: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
} from "#shared/types/media"
import { useUiStore } from "~/stores/ui"
import VImageCell from "~/components/VImageCell/VImageCell.vue"
import VImageResult from "~/components/VImageResult/VImageResult.vue"
import VAudioResult from "~/components/VSearchResultsGrid/VAudioResult.vue"
import VAudioInstructions from "~/components/VSearchResultsGrid/VAudioInstructions.vue"
Expand Down Expand Up @@ -39,7 +39,7 @@ const isSm = computed(() => uiStore.isBreakpoint("sm"))
:aria-label="collectionLabel"
>
<template v-for="item in results">
<VImageCell
<VImageResult
v-if="isDetail.image(item)"
:key="item.id"
:image="item"
Expand Down
11 changes: 8 additions & 3 deletions frontend/src/components/VSearchResultsGrid/VAudioResult.vue
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
<script setup lang="ts">
import { useNuxtApp } from "#imports"
import { toRefs } from "vue"
import { computed, ref } from "vue"
import { AUDIO } from "#shared/constants/media"
import type { AudioLayout, AudioSize } from "#shared/constants/audio"
import { singleResultQuery } from "#shared/utils/query-utils"
import type { AudioInteractionData } from "#shared/types/analytics"
import type { AudioTrackClickEvent } from "#shared/types/events"
import type { AudioDetail } from "#shared/types/media"
Expand All @@ -30,8 +31,11 @@ const props = withDefaults(
const { $sendCustomEvent } = useNuxtApp()
const searchStore = useSearchStore()
const { audio } = toRefs(props)
const { isHidden: shouldBlur } = useSensitiveMedia(audio)
const { isHidden: shouldBlur } = useSensitiveMedia(ref(props.audio))
const href = computed(() => {
return `/audio/${props.audio.id}/${singleResultQuery(props.searchTerm)}`
})
const sendSelectSearchResultEvent = (
audio: AudioDetail,
Expand Down Expand Up @@ -76,6 +80,7 @@ const sendInteractionEvent = (
:layout="layout"
:size="size"
:search-term="searchTerm"
:href="href"
v-bind="$attrs"
@interacted="sendInteractionEvent"
@mousedown="sendSelectSearchResultEvent(audio, $event)"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import type { CollectionComponentProps } from "#shared/types/collection-component-props"
import VImageCell from "~/components/VImageCell/VImageCell.vue"
import VImageResult from "~/components/VImageResult/VImageResult.vue"
withDefaults(defineProps<CollectionComponentProps<"image">>(), {
relatedTo: "null",
Expand All @@ -18,7 +18,7 @@ withDefaults(defineProps<CollectionComponentProps<"image">>(), {
class="image-grid flex flex-wrap gap-6 sm:gap-4"
:aria-label="collectionLabel"
>
<VImageCell
<VImageResult
v-for="image in results"
:key="image.id"
:image="image"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ const imageCell = "a[itemprop='contentUrl']"
// exceeds the bounds of the actual component
const screenshotEl = ".image-wrapper"

const urlWithArgs = makeUrlWithArgs("components-vimagecell--default")
const urlWithArgs = makeUrlWithArgs("components-vimageresult--default")

test.describe.configure({ mode: "parallel" })

const aspectRatios: AspectRatio[] = ["square", "intrinsic"]
test.describe("VImageCell", () => {
test.describe("VImageResult", () => {
breakpoints.describeMobileXsAndDesktop(({ expectSnapshot }) => {
for (const ratio of aspectRatios) {
test.beforeEach(async ({ page }) => {
Expand All @@ -28,7 +28,7 @@ test.describe("VImageCell", () => {
await page.goto(urlWithArgs({ aspectRatio: ratio }))
await expectSnapshot(
page,
`v-image-cell-${ratio}-loaded`,
`v-image-result-${ratio}-loaded`,
page.locator(screenshotEl)
)
})
Expand All @@ -40,7 +40,7 @@ test.describe("VImageCell", () => {

await expectSnapshot(
page,
`v-image-cell-${ratio}-focused`,
`v-image-result-${ratio}-focused`,
page.locator(screenshotEl)
)
})
Expand All @@ -52,7 +52,7 @@ test.describe("VImageCell", () => {

await expectSnapshot(
page,
`v-image-cell-${ratio}-hovered`,
`v-image-result-${ratio}-hovered`,
page.locator(screenshotEl)
)
})
Expand All @@ -66,7 +66,7 @@ test.describe("VImageCell", () => {

await expectSnapshot(
page,
`v-image-cell-${ratio}-focused-hovered`,
`v-image-result-${ratio}-focused-hovered`,
page.locator(screenshotEl)
)
})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { createApp } from "vue"
import { image } from "~~/test/unit/fixtures/image"
import { render } from "~~/test/unit/test-utils/render"

import VImageCell from "~/components/VImageCell/VImageCell.vue"
import VImageResult from "~/components/VImageResult/VImageResult.vue"

const RouterLinkStub = createApp({}).component("RouterLink", {
template: "<a :href='href'><slot /></a>",
Expand All @@ -14,7 +14,7 @@ const RouterLinkStub = createApp({}).component("RouterLink", {
},
},
})._context.components.RouterLink
describe("VImageCell", () => {
describe("VImageResult", () => {
let options = {}

beforeEach(() => {
Expand All @@ -35,14 +35,14 @@ describe("VImageCell", () => {

it("is blurred when the image is sensitive", async () => {
options.props.image.isSensitive = true
const { getByTestId } = await render(VImageCell, options)
const { getByTestId } = await render(VImageResult, options)
const overlay = getByTestId("blur-overlay")
expect(overlay).toBeVisible()
})

it("is does not contain title anywhere when the image is sensitive", async () => {
options.props.image.isSensitive = true
const screen = await render(VImageCell, options)
const screen = await render(VImageResult, options)
const match = RegExp(image.title)
expect(screen.queryAllByText(match)).toEqual([])
expect(screen.queryAllByTitle(match)).toEqual([])
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const testWrapperContainer = createApp({}).component("TestWrapper", {
const TestWrapper = testWrapperContainer._context.components.TestWrapper

describe("use-get-locale-formatted-number", () => {
it.each(["ar", "fa", "ckb", "ps"])(
it.each(["ar-EG", "fa", "ckb", "ps"])(
"should use en formatting for locales like %s that use Eastern Arabic Numerals",
async (locale) => {
const { container } = await render(TestWrapper, {
Expand Down

0 comments on commit 87ce0b3

Please sign in to comment.