#7065 Allow page.main.title block to decide if title should be translated #11709

adrian-martinez-interactiv4 · 2017-10-25T01:47:42Z

Related to #7065.

Layout block with name 'page.main.title', related with block \Magento\Theme\Block\Html\Title, always translates title before being returned. Unfortunately, category and product pages get their titles translated when they shouldn't (if wanted so, a different value should be defined for category / product at store view level in admin).

Description

This may be the desired behaviour for most parts of the system, but 'page.main.title' always translates the title. Because of that, that block is now capable to handle the boolean argument translate_title, injected in constructor and defined via layout xml, to decide whether it has to translate the title or not.

As it used to translate all titles before, impact has been minified making \Magento\Theme\Block\Html\Title translate all titles by default, unless it has been told to do the opposite. This is the case of category and product pages by now, but there could be other pages interested in title not being translated.

I'm aware that arguments translation and be done via translate attribute, like in Magento/Catalog/view/frontend/layout/catalog_product_compare_index.xml:

<page xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" layout="1column" xsi:noNamespaceSchemaLocation="urn:magento:framework:View/Layout/etc/page_configuration.xsd">
    <body>
        <referenceBlock name="page.main.title">
            <action method="setPageTitle">
                <argument translate="true" name="page_title" xsi:type="string">Compare Products</argument>
            </action>
        </referenceBlock>
        <referenceContainer name="content">
            <block class="Magento\Catalog\Block\Product\Compare\ListCompare" name="catalog.compare.list" template="Magento_Catalog::product/compare/list.phtml" cacheable="false"/>
        </referenceContainer>
    </body>
</page>

But this only works for static values, that get translated when layout files are parsed and layout is generated. For products and categories, this value is set afterwards in _prepareLayout method.

Fixed Issues (if relevant)

page.main.title is translating title #7065: page.main.title is translating title

Manual testing scenarios

As proposed in page.main.title is translating title #7065, I created a product with name 'Save'
I have installed translation packages for es_ES, and configured store to use that locale.
The result was the product name translated in product page view (Save => Guardar):
After applying this changes, product name does not get translated anymore:

Also checked home page, and it still gets it title translated.

Contribution checklist

Pull request has a meaningful description of its purpose
All commits are accompanied by meaningful commit messages
All new or changed code is covered with unit/integration tests (if applicable)
All automated tests passed successfully (all builds on Travis CI are green)

orlangur · 2017-10-25T09:18:54Z

app/code/Magento/Theme/Block/Html/Title.php

