The "Other" option is not translatable. In src/Element/ElementBase.php, "Other" text is static instead of being added using the "t" function.

Instead of:

protected static function addOtherOption($options) {
  $options['select_or_other'] = 'Other';
  return $options;
}

it should be 

protected static function addOtherOption($options) {
  $options['select_or_other'] = t('Other');
  return $options;
}
CommentFileSizeAuthor
#17 2926786-17.patch3.3 KBTakuma Shimabukuro
#15 2926786-15.patch510 byteshmendes
#8 make_other_option_translatable-2926786-8.patch746 bytesmariusilie
#4 make_other_option_translatable-2926786-4.patch466 bytesmariusilie
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

mariusilie created an issue. See original summary.

mariusilie’s picture

mariusilie CreditAttribution: mariusilie commented
Issue summary: View changes
legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
CreditAttribution: legolasbo at Ordina Digital Services commented

Thanks for reporting the issue @mariusilie. Feel free to supply a patch for a quick resolution of the issue.

mariusilie’s picture

mariusilie CreditAttribution: mariusilie as a volunteer commented
FileSize
make_other_option_translatable-2926786-4.patch466 bytes

I think this should do the trick.

legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
CreditAttribution: legolasbo at Ordina Digital Services commented
Version: 8.x-3.x-dev » 8.x-1.x-dev

8.x-3.x is not being developed

mariusilie’s picture

mariusilie CreditAttribution: mariusilie as a volunteer commented
legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
CreditAttribution: legolasbo at Ordina Digital Services commented
Status: Active » Needs work 
+++ b/src/Element/ElementBase.php
@@ -25,7 +25,7 @@ abstract class ElementBase extends FormElement {
+    $options['select_or_other'] = t('Other');

Since this is part of a class it should use $this->t(). If $this->t() isn't available, the class should use StringTranslationTrait.

mariusilie’s picture

mariusilie CreditAttribution: mariusilie as a volunteer commented
Status: Needs work » Active
FileSize
make_other_option_translatable-2926786-8.patch746 bytes

Since this is an abstract class and $this can't be used, I had to use a TranslatableMarkup object as per StringTranslationTrait's documentation:

When possible, use the \Drupal\Core\StringTranslation\StringTranslationTrait $this->t(). Otherwise create a new \Drupal\Core\StringTranslation\TranslatableMarkup object.

legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
CreditAttribution: legolasbo at Ordina Digital Services commented
Status: Active » Needs work

You should be able to do something like this:

abstract class ElementBase extends FormElement {

  use StringTranslationTrait;

And then $this->t() should become available.

Side note: The issue status should be 'Needs review" when an issue has a patch that needs review.

mariusilie’s picture

mariusilie CreditAttribution: mariusilie as a volunteer commented

I tried the exact thing but when I used $this->t(), I got an error:

Error: Using $this when not in object context in Drupal\select_or_other\Element\ElementBase::addOtherOption() (line 32 of modules\contrib\select_or_other\src\Element\ElementBase.php)

That's why I used a TranslatableMarkup object.

legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
CreditAttribution: legolasbo at Ordina Digital Services for Sijthoff Media commented
Status: Needs work » Reviewed & tested by the community

Allright, RTBC then.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 8: make_other_option_translatable-2926786-8.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

mariusilie’s picture

mariusilie CreditAttribution: mariusilie as a volunteer commented

I think the test needs to be updated also. Looks like it checks for static "Other" string.

$this->assertArrayEquals(['select_or_other' => "Other"], $options);
legolasbo’s picture

legolasbo
he/him
Primary language Dutch
Location Middelburg
CreditAttribution: legolasbo at Ordina Digital Services for Sijthoff Media commented

You're right, the test does need updating :)

hmendes’s picture

hmendes CreditAttribution: hmendes at CI&T commented
Status: Needs work » Needs review
FileSize
2926786-15.patch510 bytes

Adding a patch for the comment #13. The initial problem seems to be fixed on #2997264: Translation support for the "Other" option.

Takuma Shimabukuro’s picture

Takuma Shimabukuro CreditAttribution: Takuma Shimabukuro at CI&T commented
Assigned: Unassigned » Takuma Shimabukuro
Takuma Shimabukuro’s picture

Takuma Shimabukuro CreditAttribution: Takuma Shimabukuro at CI&T commented
Assigned: Takuma Shimabukuro » Unassigned
FileSize
2926786-17.patch3.3 KB

HI, I reviewed #15's patch and it's looks fine, but i ran the phpcs command, it shows some errors so i fixed it and attached the new patch for someone can review

daften’s picture

daften CreditAttribution: daften at Dropsolid commented

@Takuma Can you open a new issue for coding standards next time. It becomes harder to evaluate functional changes if you mix them with coding standards that are not related to the functional change. I will evaluate it with the changes incorporated to not lose work this time. Thanks :)

  • daften committed e67e619 on 4.x authored by hmendes 
    Issue #2926786 by mariusilie, Takuma Shimabukuro, hmendes, legolasbo:...
daften’s picture

daften CreditAttribution: daften at Dropsolid commented
Status: Needs review » Fixed

Commited, thanks for the patch all!

daften’s picture

daften CreditAttribution: daften at Dropsolid commented
Version: 8.x-1.x-dev » 4.x-dev

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.