Magento_Catalog: gallery - fotorama__caption show/hide

[joe-vortex]

Issue

The fotorama version in 2.2.5 in includes a parameter for captions in addition to showCaption, which defaults to true and shows the caption below the gallery image, there is no way to control this in the traditional sense, besides hiding with CSS or adding a plugin using the setOptions() function.

Testing

With the blank/luma default view.xml vars:

...
    <vars module="Magento_Catalog">
        <var name="gallery">
            <var name="captions">false</var> <!-- Show caption below images -->
        </var>
    </vars>
...

The fotorama__caption element should be hidden on screen.

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)

[magento-cicd2]

All committers have signed the CLA.

[magento-engcom-team]

Hi @joe-vortex. Thank you for your contribution
Here is some useful tips how you can test your changes using Magento test environment.
Add the comment under your pull request to deploy test or vanilla Magento instance:

• @magento-engcom-team give me test instance - deploy test instance based on PR changes
• @magento-engcom-team give me {$VERSION} instance - deploy vanilla Magento instance

For more details, please, review the Magento Contributor Assistant documentation

[miguelbalparda]

Thanks for your contribution @joe-vortex! I'll process this one and report back.

[magento-engcom-team]

Hi @miguelbalparda, thank you for the review.
ENGCOM-2440 has been created to process this Pull Request

[magento-engcom-team]

@joe-vortex thank you for contributing. Please accept Community Contributors team invitation here to gain extended permissions for this repository.

[joe-vortex]

Thanks @miguelbalparda !

[hostep]

Heads up guys, this might clash with the following open PR's: #16594 or #15546.

More context: #15009

/cc @gwharton

Even though this is a different variable captions vs showCaption, it might be trying to do the same thing.

Just commenting and warning in here, because I was involved in issues regarding the above PR's today which seem to also solve the same thing as this PR is trying to solve.

[gwharton]

I dont believe we need this captions option. The correct way of not showing the caption below the images is to set gallery/caption and gallery/fullscreen/caption to false in the config. This is in accordance with the Magento developer docs at https://devdocs.magento.com/guides/v2.2/javascript-dev-guide/widgets/widget_gallery.html

The above variables are set by default but unfortunately there is a bug in the 2.2.5 gallery.phtml which means that it does not handle boolean variables correctly resulting in the relevant fotorama setup variable (showCaption) never being output, even though it is defined in the default view.xml.

I believe the correct fix for this issue is to apply PR#16594 which not only fixes the gallery.phtml for showCaption, but also for all other booleans in view.xml which arent applied correctly either.

I have PR#16594 applied to 2.2.5 and do NOT have captions set in my view.xml and the captions are not displayed under the images.

Note that the fixes in PR #16594 were previously applied back around 2.2.3/2.2.4 time, but were accidentally reverted (by me....... Arghh) which meant that 2.2.5 shipped without them.

[gwharton]

Also note the new behaviour of the code $block->getVar("name") when the variable contains either the text true or false.

Since 2.2.4

The function will return boolean true if the variable "name" is present and has the text true
The function will return boolean false if the variable "name" is present and has the text false
The function will return boolean false if the variable "name" is not present

There is no way to differentiate between the variable not existing, and it existing but is false.

Hence, the need to update ALL of the boolean options in gallery.phtml, hence #16594... :)

[joe-vortex]

@gwharton I noticed this too. The variable default is true and the getvar function would just not output anything. I have tested this by hard setting showCaption to false and it's not adding the element in the DOM. The fix you referenced - "showCaption": <?= /* @escapeNotVerified */ $block->getVar("gallery/caption") ? 'true' : 'false' would prevent the confusion with not set/false.

If your PR is going to be included in the next release then we could close my PR. This was my first contribution. Better luck next time I guess ☹️

[miguelbalparda]

No worries @joe-vortex! We really appreciate having multiple options, I'll double check this internally and let you know what the next steps are. Thanks!

[miguelbalparda]

Still discussin between this one and #16594

[miguelbalparda]

Right now we have 3 different solutions for this issue #16594 #15546 #16941. It seems we will be moving forward with #16594 but we'd like to have some more input if possible.

[hostep]

we'd like to have some more input if possible

I'd vote for #16594 because:

• It doesn't introduce a new variable, so documentation doesn't need to be updated
• It was already applied to 2.3-develop in 163d4cc, so it's best to keep 2.2 & 2.3 consistent

[miguelbalparda]

Closing in favor of #16594. Thanks for your contribution @joe-vortex, sorry for all the back and forth!!!