+     */
+    public function __construct(Template\Context $context, array $data = [])
+    {
+        $this->translateTitle = isset($data['translate_title']) ? (bool) $data['translate_title'] : true;


Having flags in code to switch its behavior is almost never a good idea.

category and product pages get their titles translated when they shouldn't

Why not simply introduce another title block implementation, RawTitle or something, and use it on mentioned pages?

General note: please always start contribution from 2.2-develop branch, a default one in this repo.

adrian-martinez-interactiv4 · 2017-10-25T11:18:06Z

@orlangur Updated with suggested changes

orlangur · 2017-10-25T11:25:14Z

app/code/Magento/Catalog/view/frontend/layout/catalog_category_view.xml

-            </arguments>
-            <block class="Magento\Catalog\Block\Category\Rss\Link" name="rss.link" template="Magento_Catalog::category/rss.phtml"/>
-        </referenceBlock>
+        <referenceBlock name="page.main.title" remove="true"/>


I believe I read somewhere that changing block name is backward-incompatible change but http://devdocs.magento.com/guides/v2.2/contributor-guide/backward-compatible-development/index.html#magento-apis is quite vague about it.

Can we preserve page.main.title name but just change block type on needed places?

I have thought the same thing, I had problems in the past removing blocks and trying to re-add them with the same name. I'm trying now to override block class definition by setting the same block name, so full block node gets overriden during merge, I keep you updated.

Probably it can be done via reference: http://devdocs.magento.com/guides/v2.0/frontend-dev-guide/layouts/xml-manage.html#set_template

I think this is for defining / changing arguments for a given block, like the template, but for a given (already instantiated) block. I've achieved changing block without changing name redefining the block, see 584cfd3

Good! But, will replacing class using referenceBlock work? It would allow to not add additional referenceContainer and generally referencing is safer that redeclaring.

I don't think this is possible without rewriting xml node block redefining it with the same name, I've been investigating this possibility, and this is what I found.

Here is the xsd definition for referenceBlock, from Magento/Framework/View/Layout/etc/elements.xsd:

<xs:complexType name="blockReferenceType" mixed="true"> <xs:choice minOccurs="0" maxOccurs="unbounded"> <xs:element name="action" type="actionType" minOccurs="0" maxOccurs="unbounded"> <xs:annotation> <xs:documentation> Argument name must be unique in scope of action. </xs:documentation> </xs:annotation> <xs:key name="blockReferenceActionArgumentName"> <xs:selector xpath="argument"></xs:selector> <xs:field xpath="@name"></xs:field> </xs:key> </xs:element> <xs:element ref="arguments" minOccurs="0" maxOccurs="1"/> <xs:element ref="block" minOccurs="0"/> <xs:element name="container" type="containerType" minOccurs="0"/> <xs:element ref="referenceBlock" minOccurs="0" /> <xs:element ref="referenceContainer" minOccurs="0"/> <xs:element ref="uiComponent" minOccurs="0" /> </xs:choice> <xs:attribute type="elementNameType" name="name" use="required"/> <xs:attribute type="xs:string" name="template" use="optional"/> <xs:attribute type="xs:boolean" name="display" default="true" use="optional"/> <xs:attribute type="xs:boolean" name="remove" use="optional"/> </xs:complexType>

To change the class, I think it should be done redefining class as a referenceBlock attribute: <referenceBlock name="page.main.title" class="Magento\Theme\Block\Html\RawTitle">, thing that is not allowed by xsd definition (it only allows name, template, display and remove attributes for referenceBlock).

The rest of the xsd declaration are arguments for the already defined block class, or actions over it, or declaring children blocks, or referencing other blocks, containers or uiComponents.

I have even tried to find any example of any layout xml file trying to perform an action like this with no success. No clue of this kind of usage even in layout directives tests under Magento/Framework/View/_files/layout_directives_test.

If you still don't like the idea of doing this task the way it is done, I could still use $this->eventManager->dispatch('layout_render_before_' . $this->request->getFullActionName()); event placed in \Magento\Framework\View\Result\Layout::renderResult, or tweak it in some way in \Magento\Catalog\Block\Category\View::_prepareLayout and \Magento\Catalog\Block\Product\View::_prepareLayout, but this way I think you may lose some control about this, control that you may have per theme editing layout xml files.

thing that is not allowed by xsd definition

Thanks, that's pretty answers my question. Current implementation with declarative approach instead of some PHP code seems best to me. Will check if there was some intentional decision to disallow class overriding.

Did class overwriting work properly by the way if you expand XSD?

Surprisingly, yes, it does, this way:

At Magento/Framework/View/Layout/etc/elements.xsd:

At Magento/Catalog/view/frontend/layout/catalog_product_view.xml:

Result:

Only removing class attribute from referenceBlock:

Result:

So yes, it works. Have you found any intentional decision for this not being allowed by xsd?

Nah, didn't ask yet. Asking now. Big thanks for checking!

… RawTitle block, to avoid category and product names being translated Unit tests for new RawTitle block

adrian-martinez-interactiv4 · 2017-10-26T15:58:36Z

@orlangur I'm facing this issue with static tests, new block is expected to be marked as @api if referenced in default layouts:

I thought I wasn't supposed to add @api annotation for any added class, what should I do?

orlangur · 2017-10-26T16:11:26Z

Yeah-yeah, I noticed it on Bamboo as well, forgot we already run all static tests on Travis :)

No need to do anything from your side, I'll ask if architect approval is required and add @api on our side if we did it right.

ihor-sviziev · 2018-08-08T20:18:28Z

Hi @orlangur ,
Are you interested in finalizing this PR? Any results after discussing with architecs?

sivaschenko · 2018-11-06T17:14:33Z

According to @orlangur this pull request can be substantially improved after the merge of #18933

ghost · 2019-02-28T20:53:22Z

@super-user-test-tool unfortunately, only members of the maintainers team are allowed to assign developers to the pull request

orlangur

Hi there @adrian-martinez-interactiv4, the time has come to rework implementation considering #18933 😎

sidolov · 2019-04-09T15:06:42Z

@adrian-martinez-interactiv4 , @orlangur hey guys, any progress on this?

orlangur · 2019-04-09T15:20:15Z

@sidolov I've reached Adrian in Slack yesterday, he is now aware of this PR and there are high chances to update it :)

sivaschenko · 2019-04-29T18:42:24Z

@adrian-martinez-interactiv4 , @orlangur any updates?

sidolov · 2019-06-25T22:05:30Z

@adrian-martinez-interactiv4 , I am closing this PR now due to inactivity.
Please reopen and update if you wish to continue.
Thank you for the collaboration!

m2-assistant · 2019-06-25T22:05:32Z

Hi @adrian-martinez-interactiv4, thank you for your contribution!
Please, complete Contribution Survey, it will take less than a minute.
Your feedback will help us to improve contribution process.

orlangur requested changes Oct 25, 2017

View reviewed changes

orlangur self-assigned this Oct 25, 2017

