Skip to content

feat(web): video player i18n#28192

Open
meesfrensel wants to merge 1 commit intomainfrom
feat/video-player-i18n
Open

feat(web): video player i18n#28192
meesfrensel wants to merge 1 commit intomainfrom
feat/video-player-i18n

Conversation

@meesfrensel
Copy link
Copy Markdown
Collaborator

Description

Follow-up PR to the custom video player. This is not the solution we wanted, but the media-chrome team is not interested in weblate or adding many languages in one go (see muxinc/media-chromenumber#1290), so adding the strings to our own weblate translations is an alternative solution.

I copied over the existing translations they have upstream.

Upsides of the specific implementation of spreading ...En (the english strings) and then overriding some of them:

  • We only translate the strings that we need
  • If new strings are added upstream, we don't get type errors (because those new keys are missing from the object)

Downsides:

  • If new strings are added upstream, we won't know unless we actively check all releases for this, or get a report of missing translations after an Immich release.

How Has This Been Tested?

Change to all these languages, and another one (fallback remains english)

  • Check strings are translated
  • Check strings fit within their containers (needed to change the settings menu to min-w to fit long German words)
English German
image image

Please describe to which degree, if any, an LLM was used in creating this pull request.

None

Comment thread i18n/de.json
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't translate non-english languages

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not about the source, it's about merge conflicts with weblate that are annoying to fix. For us it'd be much easier if you could just add them on weblate after this PR is merged

Comment thread web/src/routes/+layout.svelte Outdated
});

addTranslation($lang, {
...En,
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO it's better to not fall back to the library's translations at all and just have a type error when upgrading instead. That way I can easily pick up on it when working through renovate PRs and we have a guarantee of being fully translatable

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, I have added a few translation strings that I foresee we'll maybe use in the near future (quality selection ones, captions), and deferred the others to upstream via En[{{key}}]. Does that make sense?

re: types, removing a key value pair now results in a type error.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't realize there were this many. What do you think about having a const defaults: keyof typeof En = ['Some key', 'Some other key'] and then you go ...Object.fromEntries(defaults.map((k) => [k, k])) and spread that into the object?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't make that work with typing unfortunately, all the defaults in defaults are marked not present in the object. If you have a solution, sure that sounds good, but I don'tknow how to :)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  const videoPlayerDefaults = [
    'A media error caused playback to be aborted. The media could be corrupt or your browser does not support this format.',
    'Audio',
  ] as const satisfies Array<keyof typeof En>;

  const foo = Object.fromEntries(videoPlayerDefaults.map((key) => [key, key])) as {
    [K in (typeof videoPlayerDefaults)[number]]: K;
  };

as const satisfies is probably the magic sauce you were missing. It's not pretty; most likely people who actually know what they're doing could do this more cleanly. It's what I had in mind though and it seems to be working at least 😅

@meesfrensel meesfrensel force-pushed the feat/video-player-i18n branch from 45f6150 to 9094190 Compare May 4, 2026 12:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants