feat(html): i18n#1591
Open
sampotts wants to merge 32 commits into
Open
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This was referenced May 25, 2026
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
sampotts
force-pushed
the
feat/i18n-locale-packs
branch
from
0da2fa9 to
08a4f50
Compare
sampotts
force-pushed
the
feat/i18n-locale-packs
branch
from
08a4f50 to
98c82df
Compare
sampotts
force-pushed
the
feat/i18n-locale-packs
branch
from
98c82df to
66f0283
Compare
sampotts
force-pushed
the
feat/i18n-locale-packs
branch
from
66f0283 to
33113b0
Compare
Compose i18n provider onto SkinElement, resolve playback-rate tooltips via getResolvedLabel, and normalize CDN i18n entry paths on Windows. Co-authored-by: Cursor <cursoragent@cursor.com>
Re-check lazy-load sequence after getBrowserTranslations resolves so a superseded locale cannot register browser translations globally. Co-authored-by: Cursor <cursoragent@cursor.com>
Keep tooltip copy visible before media state attaches by translating the trigger label when getResolvedLabel returns undefined. Co-authored-by: Cursor <cursoragent@cursor.com>
Adds integration coverage that play button aria-label updates when document.documentElement.lang changes and the i18n provider has no explicit lang attribute. Co-authored-by: Cursor <cursoragent@cursor.com>
Use mergeLocaleOverlays loadedTags instead of merged overlay so English-only lazy merges no longer block browser translation fallback. Co-authored-by: Cursor <cursoragent@cursor.com>
mihar-22
approved these changes
Jul 7, 2026
| * via `adoptedStyleSheets` (or `<style>` fallback). | ||
| */ | ||
| export class SkinElement extends ReactiveElement { | ||
| export class SkinElement extends I18nProviderMixin(ReactiveElement) { |
Member
There was a problem hiding this comment.
Rough thought but this is a possible smell to me in that it needs to be baked in and not an optional higher-level concern. Largely related to my comments on #1708.
In other words, I think i18n should be composed in rather than provided out of the box but this circles back to conversation around keys because a default english translation is required since keys have no meaning on their own.
| readonly #consumer: ContextConsumer<I18nContext, ReactiveControllerHost & HTMLElement>; | ||
| #unsubscribeRegistry: (() => void) | undefined; | ||
|
|
||
| constructor(host: ReactiveControllerHost & HTMLElement, context: I18nContext) { |
Member
There was a problem hiding this comment.
(suggestion): pull ReactiveControllerHost & HTMLElement up into a I18nControllerHost type
Comment on lines
+59
to
+61
| export namespace I18nController { | ||
| export type Constructor = new (host: ReactiveControllerHost & HTMLElement, context: I18nContext) => I18nController; | ||
| } |
Member
There was a problem hiding this comment.
I think this can be Constructor = typeof I18nController, also good to add Host = I18nControllerHost to the namespace
Comment on lines
+19
to
+32
| export function createI18n(options?: CreateI18nOptions): CreateI18nResult { | ||
| const ProviderMixin = createI18nProviderMixin({ | ||
| i18nContext, | ||
| loadLocale: options?.loadLocale, | ||
| }); | ||
| const TextMixin = createTextMixin({ i18nContext }); | ||
|
|
||
| return { | ||
| context: i18nContext, | ||
| I18nController, | ||
| ProviderMixin, | ||
| TextMixin, | ||
| }; | ||
| } |
Comment on lines
+4
to
+7
| /** DOM `lang` values are untyped strings; align with core {@link Locale} at the boundary. */ | ||
| export function localeFromDomLang(raw: string | undefined): Locale | undefined { | ||
| return (domLangFromString(raw) as Locale) ?? undefined; | ||
| } |
Member
There was a problem hiding this comment.
we could make the return type generic on domLangFromString, or we could move it out of from utils for now if it's only required in the html package.
Comment on lines
+10
to
+12
| export function resolvePlayerLocale(explicit: Locale | undefined, inherited: Locale | undefined): Locale { | ||
| return effectiveLocale(explicit, inherited) as Locale; | ||
| } |
Member
There was a problem hiding this comment.
same comments as localeFromDomLang
Comment on lines
+26
to
+27
| /** Override lazy loading of shipped locale packs (tests or custom loaders). */ | ||
| loadLocale?: LocaleLoader | undefined; |
| import type { LocaleLoader, ReactiveElementMixinBase } from './types'; | ||
|
|
||
| export interface I18nProviderConfig { | ||
| i18nContext: I18nContext; |
| export type I18nTextMixin = <Base extends ReactiveElementMixinBase>(Base: Base) => Constructor<ReactiveElement> & Base; | ||
|
|
||
| export interface TextMixinConfig { | ||
| i18nContext: I18nContext; |
Member
There was a problem hiding this comment.
Maybe it'd help if we had a base radio group element that the rest can extend and share basic logic on
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Refs #222
Closes #1365
Closes #1367
Summary
Adds the HTML i18n layer with ambient
<html lang>inheritance, browser translation fallback, defaultloadLocale, and translated labels across web component controls.Changes
createI18n()—I18nController,ProviderMixin,TextMixin,<media-i18n-provider>,<media-text>aria-labels and tooltips viaI18nController@videojs/html/i18nand@videojs/html/cdn/i18nStack info
PR 3 of 5 — stacks on locale packs PR.
Testing
pnpm -F @videojs/html build pnpm -F @videojs/html test src/i18nMade with Cursor
Note
Medium Risk
Touches many player UI and accessibility strings across skins and controls; async locale/browser loading adds edge cases, though covered by new tests.
Overview
Adds an HTML i18n layer on top of
@videojs/core/i18n:createI18n()exposes Lit context,I18nController, provider/text mixins, and registers<media-i18n-provider>/<media-text>. Locale resolution follows explicitlang, ambient<html lang>, lazy shipped locale packs, registry overlays, and optional browser translation (with sequence guards on async loads).Players and
SkinElementare wrapped withI18nProviderMixinso skins and controls share a translator. UI elements consumeI18nControllerand useresolveControlAttrs/resolveControlLabelfor translatedaria-labels, tooltips, error-dialog copy, time remaining phrases, and input-indicator labels. Skin templates drop hardcoded error-dialog strings in favor of runtime-filled elements.Packaging & CDN: new
@videojs/html/i18nand locale export paths,build:cdnruns locale generation pluscdnI18nExternalPlugin(sharedi18n.json jsDelivr in prod, relative in dev), and the video CDN entry pulls indefine/i18n.Reviewed by Cursor Bugbot for commit 85e56ff. Bugbot is set up for automated code reviews on this repo. Configure here.