Views - Add a CE block display

Created on 17 April 2023, almost 2 years ago

This MR adds a new "Custom Elements Block" Views display class.

This is useful if you want to place a Views block leveraging lupus_decoupled_blocks.

Feature request
Status

Active

Version

1.0

Component

Code

Created by

🇬🇧United Kingdom Dan.Ashdown

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @Dan.Ashdown
  • Status changed to Needs review almost 2 years ago
  • First commit to issue fork.
  • Status changed to Needs work almost 2 years ago
  • 🇦🇹Austria fago Vienna

    Tested this, but it's not fully working yet.

    I've tried to improve the code so the block provides a proper CE response. When done so the block rendering should pick up the CE wrapped in the render-array. But it seems the views/block rendering pipeline wraps the render array somewhere, so it ends up markup instead of json still.

    "views_block__teasers_custom_elements_block_1": "<drupal-markup><div class=\"views-element-container contextual-region\"><drupal-block-view display-id=\"custom_elements_block_1\" pager=\"{&quot;total_pages&quot;:2,&quot;current&quot;:0}\"><node type=\"article\" view-mode=\"teaser\" title=\"Abico At Bene Jus Utrum\" created=\"1682611419\" slot=\"rows\"><div slot=\"body\"><p>Conventio jumentum luptatum nobis pecus proprius singularis ulciscor. Conventio genitus iustum nobis ulciscor vulputate. Dolor dolore dolus ibidem singularis. Causa gilvus ille macto os valde.</p>\n<p>Immitto iriure melior sed venio vindico. Elit importunus pneum quidem torqueo vel. Amet distineo hendrerit illum iriure jugis sino valetudo. Accumsan haero modo praemitto saepius. Distineo erat inhibeo roto verto virtus. Aliquam natu neo nulla tum velit vero. Antehabeo appellatio gilvus humo ibidem uxor vulputat

  • @fago opened merge request.
  • 🇦🇹Austria fago Vienna

    re-titling and raising prio

  • First commit to issue fork.
  • First commit to issue fork.
  • 🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

    roderik changed the visibility of the branch 3363234-subrequests-request-uri-patch-8678 to hidden.

  • 🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

    Rebased the old MR because the underlying code changed a lot with 🐛 When editing a custom elements page view php errors are produced by renderer Needs work (code was moved from the controller to CustomElementsPage plugin). Tested that output is the same as before.

    The MR is not ready, because of now 2 reasons:

    1) CustomElementsPage and CustomElementsBlock contain pretty much the same code though they extend different classes. This wants a trait.

    2) The originally reported issue still exists. I need more time to properly comment on this, than I have right now.

  • Assigned to fago
  • Status changed to Needs review 26 days ago
  • 🇳🇱Netherlands roderik Amsterdam,NL / Budapest,HU

    Mustapha did the tracing for this, and made a first change. I picked it up to check how exactly the change should be implemented (and to confuse myself on the way).

    I've tried to improve the code so the block provides a proper CE response. When done so the block rendering should pick up the CE wrapped in the render-array. But it seems the views/block rendering pipeline wraps the render array somewhere, so it ends up markup instead of json still.

    Yes, the block rendering does pick up the CE wrapped in some render array:

    The call tree is:
    1. LupusDecoupledBlockRenderer::getBlocks()
    2. Drupal\views\Plugin\Block\ViewsBlock::build()
    3. Drupal\views\Element\View::prerenderViewElement()
    4. Drupal\views\ViewExecutable::executeDisplay()

    4. returns the simple '#theme = custom_element' wrapper, and 2 + 3 add all the other stuff around it, to 'complete' the render array for a view.

    (...) the block rendering should pick up the CE wrapped in the render-array.

    I assume this means that you want to do a combination of two things:

    A. pick the CE out of above ViewsBlock::build() structure;

    B. set the CE instead of the rendered HTML, into $output[$region], in getBlocks(). (Until now, this is never done.)

    About "B": OK, no further remarks.

    re. "A": this feels to me like, at least in theory, 1) we could be throwing away part of the rendered output (produced by e.g. theme functions set by the block plugin; 2) we could be breaking compatibility by changing HTML output to something else. So I felt like I should put some thought into when exactly we are 'allowed' to do this:

    • If the CE is the only render-element-child, like this specific example? Sure. We can throw away the theme-wrapper function.
    • If there are several render-element-children, all CEs? Maybe, but...
    • ...what if there are several render-element-children, some of which are not CEs? We can probably renderRoot() the non-CE render-element-children individually, and add them as drupal-markup CEs (like is mentioned in 📌 Improve CustomElement::createFromRenderArray() to better handle entites that render to custom elements Active )?
    • ...and what if they're not all first-level children? Do we need to recurse, then?

    I've also seen 📌 Improve CustomElement::createFromRenderArray() to better handle entites that render to custom elements Active , which proposes to do something like this inside CustomElement::createFromRenderArray(). But thinking about the above points just confused me.

    So, in the end, I added only the first bullet point, directly into getBlocks(). That's what we need right now. And I can't justify the testing time for making this more generic / decide how 'backward compatible' the other cases should be... unless I'm explicitly told that we should address those cases now.

    @fago see MR. Is this the code change you want? Or am I misinterpreting some things?

  • 🇦🇹Austria fago Vienna

    93475a99 - Make LupusDecoupledBlockRenderer return a CE instead of HTML for some blocks.

    I've no clue how/why this relates to this improvement. If the block-layout support has issues, please open a dedicate issue and move the code there. We can also use layout-builder and place the block to make sure it works. That'S the main use-case anyway.

    (...) the block rendering should pick up the CE wrapped in the render-array.

    I assume this means that you want to do a combination of two things:

    NO, neither A or B shall be done here, ideally. We need to make sure the block provides a proper render-array with custom element - as documented - and it all shall work with that. If not, that's separate bugs to fix/open, but for layout-builder it definitely works.

    The output of the above render array below "views_build" looks good, if this is the block's render array, it's all good. if the whole thing is the blocks render-array we need to change it, ideally from our views-plugin, to provide only the main-part we need.

    Finally, we need to include test-coverage, i.e. a view with block + verified it renders correctly. Best we ship some layout-builder config that embeds it + test it works as it should.

  • 🇦🇹Austria fago Vienna
  • Pipeline finished with Success
    24 days ago
    Total: 397s
    #452974
  • 🇹🇳Tunisia benellefimostfa

    Whatever we do in the views Plugin, it's always wrapped by the block build in the getBlocks function,
    However when I dump the rendered custom element in the getblocks here:

     $customElement = CustomElement::createFromRenderArray($render)
                ->toRenderArray();
    dump( $customElement);
              $output[$region][$block->id()] = $this->getrenderer()
                ->renderRoot($customElement);
    

    we get this custom element Array which looks correct to me, right?

  • 🇦🇹Austria fago Vienna

    The code looks mostly good now!

    However, as discussed with roderik, the remaining todo is that we need to unwrap the resulting custom-element. It seems the views-block integration results into some render-array wrapper, what we need to get rid off, such that the API contract is on the block-level holds.

Production build 0.71.5 2024