Skip to content
Snippets Groups Projects

Remove CE pre-generation, replace by on-demand generation.

see title

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Roderik Muit
  • 294 314 * The bundle.
    295 315 * @param string $viewMode
    296 316 * The view mode.
    317 * @param bool $anyStatus
    318 * (Optional) always return the display for the specified view mode,
    319 * regardless of status; never return NULL. The default/FALSE may return
    320 * the default displayl, and never returns a display that's not 'in use'.
  • 260 203 ->generate($paragraph, 'full');
    261 204 $markup = $this->renderCustomElement($custom_element);
    262 205 if (!$expected_markup) {
    263 206 // Note that the field is multi-valued, so the data is correctly wrapped
    264 207 // in an array.
    265 $data_string = htmlspecialchars(json_encode([
    266 ['uri' => 'http://example.com', 'title' => 'Example site', 'options' => []],
    267 ]));
    208 // $data_string = htmlspecialchars(json_encode([
    209 // ['uri' => 'http://example.com', 'title' => 'Example site', 'options' => []],
    210 // ]));
    211 // $expected_markup = <<<EOF
    212 // <pg-link type="link" view-mode="full" link="$data_string"></pg-link>
    213 // EOF;
    214 // @todo fix this or properly describe the fact that the above isn't
    215 // happening anymore.
  • 566 $ce_display->setStatus(TRUE);
    567 $ce_display->save();
    503 $this->getCustomElementGenerator()
    504 ->getCeRenderDisplay('node', 'article', 'default', TRUE)
    505 ->setForceAutoProcessing(FALSE)
    506 ->save();
    507 $this->getCustomElementGenerator()
    508 ->getCeRenderDisplay('media', 'image', 'default', TRUE)
    509 ->setForceAutoProcessing(FALSE)
    510 ->save();
    568 511
    569 512 $custom_element = $this->getCustomElementGenerator()
    570 513 ->generate($this->node, 'full');
    571 514 $markup = $this->renderCustomElement($custom_element);
    572 // @todo This should be drupal-media. Create issue and fix.
    515 // @todo doublecheck: why does drupal-media not have created/uid/...?
  • Roderik Muit
  • Wolfgang Ziegler
  • Wolfgang Ziegler
  • 104 106
    107 /**
    108 * Sets the 'original' view mode.
    109 *
    110 * Only to be used when 'constructing' a default CE display.
    111 */
    112 public function setOriginalMode(string $viewMode): self {
    113 $this->originalMode = $viewMode;
    114 return $this;
    115 }
    116
    117 /**
    118 * {@inheritdoc}
    119 */
    120 public function isInUse(): bool {
    121 return $this->getForceAutoProcessing() && $this->status();
  • 107 107 return $this->sortedProcessors;
    108 108 }
    109 109
    110 /**
    111 * Gets the defaults to apply for the given entity type, bundle and view mode.
    112 *
    113 * @return \Drupal\custom_elements\CustomElement
    114 * A new custom element with the defaults set.
    115 */
    116 public function getViewModeDefaults(string $entityType, string $bundle, string $viewMode) {
    117 // Enable components options as done in the regular entity_view_display.
    118 $entity_view_display = \Drupal::service('entity_display.repository')
  • 303 331 $entity_ce_display = $this->entityTypeManager
    304 332 ->getStorage('entity_ce_display')
    305 333 ->load($display_id);
    306 // Fallback to default view mode.
    307 if (empty($entity_ce_display) || !$entity_ce_display->status()) {
    308 $default_id = "$entityType.$bundle.default";
    334
    335 if ((!$entity_ce_display || !$entity_ce_display->status()) && !$anyStatus) {
    336 // Fall back to default view mode. If it's not in use, return NULL.
    337 $default_id = "$entityTypeId.$bundle.default";
    309 338 $entity_ce_display = $this->entityTypeManager
    310 339 ->getStorage('entity_ce_display')
    311 340 ->load($default_id);
    341 if ($entity_ce_display) {
    342 // Set original view mode using internal-ish method.
    343 $entity_ce_display->setOriginalMode($viewMode);
    • not good to do this without cloning the object. how does core do that?

    • Author Developer

      Per my above comment: it loads entities in public static function EntityViewDisplay::collectRenderDisplays(). In there, it can access $this->originalMode.

      So yes, Core also does this without cloning the object.

    • Author Developer

      Oh, no, it's more complicated than that. (IMHO it's confusing that Core is mixing 'display modes' and 'entity view modes' in the same repository...)

      1. When you need the object to do rendering, you're apparently supposed to always call public static function EntityViewDisplay::collectRenderDisplays(), which does:
      • a. sets ->originalMode for the default display (and is the only thing that can do this)
      • b. returns the default display if requesting a more specific display, but that one's disabled
      • c. creates a fresh display if it doesn't exist yet

      Its comment says "This method should only be used internally when rendering an entity. ....."

      1. When you need anything else (e.g. if you some reason need to change a field -- many tests do this but some other Core helpers too, e.g. CommentManager::addBodyField()), you're supposed to call EntityDisplay::getViewDisplay. This does c, not a or b.

      2. Then there's also code that does not need c (like the edit form) - it calls entitytypeManager->getStorage->load(ID) directly. However I think it could just use 2 instead.

      My aim with this method was to provide both above function 1 and 2, with $anyStatus being FALSE/default and TRUE respectively. I personally don't think that's more confusing than having two separate functions - though there's a good case to be made that "case 2" does not belong inside CustomElementGenerator. But I did make a mistake: I should have done: if ($entity_ce_display && !$anyStatus) { Also, the comment is clearly not clear enough about some things.

      Edited by Roderik Muit
    • Author Developer

      I could change $anyStatus (default FALSE) to $forRendering (default TRUE) to put emphasis on the use case, rather than the internal logic ("in this case it never returns NULL")...

      and at the same time add a note that objects returned with 'default $forRendering=TRUE' should ONLY be used internally for rendering... like Core does...

      Edited by Roderik Muit
    • Wolfgang Ziegler changed this line in version 6 of the diff

      changed this line in version 6 of the diff

    • Please register or sign in to reply
  • 307 if (empty($entity_ce_display) || !$entity_ce_display->status()) {
    308 $default_id = "$entityType.$bundle.default";
    334
    335 if ((!$entity_ce_display || !$entity_ce_display->status()) && !$anyStatus) {
    336 // Fall back to default view mode. If it's not in use, return NULL.
    337 $default_id = "$entityTypeId.$bundle.default";
    309 338 $entity_ce_display = $this->entityTypeManager
    310 339 ->getStorage('entity_ce_display')
    311 340 ->load($default_id);
    341 if ($entity_ce_display) {
    342 // Set original view mode using internal-ish method.
    343 $entity_ce_display->setOriginalMode($viewMode);
    344 }
    345 }
    346
    347 if (!$entity_ce_display) {
  • 347 if (!$entity_ce_display) {
    348 // Create new object. More values are added by the entity class itself.
    349 // @see \Drupal\custom_elements\Entity\EntityCeDisplay::postCreate()
    350 // @todo or init? <- once decided, remove the todo in EntityCeDisplay
    351 $entity_ce_display = $this->entityTypeManager->getStorage('entity_ce_display')->create([
    352 'id' => $display_id,
    353 'targetEntityType' => $entityTypeId,
    354 'bundle' => $bundle,
    355 'customElementName' => $this->getViewModeDefaults($entityTypeId, $bundle, $viewMode),
    356 'mode' => $viewMode,
    357 'status' => TRUE,
    358 ]);
    312 359 }
    313 360
    314 return $entity_ce_display && $entity_ce_display->status() ? $entity_ce_display : NULL;
    361 return $anyStatus || $entity_ce_display->isInUse() ? $entity_ce_display : NULL;
    • I'd not expect to check autoprocessing here. Instead, I'd expect to always get a valid display object from this helper. The auto-processing flag should be applied expclicitly in the method where it builds the custom element for the entity.

    • Author Developer

      Do you mean "applied" or "checked"?

      If "applied" then I don't know what you mean.

      If "checked", I would not have expected this. Because if auto-processing is on, this means the ce-display object is supposed to not be used at all. Not the content/formatters, not the customElementName, not anything.

      So we are returning a useless object to the caller and then expecting each caller to check it before using it?

      (In the meantime, I'll try to implement the other two points you made in this method.)

    • Wolfgang Ziegler changed this line in version 6 of the diff

      changed this line in version 6 of the diff

    • Please register or sign in to reply
  • Roderik Muit added 1 commit

    added 1 commit

    • 243157e1 - Fix casing of config key, phpcs, Vue3 test.

    Compare with previous version

  • Roderik Muit changed title from Remove CE pre-generation, replace by on-demand generation. Vue3 test still failing. to Remove CE pre-generation, replace by on-demand generation.

    changed title from Remove CE pre-generation, replace by on-demand generation. Vue3 test still failing. to Remove CE pre-generation, replace by on-demand generation.

  • Roderik Muit added 1 commit

    added 1 commit

    Compare with previous version

  • Roderik Muit mentioned in merge request !52 (merged)

    mentioned in merge request !52 (merged)

  • Roderik Muit added 1 commit

    added 1 commit

    • 2e3fea86 - Fix application of originalMode (though not sure this is the final fix)

    Compare with previous version

  • Roderik Muit added 1 commit

    added 1 commit

    • 02ca7fa0 - Fix bugs in CEDisplay::isInUse(), CEGenerator::getCeRenderDisplay().

    Compare with previous version

  • added 1 commit

    • 65e3794c - Work over changes and tests.

    Compare with previous version

  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Loading
  • Please register or sign in to reply
    Loading