Skip to content

Reorder adding of page layout handles #12807

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 17, 2018
Merged

Reorder adding of page layout handles #12807

merged 1 commit into from
Jan 17, 2018

Conversation

@aschrammel
Copy link

@aschrammel aschrammel commented Dec 19, 2017 •
edited by mzeis
Loading

Add type-dependent layout handles before more specific ID/SKU layout
handles.

When updating a product page layout for a specific ID with catalog_product_view_id_<product_ID>.xml some changes may be overwritten by a less specific catalog_product_view_type_<product_type>.xml.

Description

Change the order of loading the layout handles in \Magento\Catalog\Helper\Product\View and \Magento\Review\Controller\Product\ListAction to add the type specific layout handles BEFORE more specific ID/SKU specific layout handles.

In \Magento\Catalog\Controller\Category\View the updates are already loaded in the above suggested order.

Fixed Issues (if relevant)

--

Manual testing scenarios

  1. Create a configurable product
  2. Create a layout override for this product on by ID base (catalog_product_view_id_<product_ID>.xml)
  3. In this layout update try to overwrite the product.info.options.configurable block with a custom template in referenced block product.info.options.wrapper
  4. Before applying the PR the custom template is not taken into account, afterwards it is taken into account

Example override

<?xml version="1.0"?>
<page xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="urn:magento:framework:View/Layout/etc/page_configuration.xsd">
    <body>
        <referenceBlock name="product.info.options.wrapper">
            <block class="Vendor\Module\Block\Product\View\Type\Configurable"
                name="product.info.options.configurable" as="options_configurable" before="-"
                template="Vendor_Module::product/view/type/options/configurable.phtml"/>
        </referenceBlock>
    </body>
</page>

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)

🐛 Reorder adding of page layout handles
7819312 
Add type-dependent layout handels before more specific id/sku layout
handles.
@mzeis mzeis self-assigned this Dec 20, 2017
@mzeis
Copy link
Contributor

mzeis commented Dec 20, 2017

Hi @aschrammel, thank you very much for your contribution!

I will process your PR. I hope to able to do this before the holidays. If not please be a little bit patient with me. :-)

@aschrammel
Copy link
Author

aschrammel commented Dec 20, 2017

Hi @mzeis, thank you for your reply.

Of course - enjoy your holidays. 😃

@mzeis mzeis added this to the December 2017 milestone Dec 20, 2017
@mzeis
Copy link
Contributor

mzeis commented Dec 20, 2017

@aschrammel I will discuss your PR with the core team. The change makes sense in my opinion but we have to consider that 3rd party integrators might rely on the current behaviour and it might not be good to change this in a patch release.

@aschrammel
Copy link
Author

aschrammel commented Jan 4, 2018

@mzeis Thanks for the feedback. Of course there may be issues with 3rd party integrations and it's clear that a merge in a patch release may not be the best.
Nevertheless I think that this obviously undesired behaviour wasn't recognized since now because it's a rare edge case to update a layout via XML for only one product (ID or SKU).

@mzeis
Copy link
Contributor

mzeis commented Jan 6, 2018

@aschrammel I think so too and am awaiting for approval from the core team. Thanks for your patience!

@okorshenko okorshenko self-requested a review January 8, 2018 15:00
@okorshenko okorshenko modified the milestones: December 2017, January 2018 Jan 8, 2018
@magento-team magento-team merged commit 7819312 into magento:2.2-develop Jan 17, 2018
magento-team pushed a commit that referenced this pull request Jan 17, 2018
MAGETWO-85876: Reorder adding of page layout handles #12807
7103f1b
@mzeis
Copy link
Contributor

mzeis commented Jan 19, 2018

Hi @aschrammel, thank you again for your contribution! It has been merged into 2.2-develop.

@aschrammel
Copy link
Author

aschrammel commented Jan 20, 2018

Hi @mzeis, was me a pleasure ;)
Thanks for processing it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@okorshenko okorshenko okorshenko approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@mzeis mzeis

Labels

Component: Catalog Progress: accept Release Line: 2.2

Projects

None yet

Milestone

January 2018

Development

Successfully merging this pull request may close these issues.

4 participants

@aschrammel @mzeis @okorshenko @magento-team