Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated content link #2102

Merged
merged 11 commits into from
May 18, 2023
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
60 changes: 35 additions & 25 deletions frontend/src/components/VContentLink/VContentLink.vue
Original file line number Diff line number Diff line change
@@ -1,27 +1,23 @@
<template>
<!-- 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"
:class="
hasResults
? ' text-dark-charcoal hover:bg-dark-charcoal hover:text-white hover:no-underline focus:border-tx focus:outline-none focus-visible:ring focus-visible:ring-pink'
: 'cursor-not-allowed text-dark-charcoal/40'
"
<VButton
as="VLink"
:href="to"
:aria-label="resultsAriaLabel"
variant="bordered-gray"
size="disabled"
class="h-auto w-full flex-col !items-start !justify-start gap-1 overflow-hidden p-4 sm:h-18 sm:flex-row sm:!items-center sm:gap-2 sm:px-6"
@keydown.native.shift.tab.exact="$emit('shift-tab', $event)"
@mousedown="handleClick"
>
<div class="flex flex-col items-start md:flex-row md:items-center">
<VIcon :name="mediaType" />
<p class="hidden pt-1 font-semibold md:block md:ps-2 md:pt-0 md:text-2xl">
{{ $t(`search-type.see-${mediaType}`) }}
</p>
<p class="block pt-1 font-semibold md:hidden md:ps-2 md:pt-0 md:text-2xl">
{{ $t(`search-type.${mediaType}`) }}
</p>
</div>
<span class="text-sr">{{ resultsCountLabel }}</span>
</VLink>
<VIcon :name="mediaType" />
<p class="label-bold sm:description-bold mt-1 sm:mt-0">
{{ $t(`search-type.${mediaType}`) }}
</p>
<span
class="label-regular sm:description-regular text-dark-charcoal-70 group-hover/button:text-dark-charcoal sm:ms-auto"
>{{ resultsCountLabel }}</span
>
</VButton>
</template>

<script lang="ts">
Expand All @@ -36,12 +32,12 @@ import { defineEvent } from "~/types/emits"

import useSearchType from "~/composables/use-search-type"

import VButton from "~/components/VButton.vue"
import VIcon from "~/components/VIcon/VIcon.vue"
import VLink from "~/components/VLink.vue"

