Remove CE pre-generation, replace by on-demand generation.
see title
Merge request reports
Activity
- Resolved by Wolfgang Ziegler
- Resolved by Wolfgang Ziegler
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'. - deleted - obsoleted by a longer comment in another thread.
Edited by Roderik MuitActually, 1 is not really fine. Because we need a helper function to return a fresh display if none exists yet.
So we need to either keep this "very weird and confusing" option, or make a separate helper (which is what Core basically also does, in a different class, see other comment).
changed this line in version 4 of the diff
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. I need to sit down and quietly think about what was happening here / what was the case before, which I'll do later.
(Your note is correct, but... the current output is 'better' in most cases? Maybe it's is_slot TRUE vs FALSE? I'm losing the thread with all these test changes, but I'll find the thread back soon.)
So: ugly todo for now.
changed this line in version 6 of the diff
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/...? changed this line in version 7 of the diff
- Resolved by Roderik Muit
- Resolved by Roderik Muit
- Resolved by Roderik Muit
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(); I'll think about whether to completely remove this function when I understand !56 (comment 309808) better. I'm starting to get the feeling that I still don't know the semantics / your intended use of forceAutoProcessing...
changed this line in version 5 of the diff
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') changed this line in version 2 of the diff
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); 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...)
- 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. ....."
-
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 callEntityDisplay::getViewDisplay
. This does c, not a or b. -
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- When you need the object to do rendering, you're apparently supposed to always call
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 Muitchanged this line in version 6 of the diff
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) { changed this line in version 2 of the diff
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; 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.)
changed this line in version 6 of the diff
mentioned in merge request !52 (merged)
added 1 commit
- 2e3fea86 - Fix application of originalMode (though not sure this is the final fix)
added 1 commit
- 02ca7fa0 - Fix bugs in CEDisplay::isInUse(), CEGenerator::getCeRenderDisplay().