terminal42 / contao-changelanguage

ChangeLanguage is an extension for the Contao CMS which allows visitors to switch between different languages of your website.
44 stars 35 forks source link

Use page title instead page name in navigation menu #179

Closed havutcuoglu closed 4 years ago

havutcuoglu commented 4 years ago

The a-tag title has the page name as title.

Actually I have as page name: Root EN and as page title: My Company Inc.

The navigation menu generate the following source code: <a href="mydomain" title="Root EN" class="lang-en nofallback" hreflang="en" lang="en" itemprop="url"><span itemprop="name">EN</span></a>

It is better to have the page title as title-tag if it's set.

Contao 4.9.3 Changelanguage 3.1.13

asaage commented 4 years ago

I think this is the nav_default template of the core and you can easily customize it if needed.

havutcuoglu commented 4 years ago

I am not sure because the regular menu has the page tittle as title attribute, if there is no page title defined, then the nav template use the page name.

But the language switcher use only the page name also if you set the page title.

Or is there an another logic for root pages from Contao Core?

fritzmg commented 4 years ago

You are right, the core uses the following:

https://github.com/contao/contao/blob/e7e0054dfc669f13e176217a1ed57282e2e7d987/core-bundle/src/Resources/contao/modules/Module.php#L414-L423

The Changelanguage Module however does not fill title at all. Instead it fills pageTitle with the title of the page, which is not expected:

https://github.com/terminal42/contao-changelanguage/blob/764854f733a3373ecf9be2d54518311bf2581f5a/library/Terminal42/ChangeLanguage/FrontendModule/ChangeLanguageModule.php#L135-L145

https://github.com/terminal42/contao-changelanguage/blob/764854f733a3373ecf9be2d54518311bf2581f5a/library/Terminal42/ChangeLanguage/Navigation/NavigationItem.php#L233-L236

asaage commented 4 years ago

OK it uses title instead of pageTitle but i could not reproduce the use of Root-pageTitle Did you forget to connect the translated pages? image

My Titles are "Home"&"Startseite" / "Kontakt"&"Contact" etc...

havutcuoglu commented 4 years ago

And that mean, the root page use only the page name as title?

asaage commented 4 years ago

no - it's the same for regular pages.

havutcuoglu commented 4 years ago

OK it uses pageTitle instead of title but i could not reproduce the use of Root-pageTitle Did you forget to connect the translated pages? image

My Titles are "Home"&"Startseite" / "Kontakt"&"Contact" etc...

I didn't use any connection to any translated page. That mean the user will redirected to root page in the language he selected.

I will change now the nav_default for my problem but it is not the right way.

Check my Screenshot. You will see, the page title and page name is different. But the nav_template use the page name.

change_language_nav

asaage commented 4 years ago

sorry i mixed that up title / pageTitle https://github.com/terminal42/contao-changelanguage/blob/764854f733a3373ecf9be2d54518311bf2581f5a/library/Terminal42/ChangeLanguage/Navigation/NavigationItem.php#L235 should be switched i guess.

fritzmg commented 4 years ago

should be switched i guess.

No, see my comments. The core navigation module provides both title and pageTitle. The changelanguage module only provides pageTitle, but fills it with the title. The NavigationItem class needs to be extended with

public function getPageTitle() 
{ 
    return $this->targetPage ? $this->targetPage->pageTitle : $this->rootPage->pageTitle;
}

and generateTemplateArray needs to be changed as follows:

            'isActive' => $item->isCurrentPage(),
            'class' => 'lang-'.$item->getNormalizedLanguage().($item->isDirectFallback() ? '' : ' nofallback').($item->isCurrentPage() ? ' active' : ''),
            'link' => $item->getLabel(),
            'subitems' => '',
            'href' => specialchars($item->getHref($urlParameterBag)),
+           'title' => strip_tags($item->getTitle()),
-           'pageTitle' => strip_tags($item->getTitle()),
+           'pageTitle' => strip_tags($item->getPageTitle()),
            'accesskey' => '',
            'tabindex' => '',
            'nofollow' => false,
            'target' => ($item->isNewWindow() ? ' target="_blank"' : '').' hreflang="'.$item->getLanguageTag().'" lang="'.$item->getLanguageTag().'"',
            'item' => $item,
fritzmg commented 4 years ago

@havutcuoglu you could test these changes and make a PR accordingly.

havutcuoglu commented 4 years ago

The method 'pageTitle' => strip_tags($item->getPageTitle()), doesn't work.

Here is the Error notice: Attempted to call an undefined method named "getPageTitle" of class "Terminal42\ChangeLanguage\Navigation\NavigationItem". Did you mean to call "getTitle"?

UPDATE: Sorry I forget the function getPageTitle() above :)

aschempp commented 4 years ago

closed in favor of #180