Skip to content
Snippets Groups Projects

Convert twitter, video and image paragraphs.

8 unresolved threads

Merge request reports

Merged results pipeline passed for 92626b1c

Approval is optional
Code Quality is loading
Test summary results are being parsed

Merged by Roderik MuitRoderik Muit 10 months ago (May 24, 2024 2:32pm UTC)

Merge details

  • Changes merged into 3.x with 59fd97f7 (commits were squashed).
  • Did not delete the source branch.

Pipeline #181382 passed

Pipeline passed for 59fd97f7 on 3.x

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
79 79 */
80 80 public function build(FieldItemListInterface $items, CustomElement $custom_element, $langcode = NULL) {
81 81 if ($items instanceof EntityReferenceFieldItemListInterface) {
82 $nested_elements = [];
83 foreach ($items->referencedEntities() as $entity) {
84 $nested_elements[] = $this->ceGenerator->generate($entity, $this->configuration['mode'], $langcode);
85 }
86 if ($this->isSlot()) {
87 $custom_element->setSlotFromNestedElements($this->getName(), $nested_elements);
  • 1 <?php
    2
    3 namespace Drupal\custom_elements\Plugin\CustomElementsFieldFormatter;
    4
    5 use Drupal\Core\Field\FieldDefinitionInterface;
    6 use Drupal\Core\Field\FieldItemListInterface;
    7 use Drupal\Core\Form\FormStateInterface;
    8 use Drupal\custom_elements\CustomElement;
    9 use Drupal\custom_elements\CustomElementsFieldFormatterBase;
    10 use Symfony\Component\DependencyInjection\ContainerInterface;
    11
    12 /**
    13 * Implementation of the 'video_embed' custom element formatter plugin.
    14 *
    15 * Sets embed URL and thumbnail from the applicable video provider.
    • Author Developer

      No idea how I could do this any other way than make a new formatter.

    • @roderik is there no core formatter doing something similar? but if not, yeah, that seems reasonable

    • Author Developer
      • There are only core formatters for "Video" and "thumbnail" -- i.e. one of both properties, not both of them.
      • The "Video" also doesn't output just the video embed URL (there seems to be nothing for that in Core), but a div with an iframe that has the video embed URL as 'src' -- didn't seem ideal to me. <pg-video type="video" view-mode="full" media-video-embed-field="&lt;div class=&quot;video-embed-field-provider-youtube video-embed-field-responsive-video&quot;&gt;&lt;iframe width=&quot;854&quot; height=&quot;480&quot; frameborder=&quot;0&quot; allowfullscreen=&quot;allowfullscreen&quot; src=&quot;https://www.youtube.com/embed/IPR36uraNwc?autoplay=1&amp;amp;start=0&amp;amp;rel=0&quot;&gt;&lt;/iframe&gt;&lt;/div&gt;"></pg-video>
    • Please register or sign in to reply
  • 1 uuid: ef603016-2b8e-4ffe-a201-fb0f38b7b748
    • Author Developer

      This + below config-export files: added without much investigation.

      There are two fields on the 'image' media entity called "Image": field_image (used by us) and field_media_image (not used).

      I don't know why. field_media_image is not mentioned in the export for core.entity_view_display.media.image.default - even though it's visible on the "Edit display" screen. (Maybe only because I didn't re-save that screen?)

      Now our new exported ce_display does mention it as a dependency, so it needs to also be exported.

    • I think this is some glitch we have on our field-naming vs thunder. it does not matter at all in the test module, but in the thunder sub-module it would be good to go with the fields that thunder install by default

    • Author Developer

      You're right. This is a mistake of me adding the CE configs to this module. I should move them into the thunder submodule.

      However, I keep getting errors in the functional test at the moment. Can I wait with this until after I convert the gallery / can we temporarily commit these extra config files now?

      I've added subtask https://www.drupal.org/project/custom_elements/issues/3449313, to do immediately after the 'gallery paragraph' conversion is done.

    • Please register or sign in to reply
  • 151 151 return preg_replace("/> +</", "><", $string);
    152 152 }
    153 153
    154 /**
    155 * Tests paragraphs.
    156 */
    157 public function testParagraphs() {
    158 // We test all paragraph types from a single test method so the setup()
    159 // routine is only run once for all of them - saves time.
    160 $this->doTestTwitterParagraph();
    161 $this->doTestVideoParagraph();
    162 $this->doTestImageParagraph();
    163 $this->doTestGalleryParagraph();
  • Roderik Muit added 1 commit

    added 1 commit

    • 4168d3f3 - Fix eslint + reinstate gallery tests (not yet converted).

    Compare with previous version

  • Roderik Muit added 1 commit

    added 1 commit

    Compare with previous version

  • 1 uuid: dadd4eca-7032-44fa-9d3a-18f605a44231
    • why are we adding this config here? imho it was great to have this config controlled from API in kernel tests. that way it is easy to see what is testing. the config-import we only need for a functional/complete integration test imho.

    • Please register or sign in to reply
  • 1 <?php
    2
    3 namespace Drupal\custom_elements_thunder\Processor;
  • 1 uuid: dadd4eca-7032-44fa-9d3a-18f605a44231
    2 langcode: en-gb
    • hm, I see, some of those configs are now. works that way as well, but maybe it would be good to comment in the respective test-case that the necessary config is shipped with this module

    • Author Developer

      For completeness: yes, we will continue testing on exported config (instead of config generated in the test) specifically for these thunder-paragraph related tests only. Because we should have 'importable' config, for the benefit of users, and that is what we're (also) testing.

      Other tests will have config generated in the test.

    • Please register or sign in to reply
  • 79 79 */
    80 80 public function build(FieldItemListInterface $items, CustomElement $custom_element, $langcode = NULL) {
    81 81 if ($items instanceof EntityReferenceFieldItemListInterface) {
    82 $nested_elements = [];
    83 foreach ($items->referencedEntities() as $entity) {
    84 $nested_elements[] = $this->ceGenerator->generate($entity, $this->configuration['mode'], $langcode);
    85 }
    86 if ($this->isSlot()) {
    87 $custom_element->setSlotFromNestedElements($this->getName(), $nested_elements);
    82 $entities = $items->referencedEntities();
    83 // Ignore 'flatten' for multi-value fields.
    84 if (empty($this->configuration['settings']['flatten'])
    • I fail to follow the logic from this formatter when reading the resulting code now. I don't see where all cases are handled. If not flattening, I don't see where all the attribute logic is done.

      IT'S not logical to me that flatten totally changes how this works. Could it do its work and apply flatten in the end?

    • Roderik Muit changed this line in version 4 of the diff

      changed this line in version 4 of the diff

    • Author Developer

      OK, I've merged the 'if' and 'else' now, so that the same logic path is followed.

      Things are parsed through a normalizer now. There are some differences:

      • Separate "type" and "COMPONENTNAME-type" properties are added, which is great.
      • All properties are now prefixed with "COMPONENTNAME-", instead of added as-is, which is great/fine.
      • It turns out that the normalizer converts hyphenated "component-name" to "componentName" which is now inconsistent with how our formatters treat 'field prefixes'. I don't know the exact reason / preferred naming scheme.

      Question:

      Anywhere where our formatters use their configured name as a prefix to add multiple attributes (like FlattenedCeFieldFormatter does), should we camelCase things instead? Like "prefixProperty" instead of "prefix-property"?

      See EntityReferenceCeFieldFormatterTest.php line 231 and ParagraphFormatterTest.php line 222/twitter for examples. (I never know if these direct links work...)

    • Author Developer

      Answered in huddle. This needs a context flag to the normalize() call to not convert things to snakeCase.

    • Please register or sign in to reply
  • Roderik Muit added 9 commits

    added 9 commits

    • a17185c5 - 1 commit from branch project:3.x
    • 20f32b6d - Tests for entityreference formatter.
    • 51566e99 - phpcs
    • 2d2f8ee4 - Convert twitter, video and image paragraphs.
    • 071b592c - Fix eslint + reinstate gallery tests (not yet converted).
    • abdf27ce - Fix botched cherry-pick.
    • ff9ab2fb - 'Flattened' tests working.
    • 4dd08dc0 - Unify behavior of flatten+multivalue with Flattened formatter.
    • 928f24be - Change 'flattened' logic in entityreference formatter; adjust tests to go along with it.

    Compare with previous version

  • Author Developer

    Did a push and answered some comments. "Actual code changes" should be done but I'm still moving exported files to their newer location.

    Edited by Roderik Muit
  • Roderik Muit added 8 commits

    added 8 commits

    • c9fda6d5 - 1 commit from branch project:3.x
    • 5f14b50c - Convert twitter, video and image paragraphs.
    • 30b8996c - Fix eslint + reinstate gallery tests (not yet converted).
    • 747c65b1 - Fix botched cherry-pick.
    • 6e05cc45 - 'Flattened' tests working.
    • 042da4db - Unify behavior of flatten+multivalue with Flattened formatter.
    • 0540b1d5 - Change 'flattened' logic in entityreference formatter; adjust tests to go along with it.
    • fa84e80f - Rename kernel tests and add comment that they are dependent on imported config.

    Compare with previous version

  • Roderik Muit added 1 commit

    added 1 commit

    • 57193bec - Rename again. No code changes.

    Compare with previous version

  • Roderik Muit added 1 commit

    added 1 commit

    • 532f95b9 - Rename again. No code changes.

    Compare with previous version

  • 83 foreach ($items->referencedEntities() as $entity) {
    89 foreach ($entities as $entity) {
    84 90 $nested_elements[] = $this->ceGenerator->generate($entity, $this->configuration['mode'], $langcode);
    85 91 }
    86 if ($this->isSlot()) {
    92
    93 if ($this->isSlot() && empty($this->configuration['flatten'])) {
    87 94 $custom_element->setSlotFromNestedElements($this->getName(), $nested_elements);
    88 95 }
    89 96 else {
    90 97 $normalizer = new CustomElementNormalizer();
    91 98 $context = ['cache_metadata' => new BubbleableMetadata()];
    92 if ($this->getFieldDefinition()->getFieldStorageDefinition()->getCardinality() == 1) {
    99 if ($this->getFieldDefinition()->getFieldStorageDefinition()->getCardinality() == 1
    100 || !empty($this->configuration['flatten'])) {
    93 101 $value = $normalizer->normalize($nested_elements[0], NULL, $context);
  • Roderik Muit added 1 commit

    added 1 commit

    • 4c3216fb - Suppress camelCasing keys in the normalized array used in EntityReferenceCeFieldFormatter.

    Compare with previous version

  • Please register or sign in to reply
    Loading