From 213c88692a42f1b4d89246a6c68fa5e89edef0c9 Mon Sep 17 00:00:00 2001 From: Olga Bulat Date: Wed, 3 Apr 2024 07:45:08 +0300 Subject: [PATCH] Add SEO properties to collection pages (#3999) * Add SEO properties to collection pages Signed-off-by: Olga Bulat * Update frontend/src/locales/scripts/en.json5 Co-authored-by: Madison Swain-Bowden * Add suggestions from code review use-collection-meta.ts is refactored to add a static " | Openverse" string to the title to prevent confusion with pluralized i18n strings Signed-off-by: Olga Bulat * Add suggestions from code review Add suggestions from code review Update the strings: add comment that the phrases will be used for page titles Signed-off-by: Olga Bulat * Fix types Signed-off-by: Olga Bulat * Simplify Signed-off-by: Olga Bulat * Replace docker-compose with docker compose for playwright Signed-off-by: Olga Bulat --------- Signed-off-by: Olga Bulat Co-authored-by: Madison Swain-Bowden --- frontend/bin/playwright.sh | 4 +- .../src/composables/use-collection-meta.ts | 46 ++++++++++++++++++ frontend/src/locales/scripts/en.json5 | 35 ++++++++++++++ frontend/src/pages/audio/collection.vue | 16 +++++-- frontend/src/pages/image/collection.vue | 15 +++++- frontend/src/utils/parse-collection-path.ts | 24 ---------- frontend/test/playwright/e2e/seo.spec.ts | 25 +++++++++- .../utils/validate-collection-params.spec.js | 48 ------------------- 8 files changed, 133 insertions(+), 80 deletions(-) create mode 100644 frontend/src/composables/use-collection-meta.ts delete mode 100644 frontend/src/utils/parse-collection-path.ts delete mode 100644 frontend/test/unit/specs/utils/validate-collection-params.spec.js diff --git a/frontend/bin/playwright.sh b/frontend/bin/playwright.sh index 57b21fd1204..c47f75ecf49 100755 --- a/frontend/bin/playwright.sh +++ b/frontend/bin/playwright.sh @@ -21,6 +21,6 @@ pnpm i18n:get-translations --en-only echo Running Playwright v"$PLAYWRIGHT_VERSION" as "$USER_ID" with Playwright arguments "$PLAYWRIGHT_ARGS" under package manager "$PACKAGE_MANAGER" -docker-compose -f docker-compose.playwright.yml up --build --force-recreate --exit-code-from playwright --remove-orphans +docker compose -f docker-compose.playwright.yml up --build --force-recreate --exit-code-from playwright --remove-orphans -docker-compose -f docker-compose.playwright.yml down +docker compose -f docker-compose.playwright.yml down diff --git a/frontend/src/composables/use-collection-meta.ts b/frontend/src/composables/use-collection-meta.ts new file mode 100644 index 00000000000..55f67c8985c --- /dev/null +++ b/frontend/src/composables/use-collection-meta.ts @@ -0,0 +1,46 @@ +import { computed } from "vue" + +import { useI18n } from "~/composables/use-i18n" +import { useProviderStore } from "~/stores/provider" +import type { SupportedMediaType } from "~/constants/media" +import type { CollectionParams } from "~/types/search" + +import type { ComputedRef } from "vue" + +export const useCollectionMeta = ({ + collectionParams, + mediaType, + i18n, +}: { + collectionParams: ComputedRef + mediaType: SupportedMediaType + i18n: ReturnType +}) => { + const pageTitle = computed(() => { + const params = collectionParams.value + + if (params) { + if (params.collection === "creator") { + return `${params.creator} | Openverse` + } + + if (params.collection === "source") { + const sourceName = useProviderStore().getProviderName( + params.source, + mediaType + ) + return `${i18n.t(`collection.pageTitle.source.${mediaType}`, { source: sourceName })} | Openverse` + } + + if (params.collection === "tag") { + return `${i18n.t(`collection.pageTitle.tag.${mediaType}`, { tag: params.tag })} | Openverse` + } + } + + return "Openly Licensed Images, Audio and More | Openverse" + }) + + return { + pageTitle, + } +} diff --git a/frontend/src/locales/scripts/en.json5 b/frontend/src/locales/scripts/en.json5 index 3b0e1b9f00a..d7c2110e993 100644 --- a/frontend/src/locales/scripts/en.json5 +++ b/frontend/src/locales/scripts/en.json5 @@ -1085,6 +1085,41 @@ creator: "Creator", source: "Source", }, + pageTitle: { + tag: { + /** + * This will be the page title and must be SEO friendly. + * {tag} will be a dynamic value such as "cat". We cannot change its case or form. + * You can change the sentence to add more context and make the sentence + * grammatically correct, for instance, to "Audio tracks with the tag {tag}". + */ + audio: "{tag} audio tracks", + + /** + * This will be the page title and must be SEO friendly. + * {tag} will be a dynamic value such as "cat". We cannot change its case or form. + * You can change the sentence to add more context and make the sentence + * grammatically correct, for instance, to "Images with the tag {tag}". + */ + image: "{tag} images", + }, + source: { + /** + * This will be the page title and must be SEO friendly. + * {source} will be a dynamic value such as "Wikimedia". We cannot change its case or form. + * You can change the sentence to add more context and make the sentence + * grammatically correct, for instance, to "Audio tracks from {source}". + */ + audio: "{source} audio tracks", + /** + * This will be the page title and must be SEO friendly. + * {source} will be a dynamic value such as "Wikimedia". We cannot change its case or form. + * You can change the sentence to add more context and make the sentence + * grammatically correct, for instance, to "Images from {source}". + */ + image: "{source} images", + }, + }, link: { source: "Open source site", creator: "Open creator page", diff --git a/frontend/src/pages/audio/collection.vue b/frontend/src/pages/audio/collection.vue index 5c26b87bc55..1aa1240e0bc 100644 --- a/frontend/src/pages/audio/collection.vue +++ b/frontend/src/pages/audio/collection.vue @@ -23,10 +23,10 @@ import { computed, ref, watch } from "vue" import { collectionMiddleware } from "~/middleware/collection" import { useSearchStore } from "~/stores/search" - import { useMediaStore } from "~/stores/media" -import type { AudioDetail } from "~/types/media" import { useI18n } from "~/composables/use-i18n" +import { useCollectionMeta } from "~/composables/use-collection-meta" +import type { AudioDetail } from "~/types/media" import VCollectionResults from "~/components/VSearchResultsGrid/VCollectionResults.vue" @@ -72,8 +72,18 @@ export default defineComponent({ fetchMedia({ shouldPersistMedia: true }) } + const { pageTitle } = useCollectionMeta({ + collectionParams, + mediaType: "audio", + i18n, + }) + useMeta({ - meta: [{ hid: "robots", name: "robots", content: "all" }], + meta: [ + { hid: "robots", name: "robots", content: "all" }, + { hid: "og:title", property: "og:title", content: pageTitle.value }, + ], + title: pageTitle.value, }) useFetch(async () => { diff --git a/frontend/src/pages/image/collection.vue b/frontend/src/pages/image/collection.vue index c4afeddae84..c823700a778 100644 --- a/frontend/src/pages/image/collection.vue +++ b/frontend/src/pages/image/collection.vue @@ -28,6 +28,7 @@ import { computed, ref, watch } from "vue" import { collectionMiddleware } from "~/middleware/collection" import { useMediaStore } from "~/stores/media" import { useSearchStore } from "~/stores/search" +import { useCollectionMeta } from "~/composables/use-collection-meta" import { skipToContentTargetId } from "~/constants/window" import { useI18n } from "~/composables/use-i18n" import type { ImageDetail } from "~/types/media" @@ -76,10 +77,20 @@ export default defineComponent({ fetchMedia({ shouldPersistMedia: true }) } - useMeta({ - meta: [{ hid: "robots", name: "robots", content: "all" }], + const { pageTitle } = useCollectionMeta({ + collectionParams, + mediaType: "image", + i18n, }) + useMeta(() => ({ + meta: [ + { hid: "robots", name: "robots", content: "all" }, + { hid: "og:title", property: "og:title", content: pageTitle.value }, + ], + title: pageTitle.value, + })) + useFetch(async () => { await fetchMedia() }) diff --git a/frontend/src/utils/parse-collection-path.ts b/frontend/src/utils/parse-collection-path.ts deleted file mode 100644 index 0c3cd19e19e..00000000000 --- a/frontend/src/utils/parse-collection-path.ts +++ /dev/null @@ -1,24 +0,0 @@ -import { CreatorCollection, SourceCollection } from "~/types/search" - -export function parseCollectionPath( - pathMatch: string -): SourceCollection | CreatorCollection | null { - // Build collection params. - // pathMatch is the part of the path after the collection name: - //`/sourceName` or `/sourceName/creator/creatorName`. - const pathMatchParts = pathMatch - .split("/") - .map((part) => part.trim()) - .filter((part) => part !== "") - - if (pathMatchParts.length === 1) { - return { collection: "source", source: pathMatchParts[0] } - } else if (pathMatchParts.length === 3 && pathMatchParts[1] === "creator") { - return { - collection: "creator", - creator: pathMatchParts[2], - source: pathMatchParts[0], - } - } - return null -} diff --git a/frontend/test/playwright/e2e/seo.spec.ts b/frontend/test/playwright/e2e/seo.spec.ts index 82556fb4187..1d3d8720b98 100644 --- a/frontend/test/playwright/e2e/seo.spec.ts +++ b/frontend/test/playwright/e2e/seo.spec.ts @@ -60,11 +60,34 @@ const pages = { ogTitle: "Openverse", robots: "all", }, + tag: { + url: "/image/collection?tag=cat", + title: "cat images | Openverse", + ogImage: "/openverse-default.jpg", + ogTitle: "cat images | Openverse", + robots: "all", + }, + source: { + url: "/image/collection?source=flickr", + title: "Flickr images | Openverse", + ogImage: "/openverse-default.jpg", + ogTitle: "Flickr images | Openverse", + robots: "all", + }, + creator: { + url: "/image/collection?source=flickr&creator=strogoscope", + title: "strogoscope | Openverse", + ogImage: "/openverse-default.jpg", + ogTitle: "strogoscope | Openverse", + robots: "all", + }, } test.describe("page metadata", () => { for (const openversePage of Object.values(pages)) { test(`${openversePage.url}`, async ({ page }) => { - await preparePageForTests(page, "xl") + await preparePageForTests(page, "xl", { + features: { additional_search_views: "on" }, + }) await page.goto(openversePage.url) await expect(page).toHaveTitle(openversePage.title) const metaDescription = page.locator('meta[name="description"]') diff --git a/frontend/test/unit/specs/utils/validate-collection-params.spec.js b/frontend/test/unit/specs/utils/validate-collection-params.spec.js deleted file mode 100644 index a3d39f7765b..00000000000 --- a/frontend/test/unit/specs/utils/validate-collection-params.spec.js +++ /dev/null @@ -1,48 +0,0 @@ -import { createPinia, setActivePinia } from "~~/test/unit/test-utils/pinia" - -import { parseCollectionPath } from "~/utils/parse-collection-path" -import { useProviderStore } from "~/stores/provider" -import { useFeatureFlagStore } from "~/stores/feature-flag" - -describe("validateCollectionParams", () => { - /** @type { import("pinia").Pinia } **/ - let pinia - - beforeEach(() => { - pinia = createPinia() - setActivePinia(pinia) - useProviderStore().isSourceNameValid = jest.fn(() => true) - useFeatureFlagStore().toggleFeature("additional_search_views", "on") - }) - - it("returns source collection", () => { - const collection = parseCollectionPath("/flickr") - - expect(collection).toEqual({ source: "flickr", collection: "source" }) - }) - - it("returns null if `creator` parameter is blank", () => { - const collection = parseCollectionPath("/flickr/creator/") - - expect(collection).toBeNull() - }) - it("returns creator collection without trailing slash", () => { - const collection = parseCollectionPath("/flickr/creator/me") - - expect(collection).toEqual({ - source: "flickr", - creator: "me", - collection: "creator", - }) - }) - - it("returns creator collection with trailing slash", () => { - const collection = parseCollectionPath("/flickr/creator/me/") - - expect(collection).toEqual({ - source: "flickr", - creator: "me", - collection: "creator", - }) - }) -})