backdrop-contrib / i18n

Collection of modules to extend Backdrop CMS multilingual capabilities
https://backdropcms.org/project/i18n
GNU General Public License v2.0
2 stars 5 forks source link

Localized terms and the page title doesn't work with the alternative "Taxonomy term" view #65

Closed laryn closed 2 years ago

laryn commented 3 years ago

I'll try to get more information noted but wanted to write this down before I lost it: it seems that a vocabulary that is set to "Localization" as the mode of translation doesn't work quite perfectly with the "Page title" block that is available in the Layout. It always displays the source language for the Page Title.

laryn commented 3 years ago

Interestingly, I was poking around and made a little custom function in a module to play with this, and the following code implemented as a block and placed on the page is enough to get the "Page title" block and the title in the browser tab to localize:

  function custommodule_termname_block_view( ) {
    if (arg(0) == 'taxonomy' && arg(1) == 'term' && is_numeric(arg(2))) {
      $tid = arg(2);
      $term = taxonomy_term_load($tid);
      $term = i18n_taxonomy_localize_terms($term);
      backdrop_set_title($term->name);
    }
  }

I'm not sure where this type of thing should actually go. I should note that I am working on a site that uses the "Taxonomy Term" alternative view rather than the "out of the box" term pages. I don't know if that makes a difference.

laryn commented 2 years ago

Just ran into this again -- thank you past self! 😂

indigoxela commented 2 years ago

What are the steps to reproduce? A localized term (or translated? both?) and use it directly as a Layout title - in the Layout UI?

Some more details could be helpful here.

indigoxela commented 2 years ago

This is what I did to reproduce (wasn't able to):

@laryn what am I missing?

laryn commented 2 years ago

@indigoxela I’ll try to get steps to reproduce today. It happened twice to me I just need to figure out what else I did in between your steps!

laryn commented 2 years ago

@indigoxela It turns out that this is important after all:

I should note that I am working on a site that uses the "Taxonomy Term" alternative view rather than the "out of the box" term pages. I don't know if that makes a difference.

The problem exists even without the Page Title block as long as the alternative view (which ships with core but is disabled by default) is used.

indigoxela commented 2 years ago

Ah, we're getting closer. The problem isn't the layout, nor the title block, it's the fact that the title gets set via a views contextual filter - and that's not accessible for i18n.

Did that ever work in D7? UPDATE: yes, that works in D7.

indigoxela commented 2 years ago

This is what I have so far.

I've no clue why/how that works in D7, but found a workaround. (My favorite complex "darlings" i18n and views give me a hard time.)

This will need thorough testing an possibly a new functional test.

laryn commented 2 years ago

Spitballing here, but the view does try to override the title:

Screen Shot 2022-02-17 at 4 43 49 PM

Can we figure out which function does that override substitution and see if the result can be localized right there?

indigoxela commented 2 years ago

the view does try to override the title

Yes, that's clear, and the substitution works fine, but in B i18n isn't able to replace that with the localized variant without extra code.

Can we figure out which function does that override substitution

Sure, that's views_handler_argument::get_title(). But that doesn't explain why things are different in B compared to D7.

Did you find the time yet, to actually test the PR?

indigoxela commented 2 years ago

Here's the problem: https://github.com/backdrop/backdrop-issues/issues/5516

There's no "label callback" for entities in Backdrop, that's why the i18n_taxonomy code never runs. Views titles set via arguments might just be the tip of the ice-berg. But at least for that we have a workaround. We'll see what pops up in the future.

indigoxela commented 2 years ago

@laryn have you already been able to test this PR?

laryn commented 2 years ago

@indigoxela Sorry for the delay, but yes, I have now tested and this PR is working.

indigoxela commented 2 years ago

@laryn also sorry for the delay - I just realized that I forgot to merge the PR.