export default defineComponent({
name: "VContentLink",
components: { VIcon, VLink },
components: { VIcon, VButton },
props: {
/**
* One of the media types supported.
Expand All @@ -50,6 +46,13 @@ export default defineComponent({
type: String as PropType<SupportedMediaType>,
required: true,
},
/**
* Current search term for aria-label.
*/
searchTerm: {
type: String,
required: true,
},
/**
* The number of results that the search returned. The link
* will be disabled if this value is zero.
Expand All @@ -69,10 +72,17 @@ export default defineComponent({
"shift-tab": defineEvent<[KeyboardEvent]>(),
},
setup(props) {
const { getI18nCount } = useI18nResultsCount()
const hasResults = computed(() => props.resultsCount > 0)
const { getI18nCount, getI18nContentLinkLabel } = useI18nResultsCount()
const resultsCountLabel = computed(() => getI18nCount(props.resultsCount))

const resultsAriaLabel = computed(() =>
getI18nContentLinkLabel(
props.resultsCount,
props.searchTerm,
props.mediaType
)
)

const { activeType } = useSearchType()
const analytics = useAnalytics()

Expand All @@ -86,7 +96,7 @@ export default defineComponent({

return {
resultsCountLabel,
hasResults,
resultsAriaLabel,

handleClick,
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
<template>
<div>
<div v-if="!noResults" class="results-grid mb-4 grid grid-cols-2 gap-x-4">
<div
v-if="!noResults"
class="results-grid mb-4 mt-2 grid grid-cols-2 gap-4 md:mt-0"
>
<VContentLink
v-for="[mediaType, count] in resultCounts"
:key="mediaType"
:media-type="mediaType"
:search-term="searchTerm"
:results-count="count"
:to="contentLinkPath(mediaType)"
/>
Expand Down
57 changes: 50 additions & 7 deletions frontend/src/composables/use-i18n-utilities.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,28 @@
import { useGetLocaleFormattedNumber } from "~/composables/use-get-locale-formatted-number"
import { useI18n } from "~/composables/use-i18n"
import type { SupportedMediaType, SupportedSearchType } from "~/constants/media"
import { ALL_MEDIA, AUDIO, IMAGE } from "~/constants/media"

/**
* Not using dynamically-generated keys to ensure that
* correct line is shown in the 'po' locale files
*/
const i18nKeys = {
noResult: "browse-page.all-no-results",
result: "browse-page.all-result-count",
more: "browse-page.all-result-count-more",
[ALL_MEDIA]: {
noResult: "browse-page.all-no-results",
result: "browse-page.all-result-count",
more: "browse-page.all-result-count-more",
},
[IMAGE]: {
noResult: "browse-page.contentLink.image.zero",
result: "browse-page.contentLink.image.count",
more: "browse-page.contentLink.image.countMore",
},
[AUDIO]: {
noResult: "browse-page.contentLink.audio.zero",
result: "browse-page.contentLink.audio.count",
more: "browse-page.contentLink.audio.countMore",
},
}

/**
Expand All @@ -20,19 +34,48 @@ export function useI18nResultsCount() {

const getLoading = () => i18n.t("header.loading").toString()

const getI18nCount = (resultsCount: number) => {
const getI18nKey = (
resultsCount: number,
searchType: SupportedSearchType
) => {
const countKey =
resultsCount === 0
? "noResult"
: resultsCount >= 10000
? "more"
: "result"
const fullKey = i18nKeys[countKey]
const localeCount = getLocaleFormattedNumber(resultsCount)
return i18n.tc(fullKey, resultsCount, { localeCount })
return i18nKeys[searchType][countKey]
}

/**
* Returns the localized text for the content link label.
* E.g. "See 3,567 image results for 'cats'".
*/
const getI18nContentLinkLabel = (
resultsCount: number,
query: string,
mediaType: SupportedMediaType
) => {
return i18n.tc(getI18nKey(resultsCount, mediaType), resultsCount, {
localeCount: getLocaleFormattedNumber(resultsCount),
query,
mediaType,
})
}
/**
* Returns the localized text for the number of search results, using corresponding
* pluralization rules and decimal separators.
* E.g. "No results", "3,567 results", "Over 10,000 results".
*/
const getI18nCount = (resultsCount: number) => {
return i18n.tc(getI18nKey(resultsCount, ALL_MEDIA), resultsCount, {
localeCount: getLocaleFormattedNumber(resultsCount),
})
}

return {
getI18nCount,
getI18nContentLinkLabel,
getLoading,
}
}
14 changes: 13 additions & 1 deletion frontend/src/locales/scripts/en.json5
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,19 @@
"browse-page": {
"all-no-results": "No results",
"all-result-count": "{localeCount} result|{localeCount} results",
"all-result-count-more": "Over {localeCount} result|Over {localeCount} results",
"all-result-count-more": "Over {localeCount} results",
contentLink: {
image: {
zero: "No images found for {query}.",
count: "See {localeCount} image found for {query}|See {localeCount} images found for {query}.",
countMore: "See over {localeCount} images found for {query}",
},
audio: {
zero: "No audio found for {query}.",
count: "See {localeCount} audio found for {query}",
countMore: "See over {localeCount} audio found for {query}",
},
},
load: "Load more results",
loading: "Loading...",
"fetching-error": "Error fetching {type}:",
Expand Down
6 changes: 3 additions & 3 deletions frontend/test/playwright/e2e/load-more.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ const openSingleMediaView = async (
page: Page,
mediaType: SupportedMediaType
) => {
const contentLinkSelector =
mediaType === IMAGE ? "See all images" : "See all audio"
await page.click(`text=${contentLinkSelector}`)
await page
.getByRole("link", { name: new RegExp(`See .+ ${mediaType}.+found for`) })
.click()
await page.waitForURL(/search\/(audio|image)/)
}
/**
Expand Down
35 changes: 21 additions & 14 deletions frontend/test/playwright/e2e/search-navigation.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { expect, test } from "@playwright/test"
import { expect, Page, test } from "@playwright/test"

import {
goToSearchTerm,
Expand All @@ -10,8 +10,15 @@ import {
import { mockProviderApis } from "~~/test/playwright/utils/route"
import breakpoints from "~~/test/playwright/utils/breakpoints"

import { AUDIO, IMAGE, SupportedMediaType } from "~/constants/media"

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

const getContentLink = async (page: Page, mediaType: SupportedMediaType) => {
const linkName = new RegExp(`See .+${mediaType}.+found for`)
return page.getByRole("link", { name: linkName })
}

test.describe("search history navigation", () => {
breakpoints.describeMobileAndDesktop(() => {
test.beforeEach(async ({ context }) => {
Expand Down Expand Up @@ -49,20 +56,18 @@ test.describe("search history navigation", () => {
page,
}) => {
await goToSearchTerm(page, "galah")
await page.click('a:has-text("See all images")')
await page
.getByRole("link", { name: /See.*images found for .*/i })
.click()

await page.waitForSelector('p:has-text("See all images")', {
state: "hidden",
})
expect(page.url()).toContain("/search/image")
await page.goBack()
await page.waitForSelector('a:has-text("See all images")')
expect(
await page.locator('a:has-text("See all images")').isVisible()
).toBe(true)
expect(
await page.locator('a:has-text("See all audio")').isVisible()
).toBe(true)
// There are no content links on single media type search pages
await expect(await getContentLink(page, IMAGE)).not.toBeVisible()

await page.goBack({ waitUntil: "networkidle" })

await expect(await getContentLink(page, IMAGE)).toBeVisible()
await expect(await getContentLink(page, AUDIO)).toBeVisible()
})

test("should update search term when back button is clicked", async ({
Expand All @@ -74,7 +79,9 @@ test.describe("search history navigation", () => {
expect(await page.locator('input[name="q"]').inputValue()).toBe("cat")

await page.goBack()
await page.waitForSelector('a:has-text("See all images")')

await expect(await getContentLink(page, IMAGE)).toBeVisible()

expect(await page.locator('input[name="q"]').inputValue()).toBe("galah")
})

Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
17 changes: 6 additions & 11 deletions frontend/test/unit/specs/components/v-content-link.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,12 @@ describe("VContentLink", () => {

beforeEach(() => {
options = {
props: { mediaType: "image", resultsCount: 123, to: "/images" },
props: {
mediaType: "image",
resultsCount: 123,
to: "/images",
searchTerm: "cat",
},
}
})

Expand All @@ -27,16 +32,6 @@ describe("VContentLink", () => {
expect(btn).not.toHaveAttribute("aria-disabled")
})

it("is disabled when there are no results", () => {
options.props.resultsCount = 0
render(VContentLink, options)
const btn = screen.getByRole("link")

expect(btn).not.toHaveAttribute("href")
expect(btn).toHaveAttribute("aria-disabled")
expect(btn.getAttribute("aria-disabled")).toBeTruthy()
})

it("sends CHANGE_CONTENT_TYPE event when clicked", async () => {
const sendCustomEventMock = jest.fn()

Expand Down