Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Aria label key words of virtual backgrounds are not translatable #9610

Closed
nickvergessen opened this issue May 24, 2023 · 6 comments · Fixed by #9710
Closed

Aria label key words of virtual backgrounds are not translatable #9610

nickvergessen opened this issue May 24, 2023 · 6 comments · Fixed by #9710

Comments

@nickvergessen
Copy link
Member

The words are still not translated, but at least no TODO aria labels anymore. Will have to think about a way to translate them.
Will raise a follow up issue for that.

Originally posted by @nickvergessen in #9609 (comment)

@ShGKme
Copy link
Contributor

ShGKme commented Jun 6, 2023

Currently, background image presets are hardcoded in the source.

What about making a constant with filename-to-description map?

const presetbackgroundLabels = {
  '1_office': t('spreed', 'Office'),
  '2_home': t('spreed', 'Home'),
  // ...
}

@ShGKme
Copy link
Contributor

ShGKme commented Jun 6, 2023

@marcoambrosini @Antreesy what do you think?

@Antreesy
Copy link
Contributor

Antreesy commented Jun 6, 2023

What about making a constant with filename-to-description map?

As far as I know, It will be impossible for translators to translate differently Office and Select virtual {{insert Office here}} background.
For Russian and German langs, for example, inserted part will be in the end and in different case (Fall 🇩🇪 / падеж 🇷🇺 )

But constant with complete sentence should work fine, and doesn't require a lot of efforts from translators:
'1_office': t('spreed', 'Select virtual Office background'),

@ShGKme
Copy link
Contributor

ShGKme commented Jun 6, 2023

Currently, background image presets are hardcoded in the source.

I forgot that they are actually got from

getCapabilities()?.spreed?.config?.call?.['predefined-backgrounds']

Which is defined on runtime according to the img/backgrounds/ content

spreed/lib/Capabilities.php

Lines 176 to 205 in b8eb265

$predefinedBackgrounds = $this->talkCache->get('predefined_backgrounds');
if ($predefinedBackgrounds !== null) {
// Try using cached value
$predefinedBackgrounds = json_decode($predefinedBackgrounds, true);
}
if (!is_array($predefinedBackgrounds)) {
// Cache was empty or invalid, regenerate
$predefinedBackgrounds = [];
if (file_exists(__DIR__ . '/../img/backgrounds')) {
$directoryIterator = new \DirectoryIterator(__DIR__ . '/../img/backgrounds');
foreach ($directoryIterator as $file) {
if (!$file->isFile()) {
continue;
}
if ($file->isDot()) {
continue;
}
if ($file->getFilename() === 'COPYING') {
continue;
}
$predefinedBackgrounds[] = $file->getFilename();
}
sort($predefinedBackgrounds);
}
$this->talkCache->set('predefined_backgrounds', json_encode($predefinedBackgrounds), 300);
}
$capabilities['config']['call']['predefined-backgrounds'] = $predefinedBackgrounds;

End-user is not supposed to edit the app's source, but it is possible.

@Antreesy
Copy link
Contributor

Antreesy commented Jun 6, 2023

End-user is not supposed to edit the app's source, but it is possible.

The new constant is not exposed, so shouldn't be a big problem, i guess.
We could send translations and filepath together, via capabilities, in PHP-code (it supports t('spreed'), if I'm not wrong). But it's going to be hardcoded in another place then.

@ShGKme
Copy link
Contributor

ShGKme commented Jun 6, 2023

The new constant is not exposed, so shouldn't be a big problem, i guess.
We could send translations and filepath together, via capabilities, in PHP-code (it supports t('spreed'), if I'm not wrong). But it's going to be hardcoded in another place then.

It is changeable not by constant editing in the PHP/JS source code, but by updating files in the img/backgrounds/ folder. Translations on the server side cannot help here either.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants