Use inline_templates in TranslateEditForm::buildForm() and PluralString::getTranslationElement()

Looks like this is not being worked on, so removing from sprint. Please add it back if so.

1. +++ b/core/modules/config_translation/src/FormElement/PluralVariants.php
   @@ -23,23 +23,28 @@ class PluralVariants extends FormElementBase {
   +      '#title' => $environment->renderInline($title_template, $title_context),
   ...
   +        '#markup' => $environment->renderInline($template, $context),
   
   @@ -51,12 +56,15 @@ protected function getSourceElement(LanguageInterface $source_language, $source_
   +      '#title' => $environment->renderInline($title_template, $title_context),
   
   +++ b/core/modules/locale/src/Form/TranslateEditForm.php
   @@ -72,9 +76,7 @@ public function buildForm(array $form, FormStateInterface $form_state) {
   +            '#markup' => $environment->renderInline($value_template, array('value' => $source_array[0])),
   
   @@ -83,21 +85,21 @@ public function buildForm(array $form, FormStateInterface $form_state) {
   +            '#markup' => $environment->renderInline($value_template, array('value' => $source_array[0])),
   ...
   +            '#markup' => $environment->renderInline($value_template, array('value' => $source_array[1])),
   
   @@ -132,7 +138,7 @@ public function buildForm(array $form, FormStateInterface $form_state) {
   +              '#prefix' => $i == 0 ? $environment->renderInline($prefix_template, $prefix_context) : '',
   

   I think in most places you can leave that as render array (no need to render)

2. +++ b/core/modules/config_translation/src/FormElement/PluralVariants.php
   @@ -23,23 +23,28 @@ class PluralVariants extends FormElementBase {
        for ($i = 0; $i < $plurals; $i++) {
   +      $template = '<span lang="{{ langcode }}">{{ value }}</span>';
   
   +++ b/core/modules/locale/src/Form/TranslateEditForm.php
   @@ -125,6 +127,10 @@ public function buildForm(array $form, FormStateInterface $form_state) {
              for ($i = 0; $i < $plurals; $i++) {
   +            $prefix_template = '<span class="visually-hidden">{{ translation_language }}</span>';
   

   makes sense to move template definition out of loop

@maxocub please try to change render array to prevent rendering, it looks really strange that we need render in build

It's totally fine, nice cleanup :)

Added beta evaluation template.

Tested that works correctly.

+++ b/core/modules/config_translation/src/Tests/ConfigTranslationUiTest.php
@@ -665,10 +665,10 @@ public function testPluralConfigStringsSourceElements() {
-          $this->assertRaw('edit-source-config-names-viewsviewfiles-display-default-display-options-fields-count-format-plural-string-' . $index);
+          $this->assertRaw('edit-source-config-format-plural-string-' . $index);

Why is this needed? If we are changing the HTML element ids this way, we may have duplicated HTML ids which makes our HTML invalid.

1. +++ b/core/modules/config_translation/src/FormElement/PluralVariants.php
   @@ -25,21 +25,26 @@ protected function getSourceElement(LanguageInterface $source_language, $source_
        $element = array(
          '#type' => 'fieldset',
   -      '#title' => SafeMarkup::format('@label <span class="visually-hidden">(@source_language)</span>', array(
   -        '@label' => $this->t($this->definition->getLabel()),
   -        '@source_language' => $source_language->getName(),
   -      )),
   +      '#title' => $this->t($this->definition->getLabel()),
          '#tree' => TRUE,
        );
   +    $element['source_language'] = array(
   +      '#type' => 'inline_template',
   +      '#template' => '<span class="visually-hidden">{{ source_language }}</span>',
   +      '#context' => array(
   +        'source_language' => $this->t('Source string (@language)', array('@language' => $source_language->getName())),
   +      ),
   +    );
   
   @@ -53,12 +58,16 @@ protected function getTranslationElement(LanguageInterface $translation_language
        $element = array(
          '#type' => 'fieldset',
   -      '#title' => SafeMarkup::format('@label <span class="visually-hidden">(@translation_language)</span>', array(
   -        '@label' => $this->t($this->definition->getLabel()),
   -        '@translation_language' => $translation_language->getName(),
   -      )),
   +      '#title' => $this->t($this->definition->getLabel()),
          '#tree' => TRUE,
        );
   +    $element['translation_language'] = array(
   +      '#type' => 'inline_template',
   +      '#template' => '<span class="visually-hidden">{{ translation_language }}</span>',
   +      '#context' => array(
   +        'translation_language' => $this->t('Translated string (@language)', array('@language' => $translation_language->getName())),
   +      ),
   +    );
   

   The modified code seems to be doing a different thing now? The fieldset title is the definition label and then it has a child element instead of the title contain the text?

2. +++ b/core/modules/locale/src/Form/TranslateEditForm.php
   @@ -110,13 +102,20 @@ public function buildForm(array $form, FormStateInterface $form_state) {
   +        $form['strings'][$string->lid]['translations'] = array(
   +          '#type' => 'inline_template',
   +          '#template' => '<span class="visually-hidden">{{ translation_language }}</span>',
   +          '#context' => array(
   +            'translation_language' => $this->t('Translated string (@language)', array('@language' => $langname)),
   +          ),
   +        );
   +
            // Approximate the number of rows to use in the default textarea.
            $rows = min(ceil(str_word_count($source_array[0]) / 12), 10);
            if (!$plural) {
              $form['strings'][$string->lid]['translations'][0] = array(
                '#type' => 'textarea',
   -            '#title' => $this->t('Translated string (@language)', array('@language' => $langname)),
   -            '#title_display' => 'invisible',
                '#rows' => $rows,
                '#default_value' => $translation_array[0],
                '#attributes' => array('lang' => $langcode),
   @@ -132,7 +131,6 @@ public function buildForm(array $form, FormStateInterface $form_state) {
   
   @@ -132,7 +131,6 @@ public function buildForm(array $form, FormStateInterface $form_state) {
                  '#rows' => $rows,
                  '#default_value' => isset($translation_array[$i]) ? $translation_array[$i] : '',
                  '#attributes' => array('lang' => $langcode),
   -              '#prefix' => $i == 0 ? ('<span class="visually-hidden">' . $this->t('Translated string (@language)', array('@language' => $langname)) . '</span>') : '',
                );
              }
   

   Same here, the template seems to be in a different structure?!

All righto, I misread something then, let's get this in :) Thanks a lot!

+++ b/PluralVariants.html
@@ -2,15 +2,15 @@
+      <label>Singular form</label>
...
+      <label>First plural form</label>

+++ b/TranslateEditForm.html
@@ -1,11 +1,11 @@
+      <label>Singular form</label>
...
+      <label>Plural form</label>

Aren't these missing the for attribute - I think that for was wrong before though - so not sure that this matters.

This issue is being reviewed by the kind folks in Slack, #need-reveiw-queue. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge require as a guide.

Taking a look at the code for 10.1 and I see now it's using FormattableMarkup() does that resolve the issue here?

If so think this can be closed and credit moved over to #2958429: Properly deprecate SafeMarkup::format()

Just following up if still valid? If no follow up could close in 3 months

This was fixed apparently in #2958429: Properly deprecate SafeMarkup::format() and #1938916: Convert locale theme tables to table #type, but it's hard to track 😅
I reviewed HEAD and definitely not an issue anymore.