feat(core): add og:locale and og:locale:alternative meta tags #8894
Conversation
✅ [V2]Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site settings. |
⚡️ Lighthouse report for the deploy preview of this PR
|
packages/docusaurus-theme-classic/src/theme/SiteMetadata/index.tsx
Outdated
Show resolved
Hide resolved
packages/docusaurus-theme-classic/src/theme/SiteMetadata/index.tsx
Outdated
Show resolved
Hide resolved
packages/docusaurus-theme-classic/src/theme/SiteMetadata/index.tsx
Outdated
Show resolved
Hide resolved
|
I am having some issues with git. I need some time |
NotHr
changed the title
| })} | ||
| hrefLang={htmlLang} | ||
| /> | ||
| <> |
There was a problem hiding this comment.
I think you have to do <Fragment key={locale}> here instead
There was a problem hiding this comment.
Yes 👍
@NotHr I think you could loop twice instead of using a fragment.
Probably not a big deal but it would look cleaner if hrefLang and og:locale:alternate were not "interleaved" in the final html markup.
| ))} | ||
|
|
||
| <meta property="og:locale" content={defaultLocale} /> |
There was a problem hiding this comment.
We should use the currentLocale (and not the defaultLocale) as bcp47 code (htmlLang attribute like in hreflang links), but converted to og locale format (see #8887 (comment))
| })} | ||
| hrefLang={htmlLang} | ||
| /> | ||
| <meta property="og:locale:alternate" content={locale} /> |
There was a problem hiding this comment.
Same, we should use the locale bcp47 code (htmlLang attribute like in hreflang links), but converted to og locale format (see #8887 (comment))
Also, it seems OpenGraph does not recommend outputting the current locale as an alternate
<meta property="og:locale" content="en_GB" />
<meta property="og:locale:alternate" content="fr_FR" />
<meta property="og:locale:alternate" content="es_ES" />
<meta property="og:locale:alternate" content="en_GB" /> ❌
Josh-Cena
changed the title
Josh-Cena
added
the
pr: polish
|
@Josh-Cena - this PR doesn't have any activity since April and #9152 was already addressing the feedback here. Is there anything I can do to help merge this? |
|
@NotHr Do you plan to address the feedback above? @FlorinaPacurar Sorry for the confusion. For large PRs we usually close if it goes unresponsive, but for small ones, sometimes the maintainer just self-applies the change and merges. We'll wait a while and if there's still no response we can reopen yours since you were second to claim it. |
|
Hey 👋 Since @NotHr doesn't answer/update, let's move on and reopen @FlorinaPacurar PR. @FlorinaPacurar can you take a look at my comments on this PR and make sure they are addressed on your PR too? |
Pre-flight checklist
Related issues/PRs
Fix #8887