Nice module that works as intended except that I miss the option to be able to translate the value for "Title for the front page segment in the breadcrumb" on a multi lingual site.

I added the Drupal 7 "Home breadcrumb does not get translated" issue as a related issue.

CommentFileSizeAuthor
#14 2824846-13-translate_easy_breadcrumb_config.patch1.89 KBstella
#12 2824846-12-translate_easy_breadcrumb_config.patch1.54 KBstella
#11 2824846-11-translate_easy_breadcrumb_config.patch1.17 KBstella
#4 home_breadcrumb_translation.patch911 bytesollu

Comments

ollu created an issue. See original summary.

greg boggs’s picture

greg boggs
he/him
Primary language English
Location Portland Oregon
commented

In the case of Drupal 8, different from Drupal 7, the home string is a configuration, which I believe should be translatable out of the box by configuration translation. Here is the relevant line of code. If I've made a mistake, patches or pull requests are welcome.

        $links[] = Link::createFromRoute($this->config->get(EasyBreadcrumbConstants::HOME_SEGMENT_TITLE), '<front>');
ollu’s picture

ollu commented

That was my thought as well, that the translation would end up on the User translation page.

I tried this ugly fix:
$this->t($this->config->get(EasyBreadcrumbConstants::HOME_SEGMENT_TITLE));

But it didn't show up on the User interface translation page in drupal after I had visit a page were the string were visible.

ollu’s picture

ollu commented
StatusFileSize
new home_breadcrumb_translation.patch911 bytes

Got it working, aparently I forgot to clear cache after I made the changes (see attached file).

greg boggs’s picture

greg boggs
he/him
Primary language English
Location Portland Oregon
commented

That looks reasonable. But, I was really hopeful that it wouldn't be needed in D8 config. I'm going to ping some folks to make sure I'm not going too far off base before committing this one.

greg boggs’s picture

greg boggs
he/him
Primary language English
Location Portland Oregon
commented

Specifically, this documentation leads me to believe t() is not needed for configuration:

https://www.drupal.org/docs/8/multilingual/translating-configuration

ollu’s picture

ollu commented

As far as I can understand from the page the sentence "Content Translation module in Drupal core, while the built-in user interface (such as registration forms, content submission and administration interfaces)" should cover that as you say.
I agree that it seems redundant to have the t() wrapped around the string for it to be translatable.

For now I will stick with my solution and if I stumble upon a better solution I sure will let you know.

I think that you have done a nice job with the module!

fabianx’s picture

fabianx commented
Status: Active » Needs work
Issue tags: +Needs tests

t() should indeed not be needed for translation of config entities, so this is a bug.

That means we will need a test for this.

----

Code review:

+++ b/modules/contrib/easy_breadcrumb/src/EasyBreadcrumbBuilder.php
@@ -212,8 +212,10 @@ public function build(RouteMatchInterface $route_match) {
+      $translated_home_link = $this->t($this->config->get(EasyBreadcrumbConstants::HOME_SEGMENT_TITLE));
+      ¶

nit - spacing

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented

In order for this to be translatable and deployable via configuration we need a configuration schema for the simple config provided by the module. Not too tricky given that the config only has 4 keys. The lack of a schema also hints that the module might not have any tests since if you have tests your config is automatically checked against any schema you provide :)

For more about configuration see https://www.drupal.org/docs/8/api/configuration-api/configuration-schema...

greg boggs’s picture

greg boggs
he/him
Primary language English
Location Portland Oregon
commented

Thanks Alex! I've updated the Config Translation documentation with this info! https://www.drupal.org/docs/8/multilingual/translating-configuration, and I'll get this fixed shortly. Now that the module has passed 1,000 installs, I should work on tests. I'll see what I can do.

stella’s picture

stella commented
StatusFileSize
new 2824846-11-translate_easy_breadcrumb_config.patch1.17 KB

Patch for this. It should work I think, but isn't being picked up as a translatable string for me.

stella’s picture

stella commented
StatusFileSize
new 2824846-12-translate_easy_breadcrumb_config.patch1.54 KB

New version of the patch, this time with an easy_breadcrumb.config_translation.yml file, but still not working yet.

greg boggs’s picture

greg boggs
he/him
Primary language English
Location Portland Oregon
commented

The patch looks great.

I'll do some more testing and see if I can figure out how to make it work.
~Greg

stella’s picture

stella commented
StatusFileSize
new 2824846-13-translate_easy_breadcrumb_config.patch1.89 KB

This patch works! A links.task.yml file needed to be added for the configuration page in order for the 2nd tab for translation to appear.

stella’s picture

stella commented
Status: Needs work » Needs review
greg boggs’s picture

greg boggs
he/him
Primary language English
Location Portland Oregon
commented
Status: Needs review » Fixed

Thanks for your contribution on this fix Stella and Ollu! This will be in the next release.

  • Greg Boggs committed 6af415b on 8.x-1.x authored by stella 
    Issue #2824846 by stella, ollu: Home breadcrumb does not get translated

Status: Fixed » Closed (fixed)

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