Add SEO properties to collection pages #3999
Conversation
Signed-off-by: Olga Bulat <obulat@gmail.com>
openverse-bot
added
🟧 priority: high
Co-authored-by: Madison Swain-Bowden <bowdenm@spu.edu>
There was a problem hiding this comment.
Requesting changes for just a couple specific things, with a few others left in the air open for you to decide. The specific changes are: clarify the purpose of the string and simplify the translator comment, with the goal of making it clear that it's a browser page title and needs to be SEO friendly; and interpolate {openverse} the way we do in other places.
POT generation needs to be checked to make sure it doesn't get confused about the | character.
| /** | ||
| * {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 | Openverse", | ||
|
|
||
| /** | ||
| * {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 | Openverse", | ||
| }, | ||
| source: { | ||
| /** | ||
| * {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 | Openverse", | ||
| /** | ||
| * {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 | Openverse", |
There was a problem hiding this comment.
Might be worth mentioning in these comments that they are used as browser page titles in particular. That might help clarify a preference for a succinct string rather than something fully descriptive or strictly gramatically correct, rather, it needs to be SEO friendly (I imagine length is a factor here). We can probably leave out the "You can change the sentence...", so that it's clear it does not need to be a complete sentence, but rather clarifying that it must be an SEO friendly phrase?
We normally interpolate Openverse as a variable. Any reason not to do that here?
Also, in Glotpress the curly praces are replaced with ###, so it might be better to just quote "tag" (etc.) than reference it with the curly braces, which only make sense in the context of the original string.
Just suggestions below, nothing blocking here other than the Openverse intepolation if we should keep it.
Also: will the POT transformer get confused about the | in the string and think it's a plural form string, by any chance? Maybe that also needs to be interpolated or included in {openverse} as " | Openverse with the leading space? It could be better to have the strings here only be the titles, leaving out the | Openverse part altogether, and then interpolating the translated result into a plain template string, like `${i18n.t("pageTitle.tag.image", { tag })} | Openverse`? That way there is no chance the POT transformer gets confused about whether these are plural form strings and translators don't need to worry about or focus on spacing of the pipe and Openverse. If we need to use a template for the whole thing for e.g., RTL, then we can use two separate templates and interpolate them into each other, with a generic template for the page-title form: "{pageTitle} {pipe} {openverse}" for example. And then a note to the translator explaining what each variable will be and to reorder it as needed with Openverse always separated from the actual title by the pipe.
| /** | |
| * {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 | Openverse", | |
| /** | |
| * {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 | Openverse", | |
| }, | |
| source: { | |
| /** | |
| * {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 | Openverse", | |
| /** | |
| * {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 | Openverse", | |
| /** | |
| * "tag" will be a dynamic value such as "cat". We cannot change its case or form. | |
| * Used as browser page title and must be SEO friendly | |
| */ | |
| audio: "{tag} audio tracks | {openverse}", | |
| /** | |
| * "tag" will be a dynamic value such as "cat". We cannot change its case or form. | |
| * Used as browser page title and must be SEO friendly | |
| */ | |
| image: "{tag} images | {openverse}", | |
| }, | |
| source: { | |
| /** | |
| * "source" will be a dynamic value such as "Wikimedia Commons". We cannot change its case or form. | |
| * Used as browser page title and must be SEO friendly | |
| */ | |
| audio: "{source} audio tracks | {openverse}", | |
| /** | |
| * "source" will be a dynamic value such as "Wikimedia Commons". We cannot change its case or form. | |
| * Used as browser page title and must be SEO friendly | |
| */ | |
| image: "{source} images | {openverse}", |
| const providerStore = useProviderStore() | ||
|
|
||
| if (collectionParams.value?.collection === "creator") { | ||
| return `${collectionParams.value.creator} | Openverse` |
There was a problem hiding this comment.
For example, in RTL this might also need to be changed? I'm not sure how a browser configured for RTL and with dir="rtl" set on a page will interpret a browser title that isn't formatted RTL? Maybe page titles cannot be RTL at all, do you know?
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 <obulat@gmail.com>
Add suggestions from code review Update the strings: add comment that the phrases will be used for page titles Signed-off-by: Olga Bulat <obulat@gmail.com>
Thank you for your review, these points are really important. For other page titles, we use i18n to translate the title itself, and add the I've refactored the code to do the same here. This way, we don't have to interpolate openverse, and the I added the notes that the phrases will be used as page title and that they must be SEO-friendly. I didn't remove the comment about a different sentence, though, because we don't want the title to be ungrammatical simply to be SEO-friendly. I have seen the translations done that way in other apps, and I am strongly against doing this in Openverse. |
| const pageTitle = computed(() => { | ||
| // The default page title. It should be overwritten by the specific collection type. | ||
| let title: string | TranslateResult = | ||
| "Openly Licensed Images, Audio and More" |
There was a problem hiding this comment.
Is this used as a fallback to appease TypeScript? If so, we can use a return in each case instead of assigning to a variable. Otherwise, I suppose this should be translated too, in case for some reason it is used? Not-blocking, but definitely needs an issue to fast-follow.
There was a problem hiding this comment.
Refactored to not use string | TranslateResult.
I tried using the switch/case, but because the function is inside a computed, it causes an Expected to return a value in computed function vue/return-in-computed-property linting error, even though all of the collectionParams.collection options are exhausted. That's why I used a redundant if cause.
Fixes
Fixes #3917 by @obulat
Description
This PR adds the titles to the collection pages (
titleused at the top of a page andog:titlethat's used when sharing the page). The following titles were used, suggested by @zackkrida :I also added the comments to i18n strings to give some context for translators.
Testing Instructions
Run the app locally using
just frontend/run devSwitch the
additional_search_viewsflag on onhttp://localhost8443/preferencesCheck the
<head>of the collection pages. It should have correcttitleandog:titleset. This is also tested in the playwright seo test.If you have any suggestions for wording of the titles or the translator comments, please add them here, too.
Checklist
Update index.md).main) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin