Skip to content

Commit e1aaf74

Browse files
bubasumabubasuma
committed
MC-40278: Logo xml configuration is always overriding dimensions configured in PA
- Fix store header logo image width/height configuration should override layout configuration
1 parent 95b1233 commit e1aaf74

File tree

7 files changed

+304
-7
lines changed

7 files changed

+304
-7
lines changed

Diff for: ‎app/code/Magento/Theme/Test/Unit/ViewModel/Block/Html/Header/LogoSizeResolverTest.php

+81
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\Theme\Test\Unit\ViewModel\Block\Html\Header;
9+
10+
use Magento\Framework\App\Config\ScopeConfigInterface;
11+
use Magento\Store\Model\ScopeInterface;
12+
use Magento\Theme\ViewModel\Block\Html\Header\LogoSizeResolver;
13+
use PHPUnit\Framework\MockObject\MockObject;
14+
use PHPUnit\Framework\TestCase;
15+
16+
/**
17+
* Test logo size resolver view model
18+
*/
19+
class LogoSizeResolverTest extends TestCase
20+
{
21+
/**
22+
* @var ScopeConfigInterface|MockObject
23+
*/
24+
private $scopeConfig;
25+
26+
/**
27+
* @var LogoSizeResolver
28+
*/
29+
private $model;
30+
31+
/**
32+
* @inheritdoc
33+
*/
34+
protected function setUp(): void
35+
{
36+
parent::setUp();
37+
$this->scopeConfig = $this->createMock(ScopeConfigInterface::class);
38+
$this->model = new LogoSizeResolver($this->scopeConfig);
39+
}
40+
41+
/**
42+
* @param string|null $configValue
43+
* @param int|null $expectedValue
44+
* @dataProvider configValueDataProvider
45+
*/
46+
public function testGetWidth(?string $configValue, ?int $expectedValue): void
47+
{
48+
$storeId = 1;
49+
$this->scopeConfig->method('getValue')
50+
->with('design/header/logo_width', ScopeInterface::SCOPE_STORE, $storeId)
51+
->willReturn($configValue);
52+
$this->assertEquals($expectedValue, $this->model->getWidth($storeId));
53+
}
54+
55+
/**
56+
* @param string|null $configValue
57+
* @param int|null $expectedValue
58+
* @dataProvider configValueDataProvider
59+
*/
60+
public function testGetHeight(?string $configValue, ?int $expectedValue): void
61+
{
62+
$storeId = 1;
63+
$this->scopeConfig->method('getValue')
64+
->with('design/header/logo_height', ScopeInterface::SCOPE_STORE, $storeId)
65+
->willReturn($configValue);
66+
$this->assertEquals($expectedValue, $this->model->getHeight($storeId));
67+
}
68+
69+
/**
70+
* @return array
71+
*/
72+
public function configValueDataProvider(): array
73+
{
74+
return [
75+
[null, null],
76+
['', null],
77+
['0', 0],
78+
['180', 180],
79+
];
80+
}
81+
}

Diff for: ‎app/code/Magento/Theme/ViewModel/Block/Html/Header/LogoSizeResolver.php

+75
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\Theme\ViewModel\Block\Html\Header;
9+
10+
use Magento\Framework\App\Config\ScopeConfigInterface;
11+
use Magento\Framework\View\Element\Block\ArgumentInterface;
12+
use Magento\Store\Model\ScopeInterface;
13+
14+
/**
15+
* Logo size resolver view model
16+
*/
17+
class LogoSizeResolver implements LogoSizeResolverInterface, ArgumentInterface
18+
{
19+
/**
20+
* Logo width config path
21+
*/
22+
private const XML_PATH_DESIGN_HEADER_LOGO_WIDTH = 'design/header/logo_width';
23+
24+
/**
25+
* Logo height config path
26+
*/
27+
private const XML_PATH_DESIGN_HEADER_LOGO_HEIGHT = 'design/header/logo_height';
28+
29+
/**
30+
* @var ScopeConfigInterface
31+
*/
32+
private $scopeConfig;
33+
34+
/**
35+
* @param ScopeConfigInterface $scopeConfig
36+
*/
37+
public function __construct(
38+
ScopeConfigInterface $scopeConfig
39+
) {
40+
$this->scopeConfig = $scopeConfig;
41+
}
42+
43+
/**
44+
* @inheritdoc
45+
*/
46+
public function getWidth(?int $storeId = null): ?int
47+
{
48+
return $this->getConfig(self::XML_PATH_DESIGN_HEADER_LOGO_WIDTH, $storeId);
49+
}
50+
51+
/**
52+
* @inheritdoc
53+
*/
54+
public function getHeight(?int $storeId = null): ?int
55+
{
56+
return $this->getConfig(self::XML_PATH_DESIGN_HEADER_LOGO_HEIGHT, $storeId);
57+
}
58+
59+
/**
60+
* Get config value
61+
*
62+
* @param string $path
63+
* @param int|null $storeId
64+
* @return int|null
65+
*/
66+
private function getConfig(string $path, ?int $storeId = null): ?int
67+
{
68+
$value = $this->scopeConfig->getValue(
69+
$path,
70+
ScopeInterface::SCOPE_STORE,
71+
$storeId
72+
);
73+
return $value === null ? null : (int) $value;
74+
}
75+
}

Diff for: ‎app/code/Magento/Theme/ViewModel/Block/Html/Header/LogoSizeResolverInterface.php

+30
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\Theme\ViewModel\Block\Html\Header;
9+
10+
/**
11+
* Interface for resolving logo size
12+
*/
13+
interface LogoSizeResolverInterface
14+
{
15+
/**
16+
* Return configured logo width
17+
*
18+
* @param int|null $storeId
19+
* @return null|int
20+
*/
21+
public function getWidth(?int $storeId = null): ?int;
22+
23+
/**
24+
* Return configured logo height
25+
*
26+
* @param int|null $storeId
27+
* @return null|int
28+
*/
29+
public function getHeight(?int $storeId = null): ?int;
30+
}

Diff for: ‎app/code/Magento/Theme/view/frontend/layout/default.xml

+1
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@
5454
<block class="Magento\Theme\Block\Html\Header\Logo" name="logo">
5555
<arguments>
5656
<argument name="logoPathResolver" xsi:type="object">Magento\Theme\ViewModel\Block\Html\Header\LogoPathResolver</argument>
57+
<argument name="logo_size_resolver" xsi:type="object">Magento\Theme\ViewModel\Block\Html\Header\LogoSizeResolver</argument>
5758
</arguments>
5859
</block>
5960
</container>

Diff for: ‎app/code/Magento/Theme/view/frontend/templates/html/header/logo.phtml

+13-3
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,17 @@
77
/**
88
* @var \Magento\Theme\Block\Html\Header\Logo $block
99
*/
10-
$storeName = $block->getThemeName() ? $block->getThemeName() : $block->getLogoAlt()
10+
$storeName = $block->getThemeName() ? $block->getThemeName() : $block->getLogoAlt();
11+
/**
12+
* @var \Magento\Theme\ViewModel\Block\Html\Header\LogoSizeResolverInterface|null $logoSizeResolver
13+
*/
14+
$logoSizeResolver = $block->getLogoSizeResolver();
15+
$logoWidth = $logoSizeResolver !== null && $logoSizeResolver->getWidth()
16+
? $logoSizeResolver->getWidth()
17+
: $block->getLogoWidth();
18+
$logoHeight = $logoSizeResolver !== null && $logoSizeResolver->getHeight()
19+
? $logoSizeResolver->getHeight()
20+
: $block->getLogoHeight();
1121
?>
1222
<span data-action="toggle-nav" class="action nav-toggle"><span><?= $block->escapeHtml(__('Toggle Nav')) ?></span></span>
1323
<a
@@ -18,7 +28,7 @@ $storeName = $block->getThemeName() ? $block->getThemeName() : $block->getLogoAl
1828
<img src="<?= $block->escapeUrl($block->getLogoSrc()) ?>"
1929
title="<?= $block->escapeHtmlAttr($block->getLogoAlt()) ?>"
2030
alt="<?= $block->escapeHtmlAttr($block->getLogoAlt()) ?>"
21-
<?= $block->getLogoWidth() ? 'width="' . $block->escapeHtmlAttr($block->getLogoWidth()) . '"' : '' ?>
22-
<?= $block->getLogoHeight() ? 'height="' . $block->escapeHtmlAttr($block->getLogoHeight()) . '"' : '' ?>
31+
<?= $logoWidth ? 'width="' . $block->escapeHtmlAttr($logoWidth) . '"' : '' ?>
32+
<?= $logoHeight ? 'height="' . $block->escapeHtmlAttr($logoHeight) . '"' : '' ?>
2333
/>
2434
</a>

