Fix swatch-renderer.js product id and isProductViewExist

[mimarcel]

Summary

This pull requests allows you to add a list of products, with swatches, on Product Page, without conflicting with the global product's own swatches.

• Fix getting product id
• Fix checking if swatch is used in product view or list view

Pre-requisites

• git clone M2 repo from develop branch; I used the latest at the time: commit 24cd0c3
• install M2 as usual
• Update color attribute
  • Set 'used_in_product_listing' to 1 for color attribute (in table catalog_eav_attribute)
  • Set color attribute as visual swatch and add 2-3 visual random test swatches
• Create Products
  • Add 3 configurable products A, B and C, all three configurable by color attribute, with at least one simple product child
  • Add products A and B as related products for C
  • Attach an image to all/most simple products and configurable products created
• In order to render swatches in products list, we will create a new module in app/code directory with the following structure:

Custom
└── AddSwatches
    ├── Plugin
    │   └── Swatches.php
    ├── etc
    │   ├── di.xml
    │   └── module.xml
    └── registration.php

• Add Swatches.php

<?php
/**
 * Copyright © 2009-2017 Custom Group. All rights reserved.
 * See LICENSE.txt for license details.
 */

namespace Custom\AddSwatches\Plugin;

use Magento\Catalog\Block\Product\AbstractProduct;

/**
 * Swatches plugin to add color swatches to product list.
 *
 * @package Custom\AddSwatches\Plugin
 */
class Swatches
{
    /**
     * Around interceptor method for \Magento\Catalog\Block\Product\AbstractProduct::getDetailsRenderer.
     * Adds the swatches renderer to the product list for configurable products.
     *
     * @param AbstractProduct $subject
     * @param \Closure $proceed
     * @param null|string $type
     * @return bool|\Magento\Framework\View\Element\AbstractBlock
     */
    public function aroundGetDetailsRenderer(AbstractProduct $subject, \Closure $proceed, $type = null)
    {
        if ($type === 'configurable') {
            $renderer = $subject->getLayout()
                ->createBlock('Magento\Swatches\Block\Product\Renderer\Listing\Configurable')
                ->setTemplate('Magento_Swatches::product/listing/renderer.phtml');
        } else {
            $renderer = $proceed();
        }
        return $renderer;
    }
}

• Add di.xml

<?xml version="1.0"?>
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:ObjectManager/etc/config.xsd">
    <type name="\Magento\Catalog\Block\Product\AbstractProduct">
        <plugin name="addSwatches" type="Custom\AddSwatches\Plugin\Swatches"/>
    </type>
</config>

• Add module.xml

<?xml version="1.0"?>
<config xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:Module/etc/module.xsd">
    <module name="Custom_AddSwatches" setup_version="1.0.0"/>
</config>

• Add registration.php

<?php
\Magento\Framework\Component\ComponentRegistrar::register(
    \Magento\Framework\Component\ComponentRegistrar::MODULE,
    'Custom_AddSwatches',
    __DIR__
);

• Go to app/code/Magento/Catalog/view/frontend/templates/product/list/items.phtml
  and change this code

<div class="product details product-item-details">

to this code

<div class="product details product-item-details">
    <?php echo $block->getProductDetailsHtml($_item); ?>

Ignore that we're changing the core here.

• Run php bin/magento setup:upgrade

Steps to Replicate

• Go to product C in frontend
  
  • In the above image, swatches for products A, B and C are highlighted
• Click on one of the swatches of related product B

Expected

Image for related product B is updated

Actual

Images for product C are updated

[zetlen]

This is a great improvement, @mimarcel, and but it's unfortunate that you still have to work so directly with CSS class selectors to derive the relevant product ID. I think it would be a lot better if there were data attributes in the HTML so that you could always know which product was attached to a given DOM element. Using the schema.org microdata approach, for instance, you could add itemscope, itemtype, and a data attribute this in /templates/product/list/items.phtml, on line 195:

<div itemscope
    itemtype="http://schema.org/Product"
    class="product details product-item-details">
    <meta itemprop="productID" content="<?php echo $_item->getId() ?>">

Then, you could have a universal JavaScript function, perhaps in a separate module, that would always return the product ID for the current context.

define(['mage/itemscope'], function(itemscope) {
    // in some function:
    var scope = itemscope(this.element);
    if (scope && scope.itemType === itemscope.PRODUCT && scope.productID) {
        // scope contains data for the product being displayed with `this.element`.
    }
});

Might be a useful, general-purpose thing to have.

[zetlen]

Apart from the overall structural change I'm asking about in my other comment, and the small bit of jQuery usage I recommend below, this looks good.

[zetlen]

This may throw an exception if there is no such element, since document.getElementsByName() can return null. Since jQuery is already present, you should perhaps replace this with var product = $('[product]')[0];.

[mimarcel]

Hello @zetlen ,

Thank you for looking into this PR with so much care.

• I did refactor the code to use jQuery instead of plain Javascript for an easier read: e9c42d2.
• However, it's worth mentioning that document.getElementsByName('product')[0] doesn't return an exception because document.getElementsByName('product') returns empty array in case no elements are found and document.getElementsByName('product')[0] will return undefined.
• The previous code document.getElementsByName('product')[0].value was indeed suspicious to throw an exception - issue which this PR fixed.

[zetlen]

I added a general comment about how sad it is that it has to be done this way. This is a common need and should be separated into a general utility function/module.

[mimarcel]

Hello @zetlen ,

This sounds like a very good idea, but I feel it's out of the scope of this PR. There are other few things I noticed that need refactoring in swatch-renderer.js, but I tried to keep the code as similar as possible with the old code and just fix the issues, so the code review will be as easy as possible. I would rather leave the refactoring on Magento team's side.

[mimarcel]

@zetlen and @vrann, I updated the PR.

[vrann]

@mimarcel can you please merge with the latest develop branch

[mimarcel]

@vrann , after merging develop into current branch, I realised that this commit f349c0e is already there.

Commit e9c42d2 still remains as part of this Pull Request (from code review from @zetlen).

[magento-team]

@mimarcel Thank you for the contribution