Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added updateElementControllers hook #1090

Closed
wants to merge 1 commit into from
Closed

added updateElementControllers hook #1090

wants to merge 1 commit into from

Conversation

rasstislav
Copy link

No description provided.

@michalkleiner
Copy link
Contributor

Hi @rasstislav. Thank you for opening the PR. Can you please update the PR description with the use case you're trying to solve by adding the extension hook? Do you want to remove some controllers? Why? Or add some that are not auto-detected?

@rasstislav
Copy link
Author

Hi @michalkleiner. Yes, right, my case is that I need to remove some controllers in specific cases.

My current solution:

SilverStripe\Core\Injector\Injector:
  DNADesign\Elemental\Models\ElementalArea:
    class: ElementalArea
<?php

use DNADesign\Elemental\Models\ElementalArea as VendorElementalArea;

class ElementalArea extends VendorElementalArea
{
    public function ElementControllers()
    {
        $controllers = parent::ElementControllers();

        // my specific conditions

        return $controllers;
    }

    public function forTemplate()
    {
        return $this->renderWith(VendorElementalArea::class);
    }
}

@GuySartorelli
Copy link
Member

@rasstislav Have you tried setting the controller_class configuration property for the BaseElement subclasses you want to have different controllers?

@rasstislav
Copy link
Author

Hi @GuySartorelli. You can look at my situation.

@michalkleiner
Copy link
Contributor

What is the ultimate goal though @rasstislav? To not display an element under certain situations, for a specific page type etc? I don't think updating the list of controllers is the best way to achieve that — I'd have the logic directly in the element's controller and each controller would decide for itself when and how it could/should render.

@GuySartorelli
Copy link
Member

I also don't see why you'd have an element on the page, but not display it? That seems like a really confusing experience for content authors. You could introduce a validator for your pages' elements instead and simply not allow those elements, as an alternative approach. See the RequiredBlocksValidator class in guysartorelli/silverstripe-composable-validators for an example of how to apply validation to a page based on its elemental blocks.

@michalkleiner
Copy link
Contributor

@GuySartorelli I think your suggestion is for when the blocks are being added to a page (and preventing that), whereas my suggestion and the issue here (as I understand it) is how to conditionally skip rendering of some elements e.g. when it's a virtual page or based on other similar conditions.

@GuySartorelli
Copy link
Member

GuySartorelli commented Aug 31, 2023

Yeah, it all depends on what the underlying intention is.

If it's just "in some scenarios blocks shouldn't be allowed, and I can't currently control that in the CMS so I'm removing their controllers", then validation would resolve that.

If it's "there are some scenarios where the elements are allowed to be added, but could be rendered when I don't want them to" - and then your recommendation of applying appropriate controllers with relevant logic to the elements that need that logic is perfect.

In any case, I don't see a use case for this new extension hook that isn't better served with some pre-existing solution.

@rasstislav
Copy link
Author

You can also look at this part of the code.
My goal was to make this element global if it was at the very top and render it elsewhere without having to add a new area.

But probably the best solution is to add a new area....

@kinglozzer
Copy link
Member

We’ve done something very similar before - we wanted to render ElementSocialFeeds completely separate to the rest of the elements, but only if certain conditions were met.

templates/DNADesign/Elemental/Layout/ElementHolder.ss:

<% if $Last && $Element.ClassName.Shortname = 'ElementSocialFeeds' && $Page.SocialFeedElementIfRenderedSeparately %>
    <%-- In this scenario, the element will be manually rendered outside of the ElementalArea --%>
<% else %>
    <div class="elements__element" id="{$Anchor}">
        {$Element}
    </div>
<% end_if %>

templates/App/Model/Layout/Page.ss:

{$ElementalArea}

...

<% if $SocialFeedElementIfRenderedSeparately %>
    <% with $SocialFeedElementIfRenderedSeparately %>
        {$Me}
    <% end_with %>
<% end_if %>

App\Model\Page:

    /**
     * A social feed element, but *only* if it should be rendered outside of the ElementalArea
     *
     * @see getSocialFeedElementIfRenderedSeparately()
     * @var ElementSocialFeeds|false
     */
    protected $socialFeedElementIfRenderedSeparately;

    /**
     * If the social feed element should be rendered separately to the main ElementalArea return it,
     * otherwise return false. The social feed element should rendered separately to the ElementalArea
     * only if *both* of the following criteria are met:
     *
     * - The feed element is the last element in the collection
     * - The page has a sidebar present (limiting the available width)
     *
     * @return ElementSocialFeeds|false
     */
    public function getSocialFeedElementIfRenderedSeparately()
    {
        if ($this->socialFeedElementIfRenderedSeparately !== null) {
            return $this->socialFeedElementIfRenderedSeparately;
        }

        $this->socialFeedElementIfRenderedSeparately = false;

        if (!$this->getHasSidebar()) {
            return $this->socialFeedElementIfRenderedSeparately;
        }

        /** @var ElementalArea $elementalArea */
        $elementalArea = $this->ElementalArea();
        $lastElement = $elementalArea->Elements()->Last();
        if ($lastElement instanceof ElementSocialFeeds) {
            $this->socialFeedElementIfRenderedSeparately = $lastElement;
        }

        return $this->socialFeedElementIfRenderedSeparately;
    }

@michalkleiner
Copy link
Contributor

Thanks all for the discussion and for Loz's suggestion on how to approach it differently. I'll close this now as adding the hook here isn't really needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants