Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Topics Attribute Search Form is not getting translated on Frontend #7731

Closed
haeflimi opened this issue Apr 17, 2019 · 8 comments
Closed

Topics Attribute Search Form is not getting translated on Frontend #7731

haeflimi opened this issue Apr 17, 2019 · 8 comments
Labels
Affects:Developers Affects those who customize the CMS using code. Product Areas:Internationalization Status:Available Reviewed issue, it’s real, we’d review a pull request. Type:Enhancement A need for something new.

Comments

@haeflimi
Copy link
Contributor

haeflimi commented Apr 17, 2019

When outputing the Attribute Search Form of the Topics Attribute on some Fronted Page, it does not get translated properly.
Instead of getting translated to the language associated with the current Page, it uses the Default Dashboard language or the language specifyied by the User for the Dashboard.(if any)
The Select Attribute for Example behaves the way i would expect. - Outputing the Attribute Form Translated into the current Page Language.

This might not be an Issue noticable with Concrete5 Core functionality, since Attribute Search Forms are not used in any of the Frontend Blocks, as far as i am aware of.
Anyhow, if looking at c5 as a Framework for developers it might be valid Issue.
We are maintaining a custom Page List Block, similar to: Page List+ that allows for filtering Page Lists on the Frontend using the Attribute Search Forms. - There the issue becomes noticable.
And it seems like the before mentioned Package is having the same Issue.

How to reproduce on a Multilingual Setup without one of the packages:

  • Create a Topic Three with a few entries
  • Assign The Topic Tree Attribute with 'topic_attr' to a Test Page
  • Assign A Select Attribute with 'select_attr' to a Test Page (for comaprison)
  • Translate the TopicName Entries using the "Translate Site Interface" Feature in the Dashboard
  • Use following Code Snippet in a Single Page or Page Template View to get a comparison of the translated/ not translated output:
$app = \Concrete\Core\Support\Facade\Application::getFacadeApplication();
$em = $app->make('Doctrine\ORM\EntityManager');
$tf = $app->make('Concrete\Core\Attribute\TypeFactory');
$tk = $app->make('Concrete\Core\Attribute\Key\CollectionKey');

$topicsAk = $tk->getByHandle("topic_attr");
$topicsAt = $tf->getByHandle("topics");
$topicsValue = $c->getAttribute("topic_attr");
$selectValue = $c->getAttribute("select_attr");
$selectAt = $tf->getByHandle("select");
$selectAk = $tk->getByHandle("select_attr");`

<h2>Topic Attribute</h2>
<h3>Default Output</h3>
<?=current($topicsValue)->getTreeNodeDisplayName('text');?>
<h3>Form Output</h3>
<?=$topicsAt->render('search', $topicsAk);?>


<h2>Select Attribute</h2>
<h3>Default Output</h3>
<?=$selectValue->getValue();?>
<h3>Form Output< /h3>
<?=$selectAt->render('search', $selectAk);?>

Then the behaviour can be observed by switching between languages and comparing the Attribute Output.:
The Default Output of both Attributes is correctly translated to the Language of the Current Page.
The Select Attribute Form is correctly translated
The Topics Attribute Form is not.

@aembler aembler added Status:Available Reviewed issue, it’s real, we’d review a pull request. priority:like to have Type:Enhancement A need for something new. labels Jun 12, 2019
@aembler aembler added Affects:Developers Affects those who customize the CMS using code. Product Areas:Internationalization and removed Recategorize:Priority labels Jan 8, 2020
@1stthomas
Copy link
Contributor

This is still an issue with 8.5.7.
The problem is, that the topics are loaded with AJAX after the page has been built on the browser. And then the called method does not respect the language of the current request. If I change the locale on following line #45 (before $result = $tree->getJSON();):
https://github.com/concrete5/concrete5/blob/94a231fc96944c0a1227d10adcd152586cfa13bc/concrete/controllers/backend/tree.php#L30-L48
I get the desired translations.

In the session I can find the key/value pair 'frontendPreviousPageID' => 248 which is the page of the present AJAX request. With this entry we could gather the correct language on the said line.
Is there a better way to gather the desired language or fix this problem? Or is it ok for you @aembler ? If it's ok, I will create a pull request addressing the suggested fix.

@aembler
Copy link
Member

aembler commented Mar 31, 2022

Thanks - that makes sense. A pull request to fix this would be accepted however it would have to do the following.

  1. The PR must go against the develop branch.
  2. A companion PR could also be submitted against 8.5.x if you'd like to try and get this fixed in an upcoming 8.5.8
  3. I don't want to use the frontendPreviousPageID key/value for that, as that may not always remain. Instead can you send the page ID up with the request to this controller method and just check the page directly there?

@1stthomas
Copy link
Contributor

1stthomas commented Mar 31, 2022

Isn't this a problem in general, that we have routes without the required language?

A similar route for the tree node:
https://github.com/concrete5/concrete5/blob/94a231fc96944c0a1227d10adcd152586cfa13bc/concrete/routes/trees.php#L8

This one doesn't change the locale too:
https://github.com/concrete5/concrete5/blob/94a231fc96944c0a1227d10adcd152586cfa13bc/concrete/controllers/backend/tree/node.php#L73-L84
Would this mean, we have to fix all these controller methods and the related client requests?
Otherwise children of a page do not have to be translated.

@aembler
Copy link
Member

aembler commented Mar 31, 2022

Yes, there are other routes that ought to take a language parameter as well. There's a similar task for conversation replies that has the same problem.

@1stthomas
Copy link
Contributor

Thanks for the clarification. I'll be back on this next week.

@aembler
Copy link
Member

aembler commented Mar 31, 2022

Thanks!

@1stthomas
Copy link
Contributor

After internal clarification on this topic: I will fix it in probably 2 weeks after doing some urgent, internal tasks. I have no problem if someone else wants to fix it in the meantime 😉 .

@1stthomas
Copy link
Contributor

Additional PR for the frontend: concretecms/bedrock#232

@aembler aembler closed this as completed in f70aa83 May 5, 2022
1stthomas added a commit to 1stthomas/concrete5 that referenced this issue May 9, 2022
aembler added a commit that referenced this issue May 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Affects:Developers Affects those who customize the CMS using code. Product Areas:Internationalization Status:Available Reviewed issue, it’s real, we’d review a pull request. Type:Enhancement A need for something new.
Projects
None yet
Development

No branches or pull requests

3 participants