openeuropa / oe_bootstrap_theme

Bootstrap-based theme
European Union Public License 1.2
6 stars 12 forks source link

Gallery pattern: caption title, link is rendered for playable media #327

Open kp77 opened 1 year ago

kp77 commented 1 year ago

In the BCL example, the gallery component never has a caption overlay for videos. This is probably because with the caption overlay, the video controls are not usable.

In the Gallery pattern, this is handled, but only for the item.caption property. If the item has caption_title or link property, the caption overlay is displayed, making the video controls unusable.

The issue is mitigated by the fact that we can map the video media name to the caption property instead of the caption_title, but I think the correct solution would be if the caption overlay is never displayed for playable media in the pattern.

Suggestion for https://github.com/openeuropa/oe_bootstrap_theme/blob/1.3.0/templates/patterns/gallery/pattern-gallery.html.twig#L24-L26:

    items: items|map(_item => _item|merge({
      caption: _item.is_playable ? '' : _item.caption,
      caption_title: _item.is_playable ? '' : _item.caption_title,
      link: _item.is_playable ? '' : _item.link
    })),

Or another option would be to move this logic to the bcl_gallery_items twig function.

tibi2303 commented 1 year ago

Thank you for this, we will actually fix this in this pr from BCL. https://github.com/openeuropa/bootstrap-component-library/pull/509. Where we will be able to display the captions without any issue on videos

We can actually use this issue to display the caption that it's removed after we will have the BCL pr merged and released.

tibi2303 commented 1 month ago

Do we still need this issue? Please have a look here: https://github.com/openeuropa/bootstrap-component-library/blob/development/src/compositions/bcl-gallery/gallery.html.twig#L173

We manage it in BCL and we hide them if it's an iframe or video. I don't see any other use case to hide them. Close this if you agree and if it's fixed, please.