Make use of display_pattern on Product Frontend View

[ecrocombe]

implementation of #3038

1: Change the attribute display value available to the template

Override Class Method Magento\Catalog\Block\Product\View\Attributes::getAdditionalData and add :
elseif (is_numeric($value) && str_contains($attribute->getData('display_pattern') ?? '','%s')){
$value = sprintf($attribute->getData('display_pattern'),$value);
}

2: Use newly created block in product view page

Reference existing Block class and change to new block class
<referenceBlock name="product.attributes" class="Smile\ElasticsuiteCatalog\Block\Product\View\Attributes"/>

NOTE: I don't believe that #2 is the correct way about using the new block class. I tried to create a preference in both di.xml and frontend/di.xml (<preference for="Magento\Catalog\Block\Product\View\Attributes" type="Smile\ElasticsuiteCatalog\Block\Product\View\Attributes"/>) but it didn't work, I'm not sure if this is my installation or not.

[rbayet]

Hello @ecrocombe,

I wouldn't go the way of a block override, considering \Magento\Catalog\Block\Product\View\Attributes is flagged as @api.

Could you re-implement this with an "after" plugin ?

Instead of looping again on the list of product attributes, though, I would probably inject an attribute collection allowing to get the list of attributes for that product that are

• visible in frontend
• having a non-void display pattern

Also, considering not everyone would want the feature enabled by default, could you add a Yes/No store configuration setting somewhere below smile_elasticsuite_catalogsearch_settings/catalogsearch ?

Regards,

[ecrocombe]

Hello @rbayet

I will look into using a plugin.

I did consider having a configuration yes/no for this, however by setting the display_pattern on an attribute to:

• '%s GB' = yes
• '%s' = no

is, in its own right, a yes/no configuration? although, not store scoped. Please correct me if I'm wrong, it's currently enabled by default on the layered navigation, and I cannot come up with a scenario where a user would want it in layered navigation but not on product page?

I'm also working on this for the product compare page, can anyone think of somewhere else on the frontend where display_pattern should be used?

[rbayet]

I did consider having a configuration yes/no for this, however by setting the display_pattern on an attribute to:

• '%s GB' = yes
• '%s' = no

is, in its own right, a yes/no configuration? although, not store scoped.

Yes, I guess but the point of making it store scoped is to allow a Magento admin to answer both the question

• do I want it ?
• do I have an i18n translation for displaying correctly in the store langue ?

That last part was to answer your question in the issue about

Also, could we scope display_pattern to store_view? Since it could include 'pieces' or 'куски'.

[...] Please correct me if I'm wrong, it's currently enabled by default on the layered navigation, and I cannot come up with a scenario where a user would want it in layered navigation but not on product page?

Because people could have already taken the matter in their own hands and have some pre-existing customization to handle unit display.
For instance, they could have decided

• to have the unit in the attribute label "Weight (kg)"
• to enable the display pattern in the layered navigation slider too

And considered that, on the product page, where the attributes are displayed, having the attribute label "Weight (kg)" is enough.

Since we are introducing something that is not essentially search and navigation related, we're walking a bit outside of our own territory, so I would prefer it to not affect any pre-existing behavior by default.

I'm also working on this for the product compare page, can anyone think of somewhere else on the frontend where display_pattern should be used?
Not out of the top of my head, no.

Regards

[ecrocombe]

Thank you for taking the time and clearing that up for me, I will go back to the drawing board and add your suggested changes.

[ecrocombe]

Ready for review/approval

[rbayet]

Ready for review/approval

Hello @ecrocombe,

Thanks for updating the PR.
Could you fix the PHPCS issues pointed out by the "PHP Code Quality" runner (https://github.com/Smile-SA/elasticsuite/actions/runs/6154992630/job/16701201842?pr=3039) ?

Regards

[ecrocombe]

I have cleaned this up as much as I can, the remaining errors say they can be auto fixed?

Mainly having trouble with the combination of these 3 rules AND the use of coalescing on multiple lines, They seem to conflict each other making a solution impossible, at least to my best abilities.

1. Line char/column maximum count.
2. the char ? must be prefixed with a space, newline found.
3. the char ? must be suffixed with a space, newline found.

[rbayet]

Hello @ecrocombe,

Let me check the remaining rules and get back to you with a solution.

Regards,

[rbayet]

Hello @ecrocombe,

Please find the suggestions above.

Regards,

[ecrocombe]

Thank you @rbayet for walking me through this PR.

If you haven't already noticed, this is my first contribution to a Git project, and I appreciate the time you have taken to educate me.

All the best,

[rbayet]

Hmmm... something strange happened locally.

[rbayet]

OK @ecrocombe, that's weird, I rebased your branch locally on top of 2.11.x and squashed the commit, but I mistakenly (force) pushed this repository 2.11.x on top of you 2.11.x branch.
Yet, I cannot re-force push the correct branch.
Anyway, I've created the new PR with your code here : #3047

[rbayet]

If you want @ecrocombe, you can

• add my fork as a new remote with git remote add rbayet [email protected]:rbayet/elasticsuite.git
• do a git fetch rbayet then checkout the branch ecrocombe-2.11.x with git checkout ecrocombe-2.11.x
• and then push this branch in your own fork with git push origin ecrocombe-2.11.x (if "origin" is the name of your remote)

You'll be able to create a new PR with the same code as #3047 but get the badge once I merge it.

Regards,