adrian-martinez-interactiv4 force-pushed the FR#PRODUCT-TITLE-TRANSLATED-PDP branch from 86d39c5 to a04ac2f Compare October 25, 2017 11:16

orlangur requested changes Oct 25, 2017

View reviewed changes

Override page.main.title block in category and product pages with new…

584cfd3

… RawTitle block, to avoid category and product names being translated Unit tests for new RawTitle block

adrian-martinez-interactiv4 force-pushed the FR#PRODUCT-TITLE-TRANSLATED-PDP branch from 667a892 to 584cfd3 Compare October 25, 2017 11:52

orlangur added this to the October 2017 milestone Oct 25, 2017

okorshenko modified the milestones: October 2017, November 2017 Nov 1, 2017

magento-engcom-team added the bugfix label Nov 7, 2017

okorshenko modified the milestones: November 2017, December 2017 Dec 1, 2017

okorshenko removed the 2.2.x label Dec 14, 2017

okorshenko modified the milestones: December 2017, January 2018 Jan 8, 2018

okorshenko modified the milestones: January 2018, February 2018 Feb 7, 2018

okorshenko removed this from the February 2018 milestone Mar 1, 2018

okorshenko added this to the May 2018 milestone May 18, 2018

okorshenko removed this from the May 2018 milestone May 31, 2018

sidolov removed the Admin Panel label Jul 31, 2018

ihor-sviziev added Progress: needs update and removed Progress: needs update labels Aug 8, 2018

ishakhsuvarov added this to the Release: 2.3.1 milestone Sep 18, 2018

orlangur mentioned this pull request Oct 29, 2018

Update typeReferenceBlock definition #18933

Merged

4 tasks

sivaschenko self-assigned this Nov 6, 2018

sivaschenko removed their assignment Nov 6, 2018

sivaschenko added the Progress: on hold label Nov 12, 2018

okorshenko removed this from the Release: 2.3.1 milestone Jan 28, 2019

sidolov assigned sidolov and unassigned sidolov Feb 28, 2019

super-user-test-tool self-assigned this Feb 28, 2019

ghost unassigned super-user-test-tool Feb 28, 2019

sidolov removed Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release labels Mar 13, 2019

orlangur requested changes Mar 26, 2019

View reviewed changes

ghost added Progress: needs update and removed Progress: on hold labels Mar 26, 2019

sidolov closed this Jun 25, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#7065 Allow page.main.title block to decide if title should be translated #11709

#7065 Allow page.main.title block to decide if title should be translated #11709

adrian-martinez-interactiv4 commented Oct 25, 2017 •

edited

Loading

orlangur Oct 25, 2017

adrian-martinez-interactiv4 Oct 25, 2017

adrian-martinez-interactiv4 commented Oct 25, 2017

orlangur Oct 25, 2017

adrian-martinez-interactiv4 Oct 25, 2017

orlangur Oct 25, 2017

adrian-martinez-interactiv4 Oct 25, 2017

orlangur Oct 25, 2017

adrian-martinez-interactiv4 Oct 26, 2017

orlangur Oct 26, 2017

adrian-martinez-interactiv4 Oct 26, 2017 •

edited

Loading

orlangur Oct 26, 2017

adrian-martinez-interactiv4 commented Oct 26, 2017

orlangur commented Oct 26, 2017

ihor-sviziev commented Aug 8, 2018 •

edited

Loading

sivaschenko commented Nov 6, 2018

ghost commented Feb 28, 2019

orlangur left a comment

sidolov commented Apr 9, 2019

orlangur commented Apr 9, 2019

sivaschenko commented Apr 29, 2019

sidolov commented Jun 25, 2019

m2-assistant bot commented Jun 25, 2019

#7065 Allow page.main.title block to decide if title should be translated #11709

#7065 Allow page.main.title block to decide if title should be translated #11709

Conversation

adrian-martinez-interactiv4 commented Oct 25, 2017 • edited Loading

Description

Fixed Issues (if relevant)

Manual testing scenarios

Contribution checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adrian-martinez-interactiv4 commented Oct 25, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adrian-martinez-interactiv4 Oct 26, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adrian-martinez-interactiv4 commented Oct 26, 2017

orlangur commented Oct 26, 2017

ihor-sviziev commented Aug 8, 2018 • edited Loading

sivaschenko commented Nov 6, 2018

ghost commented Feb 28, 2019

orlangur left a comment

Choose a reason for hiding this comment

sidolov commented Apr 9, 2019

orlangur commented Apr 9, 2019

sivaschenko commented Apr 29, 2019

sidolov commented Jun 25, 2019

m2-assistant bot commented Jun 25, 2019

adrian-martinez-interactiv4 commented Oct 25, 2017 •

edited

Loading

adrian-martinez-interactiv4 Oct 26, 2017 •

edited

Loading

ihor-sviziev commented Aug 8, 2018 •

edited

Loading