Proposal to Improve/Fix block template loading and support package overrides

[aembler]

Originally from @pietschy

Hi all,

I've just been implementing a theme and wanted to provide template overrides for another package (in this case problog). This didn't work and the only option was to put them in /blocks/. This situation is pretty painful from the perspective of both the theme developer and end user so I decided to see if it was fixable.

In looking at the code I found that themes (in packages) could override custom block templates from other packages, but it's random chance and only happens if the theme is the last package searched by BlockViewTemplate->createView().

I implemented a quick fix to always search the package of the current theme last. This fixed the issue for custom templates.

Rather than stop there I tried to get my head around the rest of the code and I think it might be possible to for themes/packages to override the default view template as well (rather than just custom templates). It think it could also be made considerably DRYer with the use of a couple of standard design patterns.

I've created a gist (https://gist.github.com/pietschy/7241034) with my thoughts along, comments and findings before I go changing too much (I've already made the theme the last package to search). My code comments/notes are prefixed with 'AP: '.

Please Note: the gist is based on 5.6 but 5.7 looks the same except for the addition of the new asset handling (which would continue work as is in the approach I'm proposing).

I'd be happy to have a crack at this. Does anyone see any gotchas, snake holes or other reasons why I should run away???

[pietschy]

Further to this, it would also be nice if:

• C5 has a way to render a package element but allowing the current theme to override it (my implementation of that method is below).
• Package developers composed larger view templates from smaller pieces using elements (contained within the package).

This approach allows the current theme to tweak aspects of a package without having the cut and paste entire views. This is something I would have liked when working with pro-blog, small changes the the layout required cutting and pasting a lot of functionality.

The combination of these two things would let theme developers create pretty seemless integration with popular packages with minimal dependency problems.

/**
     * Renders a package element but first checks if there is an override provided by the current theme.
     * @param $file String the element to render.
     * @param $pkgHandle String the package handle
     * @param null $args array any args required by the element to render.
     */
    public function packageElementWithThemeOverride($file, $pkgHandle, $args = null)
    {
        // we're copying the technique of the Loader::packageElement method here but checking
        // the current theme first for any overrides.
        $env = Environment::get();
        $theme = PageTheme::getSiteTheme();
        $overrideRecord = $env->getRecord(DIRNAME_THEMES . '/' . $theme->getThemeHandle() . '/' . DIRNAME_ELEMENTS . '/' . $file . '.php', $theme->getPackageHandle());
        if ($overrideRecord->exists()) {
            if (is_array($args)) {
                extract($args);
            }
            include($overrideRecord->file);
        } else {
            Loader::packageElement($file, $pkgHandle, $args);
        }
    }

[Remo]

Personally, I wouldn't like this, because in my opinion, the order should be.

1. Package
2. Theme
3. Site

If I place my element in the root of a site, this should win over anything there is. Sometimes I have a project where I have to customize something. I don't want to touch the package nor the theme because those are things I like to re-use. That's why I'd vote to let the root element win.

Besides, couldn't we use the override core by package feature to achieve the same without changing anything in the core?

[pietschy]

Hi @Remo,

The priority you mentioned is what the issue is about i.e. making the search priority to be package, current theme, site.

The gist has more details about the proposal.

The second comment was more about a development pattern for package developers that would allow theme developers to customise without massive cut and pastes. This only C5 impact would be to have a convenience method to render and element with the same priority as the templates.

Cheers

[Remo]

It does have more information (: I'll have a look later. As long as the site root wins, I'm all for more modular add-ons.

[pietschy]

Cool (c: Thanks for checking it out.