OEL-739: Text with feature media video variants #109
Conversation
| @@ -72,30 +75,19 @@ function oe_bootstrap_theme_preprocess_paragraph__oe_text_feature_media(array &$ | |||
| /** @var \Drupal\paragraphs\Entity\Paragraph $paragraph */ | |||
| $paragraph = $variables['paragraph']; | |||
|
|
|||
| // Link field doesn't exist in oe_paragraphs <= 1.8.0, so ensure it exists. | |||
| if ($paragraph->hasField('field_oe_link') && !$paragraph->get('field_oe_link')->isEmpty()) { | |||
There was a problem hiding this comment.
We have a problem with the cyclomatic complexity threshold and I had to remove as many "if" as possible. Link already comes in content so for this reason I don't need to add to variables
templates/patterns/featured_media/pattern-featured-media--variant-inside_text.html.twig
Outdated
Show resolved
Hide resolved
includes/paragraphs.inc
Outdated
| * @param Drupal\media\Entity\Media $media | ||
| * Media object. | ||
| * @param array $variables | ||
| * Drupal variables. |
There was a problem hiding this comment.
In other places is 'The render array'. Change it so is consistent everywhere, please.
includes/paragraphs.inc
Outdated
| } | ||
|
|
||
| /** | ||
| * Set the image data if media is an image. |
There was a problem hiding this comment.
| * Set the image data if media is an image. | |
| * Sets the image data if the media is an image. |
includes/paragraphs.inc
Outdated
| * @param bool $is_image | ||
| * Flag to determine if is an image or not. | ||
| * @param array $variables | ||
| * Drupal variables. |
There was a problem hiding this comment.
Same as per the previous comment.
includes/paragraphs.inc
Outdated
| * @return array | ||
| * Updated variables. | ||
| */ | ||
| function set_ratio(Media $media, array &$variables, CacheableMetadata $cacheability): array { |
There was a problem hiding this comment.
Please, rename it to something like set_video_iframe_ratio to make it more explicit.
| } | ||
|
|
||
| $thumbnail = $media->get('thumbnail')->first(); | ||
| $variables['image'] = ImageValueObject::fromStyledImageItem($thumbnail, 'oe_bootstrap_theme_medium_no_crop'); |
There was a problem hiding this comment.
Couldn't it be like date in the List Item? This would mean to get the Image style not fixed as here but to get the 'configurable' image style set at the Thumbnail field in the Manage display in http://localhost:8080/build/admin/structure/media/manage/remote_video/display
There was a problem hiding this comment.
I'm not sure of this. I'm using here the oe_theme way, maybe another way is possible, we can check this together.
includes/paragraphs.inc
Outdated
| $ratio = $media->get('oe_media_iframe_ratio')->value; | ||
| $variables['ratio'] = str_replace('_', 'x', $ratio); | ||
| $cacheability->applyTo($variables); | ||
| return $variables; |
There was a problem hiding this comment.
If setting values per reference, is necessary to return this?
There was a problem hiding this comment.
Is needed because of the sign of the method:
function set_video_iframe_ratio(Media $media, array &$variables, CacheableMetadata $cacheability): array {
is waiting an array as the output of the method. So the return is needed for this reason.
| @@ -14,15 +14,19 @@ | |||
| '#alt': image.alt, | |||
| } %} | |||
| {% endif %} | |||
| {% if content.field_oe_text_long['#items'] %} | |||
There was a problem hiding this comment.
The default value is set at the paragraph.inc as FALSE, can't it be all set in same place (twig or paragraph.inc)?
There was a problem hiding this comment.
No because this settings deppends of the full text field is fulfilled or not, so by default is false but if the full text field has some content, the settings must be true.
| * Test 'text with featured media' paragraph rendering. | ||
| */ | ||
| public function testFeaturedMedia(): void { | ||
| $image_file = file_save_data(file_get_contents(drupal_get_path('theme', 'oe_bootstrap_theme') . '/tests/fixtures/example_1.jpeg'), 'public://example_1_en.jpeg'); |
There was a problem hiding this comment.
If the file is not needed as _en, use same name as source file.
Jira issue(s):