fix: media name and description usage #2631
Conversation
UPGRADE-5.7.md
Outdated
| @@ -10,11 +10,18 @@ This changelog references changes done in Shopware 5.7 patch versions. | |||
|
|
|||
| * Added compatibility with PHP 8.3 | |||
| * Added new polyfill `symfony/polyfill-php83` to be able to use PHP 8.3 features | |||
| * Added image name fallbacks if no image description is available to: | |||
There was a problem hiding this comment.
| * Added image name fallbacks if no image description is available to: | |
| * Added fallback to image name for `alt` and `title` attributes if image description is not available in following templates: |
and then please list the tpl files and then the block names
engine/Shopware/Bundle/StoreFrontBundle/Gateway/DBAL/Hydrator/MediaHydrator.php
Show resolved
Hide resolved
| @@ -206,7 +206,10 @@ | |||
| {if $sArticle.additionaltext} | |||
| {$lastSeenProductsConfig.currentArticle.articleName = $lastSeenProductsConfig.currentArticle.articleName|cat:' ':$sArticle.additionaltext} | |||
| {/if} | |||
| {$lastSeenProductsConfig.currentArticle.imageTitle = $sArticle.image.description} | |||
| {$lastSeenProductsConfig.currentArticle.imageTitle = $sArticle.image.name} | |||
There was a problem hiding this comment.
don't we need to escape here as well? 🤔
There was a problem hiding this comment.
$lastSeenProductsConfig is json encoded here so it should be escaped anyways, I think 🤔
lacknere
force-pushed
the
fix-media-name-and-description-usage
branch
from
df83dc3 to
40b94b7
Compare
| @@ -126,8 +126,9 @@ public function hydrateProductImage(array $data) | |||
| $translation = $this->getTranslation($data, '__image'); | |||
| $data = array_merge($data, $translation); | |||
|
|
|||
| $media->setName($data['__image_description']); | |||
| $media->setName($data['__image_img']); | |||
There was a problem hiding this comment.
hey @lacknere
I had another thought about that. This change is a break. People and code are assuming, that the "name" contains the image description, if this hydration method is used. If we now change this, their code will break. So unfortunately we cannot merge it like that
There was a problem hiding this comment.
What about using description primarily and name as a fallback there, if no description is available? Something like $media->setName($data['__image_description'] ?: $data['__image_img']);
There was a problem hiding this comment.
sorry for the late answer 🙈
I am not sure, if this is also a desired state 🤔 could also lead to unexpected behaviour, where not values where before.
what about deprecating the old behaviour and trigger the new one with a parameter? 🤔 where the default ensures the old behaviour.
to not break the public method, we would need to add the parameter commented at first, and get the value with https://www.php.net/manual/en/function.func-get-args.php 🤔
maybe you can find a way to build it like that, that you have the possibility to get the hydrated media with "correct" values, but the default way stays at it is.
|
hey @lacknere |
|
Hey @lacknere I will close this PR due to inactivity. Let me know, if you want to discuss this further 🙂 |
1. Why is this change necessary?
Media name and description are not correctly mapped.
2. What does this change do, exactly?
3. Describe each step to reproduce the issue or behaviour.
4. Please link to the relevant issues (if any).
5. Which documentation changes (if any) need to be made because of this PR?
6. Checklist