Read more still not translated.

Looks like this was reverted with some other code in 9beaadf7. Let me review

Figured out why that got reverted. The way the t() was used caused an PHPCS issue. I created this patch that 1) removes the unused link, and 2) uses t() in the proper vein.

Taking a look.

Thanks for the patch. Quick question:

+++ b/src/Plugin/Field/FieldFormatter/SmartTrimFormatter.php
@@ -268,7 +267,9 @@ class SmartTrimFormatter extends FormatterBase {
+          $more = $this->t('@more_text', [
+            '@more_text' => $this->getSetting('more_text'),
+          ]);

I've only seen placeholders used along with hard-coded text, not alone, in the t(). What was the motivation for this structure? Were you trying to make it more readable or something else?

For example, from includes/bootstrap.inc is the structure of what I'm used to seeing with a placeholder:

t("@name's blog", array('@name' => user_format_name($account)));

I did find one example in core in lib/Drupal/Core/Entity/Plugin/EntityReferenceSelection/SelectionBase.php:

 $fields[$field_name] = $this->t('@label', array('@label' => $field_definition->getLabel()));

but this confuses me because, from what I remember, you don't translate the placeholders, just the hard-coded text. I guess I need to brush up on my APIs (and get a lot more sleep).

I'm going to try out the patch given that it seems fine to have that structure.

@PapaGrande Yeah. My brain cells aren't firing well (little sleep) but I'm about to manually test it right now. Thanks!

Manually tested version 8.x-1.2 on simplytest.me as follows:

1. Enabled Interface Translation module
2. Added additional language (Spanish)
3. Updated the formatter for Article Body to be "Smart trimmed"
4. Enabled "Display more link?" and changed "More link text" to "Bring it on!"
5. Set the "Trim length" to 10
6. Added Article content
7. See the "Bring it on!" link
8. Strangely, clicking on "Bring it on!" doesn't open the body content like I was expecting

I'll try it on a different field next.

Ok, I don't know what I'm doing wrong (custom field doesn't work either) but I'll give up the issue so someone else can keep this going.

Found this on the project page:

The "More" link functionality may not make sense in many contexts, and may be redundant in situations where "Read More" is included in set of links included with the node. But it's there if you need it.

I was assuming the more would toggle the text. Whoops. I really want a nap :)

Testing the translation, I can only translate @more_text, which is what I suspected. Ideally, I'd want to translate the text that was added by the user which means then the placeholder shouldn't be used. I have the more text on the body field and a custom field with different text in English but I can only translate @more_text so all the more text ends up being the same in each language. Seen screenshots. IMO, the original patch that did not using the placeholder is the desired approach. Sorry for so many comments, my brain is kind of mush right now.

Thanks for the great work Kristen. The reason I am doing the

$more = $this->t('@more_text', [
  '@more_text' => $this->getSetting('more_text'),
]);

is that when I don't, I get a PHPCS notification "Only string literals should be passed to t() where possible". Plus this runs the text through formatting checks.

I think your tests are great but you're testing '@more_text' which really will be replaced by whatever you put in the more_text configuration of your field..

I am gonna be honest here and state that I have never actually tested this functionality in a multilingual environment, so this is some appreciated work!

Thinking about it more. Seems like this should be config translation like for views text. I'll try to dig around for that approach unless someone beats me to it.

I've been reviewing how configuration translation works for various modules as I think that is the correct approach for here since you want to allow translation of user-inputted information which is what configuration translation is for.

If you had a settings form for the module to configure things such as site information, then the translate tab can be provided with that and used to translate the translatable things. Smart Trim embeds the configurable text within manage display so it's trickier. It's more similar to views as there are different displays for views to configure things similar to the different display modes on the manage display tab.

I was trying to grok how views was implementing the configuration translation support but don't understand it yet. Note that views has the translation tab with the views page/form which makes sense. Since Smart Trim config isn't on a dedicated page, then I think you'd need to add a settings page for this so it can have the tab off of that.

I'm wondering if there is an issue somewhere for making manage display settings translatable. I'll search around.

I think I found the relevant issue for translating display mode configuration via configuration translation:

#2546212: Entity view/form mode formatter/widget settings have no translation UI

I tried out a couple patches from:

#2546212: Entity view/form mode formatter/widget settings have no translation UI

because I wanted to see if it would automagically handle contrib modules like Smart Trim but it wasn't working on simplytest.me so I'd have to test locally. My laptop is running out of space and generally sluggish so not sure when I'll get around to that. Maybe someone can take it for a whirl?

1. Use core 8.8.6
2. Try patch from https://www.drupal.org/project/drupal/issues/2546212#comment-13451232
3. Enable Configuration Translation module
4. Enable Smart Trim module
5. Add at least one language (/admin/config/regional/language)
6. Go to configuration translation page (/admin/config/regional/config-translation)
7. Click on "List" button for "Content view display"
8. Click on "Translate" button for "Default" for Article content type
9. See what is on the configuration translation form for the Article Default view mode
10. If there is nothing for Smart Trim shown (likely), go to Article manage display and configure something for Smart Trim and then go back to 8 and see if anything is there now

Thanks to this great todo list, I made it on simplytest.me:
https://stm5eb17f2e9107d-bfzg0xzgcu9taxwzn3euc4u4b2gpbp8f.tugboat.qa/adm...

I got a "You are not authorized to access this page." after step 8. Even after applying step 10 and a cache flush.

Thanks. That was the problem I had on simplytest.me too. I commented on that issue about it. I was hoping someone would go through the steps on their local instead. (My computer is nearly out of space so hard for me to test locally at the moment.)

I was able to get the configuration translation to work for a boolean field:

https://www.drupal.org/project/drupal/issues/2546212#comment-13606763

so IMO looking how the configuration translation is set for that is the next step here.

Ok, I think this might work:

1) Update config/schema/smart_trim.schema.yml