Diff for: ‎dev/tests/Magento/Theme/Block/Html/Header/LogoTest.php

+59
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
<?php
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
declare(strict_types=1);
7+
8+
namespace Magento\Theme\Block\Html\Header;
9+
10+
use Magento\Framework\View\LayoutInterface;
11+
use Magento\TestFramework\Helper\Bootstrap;
12+
use Magento\TestFramework\Helper\Xpath;
13+
use Magento\Theme\ViewModel\Block\Html\Header\LogoSizeResolver;
14+
use PHPUnit\Framework\TestCase;
15+
16+
/**
17+
* Test logo page header block
18+
*/
19+
class LogoTest extends TestCase
20+
{
21+
/**
22+
* @var Logo
23+
*/
24+
private $block;
25+
26+
/**
27+
* @inheritdoc
28+
*/
29+
protected function setUp(): void
30+
{
31+
parent::setUp();
32+
$objectManager = Bootstrap::getObjectManager();
33+
$logoSizeResolver = $objectManager->get(LogoSizeResolver::class);
34+
$this->block = $objectManager->create(LayoutInterface::class)
35+
->createBlock(
36+
Logo::class,
37+
'logo',
38+
[
39+
'data' => [
40+
'logo_size_resolver' => $logoSizeResolver
41+
]
42+
]
43+
);
44+
}
45+
46+
/**
47+
* @magentoAppArea frontend
48+
* @magentoConfigFixture current_store design/header/logo_width 260
49+
* @magentoConfigFixture current_store design/header/logo_height 240
50+
*/
51+
public function testStoreLogoSize()
52+
{
53+
$xpath = '//a[@class="logo"]/img';
54+
$elements = Xpath::getElementsForXpath($xpath, $this->block->toHtml());
55+
$this->assertGreaterThan(0, $elements->count(), 'Cannot find element \'' . $xpath . '"\' in the HTML');
56+
$this->assertEquals(260, $elements->item(0)->getAttribute('width'));
57+
$this->assertEquals(240, $elements->item(0)->getAttribute('height'));
58+
}
59+
}

Diff for: ‎dev/tests/integration/framework/Magento/TestFramework/Helper/Xpath.php

+45-4
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,13 @@
55
*/
66
namespace Magento\TestFramework\Helper;
77

8+
use DOMDocument;
9+
use DOMNodeList;
10+
use DOMXPath;
11+
12+
/**
13+
* Xpath query helper
14+
*/
815
class Xpath
916
{
1017
/**
@@ -16,12 +23,46 @@ class Xpath
1623
*/
1724
public static function getElementsCountForXpath($xpath, $html)
1825
{
19-
$domDocument = new \DOMDocument('1.0', 'UTF-8');
26+
$nodes = self::getElementsForXpath((string) $xpath, (string) $html);
27+
return $nodes->length;
28+
}
29+
30+
/**
31+
* Get elements for XPath
32+
*
33+
* @param string $xpath
34+
* @param string $html
35+
* @return DOMNodeList
36+
*/
37+
public static function getElementsForXpath(string $xpath, string $html): DOMNodeList
38+
{
39+
$domXpath = self::getDOMXpath($html);
40+
return $domXpath->query($xpath);
41+
}
42+
43+
/**
44+
* Get dom document instance
45+
*
46+
* @param string $html
47+
* @return DOMDocument
48+
*/
49+
public static function getDOMDocument(string $html): DOMDocument
50+
{
51+
$domDocument = new DOMDocument('1.0', 'UTF-8');
2052
libxml_use_internal_errors(true);
2153
$domDocument->loadHTML($html);
2254
libxml_use_internal_errors(false);
23-
$domXpath = new \DOMXPath($domDocument);
24-
$nodes = $domXpath->query($xpath);
25-
return $nodes->length;
55+
return $domDocument;
56+
}
57+
58+
/**
59+
* Get dom xpath instance
60+
*
61+
* @param string $html
62+
* @return DOMXPath
63+
*/
64+
public static function getDOMXpath(string $html): DOMXPath
65+
{
66+
return new DOMXPath(self::getDOMDocument($html));
2667
}
2768
}

0 commit comments

Comments
 (0)