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

CMS 5 upgrade pain: $ThemeDir removed but no suitable replacement for variable filenames. #10747

Closed
GuySartorelli opened this issue Apr 3, 2023 · 3 comments

Comments

@GuySartorelli
Copy link
Member

GuySartorelli commented Apr 3, 2023

ViewableData::ThemeDir() has been deprecated and is removed in CMS 5.

In CMS 4, this method is available in templates as $ThemeDir. It is commonly used as a way to get resources (e.g. icons) where the filename is in a variable rather than hard-coded.
e.g. <img src="{$ThemeDir}/dist/images/icons/{$Icon}.svg"> where $Icon is some db field on a DataObject, allowing content authors to pick from a list of icons to use.

This isn't doable in CMS 5. The recommended suggestion is to use $ResourcePath or $ResourceURL, but these both require the full file name. This means in CMS 5 developers either have to:

  1. hardcode the file name in the template (resulting in a ton of "if the value is this, use this; if the value is that, use that" which means both lots of duplicated code and the potential to forget to update the template when adding a new value to the list), or
  2. use <img src="_resources/themes/my-theme/dist/images/icons/{$Icon}.svg"> which hardcodes the _resources dir name which is exactly what $ResourcePath and $ResourceURL are meant to avoid.

Recommendations

  1. undeprecate and reimplement ThemeDir(), OR
  2. allow getting folders using ResourcePath and/or ResourceURL, not just files.
    • It might make sense to only allow this for ResourcePath?
    • This would mean developers would have something like <img src="{$ResourcePath('dist/images/icons/')}{$Icon}.svg">
    • We'd probably want the path to always have a trailing slash
      • note that at least for ResourcePath this is for filesystem paths not URLs so the trailing slash config variable doesn't apply here.
      • Even if we do allow this for ResourceURL this might be a place where not respecting that config makes sense - this method is mostly used in templates, where we don't have the benefit of things like Controller::join_links() to handle that sort of thing for us.

Acceptance criteria

  • Better document the difference between ResourcePath and RessourceURL
  • Update ResourcePath and/or RessourceURL so they can except a folder
    • They should still validate that the folder exist
    • We don't need to honour the no trailing slash rule in this context
  • (Stretch goal)Implement a cascading version of ResourcePath/ResourgeURL.

Note

Another solution would be to allow passing variables into method calls from templates, and make those methods accept ...$args instead of a specific number of arguments. Developers could then use it like this:
<img src="$ResourcePath('dist/images/icons/', $Icon, '.svg')">
and the arguments could be concatenated together in the ResourcePath method.
This is obviously a huge change to be making, especially with the RC imminent, so I'm not actually recommending we do this now. Just noting that it's a possible solution.

PRs

@kinglozzer
Copy link
Member

I actually wrote this for a recent SS4 project that had multiple front-end themes active at once (long story):

<?php

namespace App\View;

use SilverStripe\Core\Manifest\ModuleResourceLoader;
use SilverStripe\View\TemplateGlobalProvider;
use SilverStripe\View\ThemeResourceLoader;

class ThemeResourceHelper implements TemplateGlobalProvider
{
    public static function themeResourcePath(string $resource): ?string
    {
        return ThemeResourceLoader::inst()->findThemedResource($resource);
    }

    public static function themeResourceURL(string $resource): ?string
    {
        $path = static::themeResourcePath($resource);
        if (!$path) {
            return '';
        }

        return ModuleResourceLoader::singleton()->resolveURL($path);
    }

    public static function get_template_global_variables(): array
    {
        return [
            'themeResourcePath',
            'themeResourceURL'
        ];
    }
}

Usage example:

<img src="{$themeResourceURL('dist/images/icon.svg')}" alt="" width="24" height="24" />

It works as you’d expect and uses the theme cascade (e.g. if icon.svg doesn’t exist in the first theme, it’ll look in the second, third etc).

@GuySartorelli
Copy link
Member Author

I think you'll find something familiar in the framework PR @kinglozzer 😋
Thanks for that example

@michalkleiner
Copy link
Contributor

michalkleiner commented Apr 12, 2023

All PRs merged and all ACs met including the stretch goal.

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

No branches or pull requests

3 participants