and change:

    more_text:
      type: string
      label: 'More link text'

to

    more_text:
      type: label
      label: 'More link text'

2) Apply the latest patch from #2546212: Entity view/form mode formatter/widget settings have no translation UI

3) Follow instructions from #19

If someone will make the patch and add here, I can test this approach works.

@PapaGrande Woot! Indeed: https://git.drupalcode.org/project/drupal/commit/cb0e300 :) Marking "Novice" as this is a trivial patch requested in #23.

Note, in order to not hold up the D9 release of Smart Trim, IMO we could do:

1) Use the old (though not correct) approach for allowing translation of more text via Interface Translation using t(), e.g.

       $more = $this->t($this->getSetting('more_text'));

At least this would get rid of the regression.

2) Create a follow-up issue to implement the translation correctly using Configuration Translation per comments above. IMO, the new code should ideally also handle automatically copying or "moving" any "more text" translations from Interface Translation to Configuration Translation.

Did some digging on this today and it's really weird that the placeholder text becomes translatable.... I went ahead and committed the old change back (and removed the unused Use command) but really need to dig into this some more. Will mark this fixed so we can get on with the D9 compatible version.

Thanks @markie! Placeholders aren't translatable by design so it makes sense to me. I did get confused by the example I found in #6 but I haven't had time to look at it more closely.

It is weird that they show up in the translation UI though... @more_text can be translated there.. but.... why would we want to? Then again, it could be argued that @more_text would always be translated to what we want there.... /shrug

It's because usually the placeholder would have additional text around it, e.g.

$this->t('Use <em>@interval</em> where you want the formatted interval text to appear.'),

$this->t('Delete @entity_type', ['@entity_type' => $entity_type->getSingularLabel()]);

$summary[] = $this->t('Time zone: @timezone', ['@timezone' => $timezone]);

For these @interval, @entity_type, and @timezone won't be translated but you can translate the text around it and even move the placeholder relative to the text as needed (e.g. RTL).

It doesn't make sense to use the placeholder just by itself with translation which is why I'm confused about #6 and I just go back and see what's up with that one at some point. :)

In the newest dev version for 2.1.x (not working in the 2.1.0 release) you can use the hook "hook_smart_trim_link_modify" to modify this text or the link in general.

Example:

/**
 * Implements hook_smart_trim_link_modify()
 */
function MODULE_smart_trim_link_modify($entity, &$more, &$url) {
  $more = t('Read more');
}