Skip to content

Fix load order of view.xml when loading Swatch config #13506

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

Merged

Conversation

pmclain
Copy link
Contributor

@pmclain pmclain commented Feb 5, 2018

Description

Fix load/merge order of view.xml conifg in \Magento\Swatches\Helper\Media to match \Magento\Catalog\Helper\Image.

Fixed Issues (if relevant)

  1. Image Swatch size change not working #12647

Manual testing scenarios

  1. Create new theme named to preceed 'Magento' alphabetically. ie 'Acme\default'
  2. Add view.xml to theme's etc directory altering the swatch images sizes. Sample
<?xml version="1.0"?>
<view xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:noNamespaceSchemaLocation="urn:magento:framework:Config/etc/view.xsd">
    <media>
        <images module="Magento_Catalog">
            <image id="swatch_image" type="swatch_image">
                <width>48</width>
                <height>48</height>
            </image>
            <image id="swatch_thumb" type="swatch_thumb">
                <width>130</width>
                <height>130</height>
            </image>
            <image id="swatch_image_base" type="swatch_image">
                <width>48</width>
                <height>48</height>
            </image>
            <image id="swatch_thumb_base" type="swatch_thumb">
                <width>130</width>
                <height>130</height>
            </image>
        </images>
    </media>
</view>
  1. Enable theme in admin panel.
  2. Flush cache: bin/magento cache:flush
  3. Deploy static content: bin/magento setup:static-content:deploy
  4. Navigate to product list page containing visual swatch filters and validate swatch image dimensions match those specified in view.xml

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)

@pmclain pmclain force-pushed the feature/swatch-config branch from a0a3228 to dfd734e Compare February 9, 2018 20:47
@dmanners dmanners self-assigned this Feb 14, 2018
@dmanners dmanners added this to the February 2018 milestone Feb 14, 2018
@dmanners
Copy link
Contributor

Hi @pmclain thank you for this pull request. I will start to process it and let you know if I need anything else from you.

@dmanners
Copy link
Contributor

HI @pmclain I do have a question. How does this code change effect when you have a dependent theme? For example if you have a theme loaded that it's self does not have a swatch_image settings but has a parent theme that does have this setting does it still get loaded?

@pmclain
Copy link
Contributor Author

pmclain commented Feb 14, 2018

@dmanners Yes. When the active theme's view.xml does not contain a node the parent is used.

@josefbehr
Copy link
Contributor

@pmclain Thanks for this contribution! Would you be able and willing to provide a backport of this PR to the 2.2 branch?

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

Successfully merging this pull request may close these issues.

4 participants