openeuropa / bootstrap-component-library

Openeuropa Bootstrap Component Library
https://oelibrary.netlify.app/
MIT License
15 stars 9 forks source link

Featured media wrapper issues #533

Closed kp77 closed 1 month ago

kp77 commented 1 year ago

The Featured media BCL composition template has a wrapper div with class .bcl-featured-item. I found some issues/inconsistencies (v. 1.1.0):

1. The wrapper also has a .pb-4 class in the storybook, that is not present in the actual template. Proposed solution: The class should be removed from the storybook example to align with the template. The class is added by the wrapper_classes parameter for the example, but the wrapper_classes are not exposed in the Controls section for the composition, and this makes it look like as if the class would be part of the 'normal' output of the template.

2. The versions of the component that only display image/media without text (these are: Iframe, Video and Image) don't have the main component wrapper (this is because the condition in the template only inserts the main wrapper when the with_text parameter is true. Proposed solution: The main wrapper should be used also when there is no text. The missing wrapper also prevents using the wrapper_classes parameter when there is no text (i.e. with_text is false).

3. The component versions without text (see 2.), however, have column layout wrappers in the storybook examples, that are not in the actual template:

<div class="row">
  <div class="col-12 col-md-4">
    <figure class="rounded overflow-hidden d-inline-flex flex-wrap"><img class='img-fluid flex-end-100' alt='random image' src='https://picsum.photos/1000/400?random=1'>
      <figcaption class="bg-lighter p-3 flex-1 w-min-content">Media description text goes here.</figcaption>
    </figure>
  </div>
</div>

Proposed solution: The <div class="row"> and <div class="col-12 col-md-4"> wrappers should be removed from the example to align with actual component template.

4. The main wrapper class name (bcl-featured-item) is not consistent with the component name. Proposed solution: It would be better to use the class name bcl-featured-media instead, although this could be a BC issue.