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

Assets not loaded when using shortcode only in Twig #98

Open
nbusseneau opened this issue Aug 26, 2020 · 11 comments
Open

Assets not loaded when using shortcode only in Twig #98

nbusseneau opened this issue Aug 26, 2020 · 11 comments
Assignees

Comments

@nbusseneau
Copy link

nbusseneau commented Aug 26, 2020

Hello,

There is an issue where shortcodes used only in Twig templates (and not content) will not load assets correctly if they are added as recommended from examples (from the process function passed when adding a new shortcode handler).

Minimal plugin reproducing the issue: shortcode-asset-twig-demo.zip

Steps:

  • Install plugin. Note that blank.js is supposed to be loaded L19 in AssetTwigDemoShortcode.php while processing assetTwigDemo shortcode.
  • Create a page with shortcode [assetTwigDemo] in content.
  • Asset is loaded: <script src="/user/plugins/shortcode-asset-twig-demo/blank.js"></script> will be added to generated page.
  • Delete shortcode from content.
  • In page template, try to process the same shortcode: {{ '[assetTwigDemo]'|shortcodes }}
  • Asset is NOT loaded: <script src="/user/plugins/shortcode-asset-twig-demo/blank.js"></script> missing in generate page.

This is because Twig shortcodes are processed directly via

public function processShortcodes($str)
{
$parser = $this->getParser($this->config->get('parser'));
$processor = new Processor(new $parser(new CommonSyntax()), $this->handlers);
return $processor->process($str);
}
rather than
protected function processShortcodes(PageInterface $page, $type = 'processContent') {
$meta = [];
$config = $this->mergeConfig($page);
// Don't run in admin pages other than content
$admin_pages_only = $config['admin_pages_only'] ?? true;
if ($admin_pages_only && $this->isAdmin() && !Utils::startsWith($page->filePath(), $this->grav['locator']->findResource('page://'))) {
return;
}
$this->active = $config->get('active', true);
// if the plugin is not active (either global or on page) exit
if (!$this->active) {
return;
}
// process the content for shortcodes
$page->setRawContent($this->shortcodes->$type($page, $config));
// if objects found set them as page content meta
$shortcode_objects = $this->shortcodes->getObjects();
if (!empty($shortcode_objects)) {
$meta['shortcode'] = $shortcode_objects;
}
// if assets founds set them as page content meta
$shortcode_assets = $this->shortcodes->getAssets();
if (!empty($shortcode_assets)) {
$meta['shortcodeAssets'] = $shortcode_assets;
}
// if we have meta set, let's add it to the content meta
if (!empty($meta)) {
$page->addContentMeta('shortcodeMeta', $meta);
}
}
which is the code responsible for preparing assets insertion into page, along with its sibling
public function onPageContent(Event $event)
{
if (!$this->active) {
return;
}
$page = $event['page'];
// get the meta and check for assets
$page_meta = $page->getContentMeta('shortcodeMeta');
if (is_array($page_meta)) {
if (isset($page_meta['shortcodeAssets'])) {
$page_assets = (array) $page_meta['shortcodeAssets'];
/** @var Assets $assets */
$assets = $this->grav['assets'];
// if we actually have data now, add it to asset manager
foreach ($page_assets as $type => $asset) {
foreach ($asset as $item) {
$method = 'add'.ucfirst($type);
if (is_array($item)) {
$assets->$method($item[0], $item[1]);
} else {
$assets->$method($item);
}
}
}
}
}
}

Two simple workarounds:

  • Add a commented shortcode to content, like <!-- [assetTwigDemo] -->. The shortcode will be correctly processed (thus, assets are loaded), but since it's commented it won't render anything on the page. This has the disadvantage of leaving a comment in the generated source code.
  • Add assets directly on shortcode init() rather than in the process function. You can uncomment L19 in AssetTwigDemoShortcode.php to test this workaround. This has the disadvantage of loading assets for this shortcode on all pages, even when no shortcode of this type is present.

I guess the proper solution would be to load assets not only when processing content but also when processing Twig directly. What do you think? If you like this approach, I'm willing to try my hand at an implementation.

Cheers!

@mahagr
Copy link
Member

mahagr commented Sep 14, 2020

@rhukster Looks like this is a valid bug.

@pamtbaau
Copy link
Contributor

Any status change on this issue?

Trying to use Shortcode Owl Carousel plugin. It works fine when using from markdown, but not in Twig.

Code is parsed fine in Twig, but assets (javascripts, stylesheets and inline javascript) are missing.

@nbusseneau
Copy link
Author

nbusseneau commented Dec 21, 2020

I don't think there has been any progress on this. I still have it on my radar for potential contribution, but considering the use case and the existing workarounds, I personally decided to put it at the very bottom of my priorities when compared to other stuff here and there in the Grav ecosystem.

The workarounds still work fine by the way ;)

@pamtbaau
Copy link
Contributor

I'm afraid the workarounds do no work for shortcodes which assets depend on the ShortcodeInterface $sc passed into the handler.

@nbusseneau
Copy link
Author

nbusseneau commented Dec 22, 2020

I'm not sure I understand what you are meaning by "assets depending on the ShortcodeInterface $sc passed into the handler", can you perhaps share a code example?

From what I see over at the Owl Carousel shortcode plugin code, it looks to be loading static assets via addAssets directly at process time:
https://github.com/getgrav/grav-plugin-shortcode-owl-carousel/blob/8c5e2326d99f8640807bb89cb1266e69a6e9be1e/shortcodes/OwlCarouselShortcode.php#L33-L38

which from my testing will indeed break when used from Twig only.

However it should still be possible to have a commented Owl Carousel shortcode in content, so as to force load assets and have them available for when Twig needs it. This is actually exactly the case I was encountering initially, and for which I am still using the same workaround up until now. So I'm puzzled as to why it wouldn't work in this case 🤔

@pamtbaau
Copy link
Contributor

pamtbaau commented Dec 22, 2020

@Skymirrh ,

An example of assets depending on ShortcodeInterface $sc could be:

$id = 'owl-' . $this->shortcode->getId($sc);
...
$this->shortcode->addAssets('inlinejs', '$(document).ready(function(){ $("#' . $id . '").owlCarousel(' . json_encode($params, JSON_NUMERIC_CHECK) . '); }); ');

The inline jQuery script looks for a div with an id which is generated using $sc.

The commented Owl Carousel in Markdown would generate a script targeting an id specific to that shortcode, which is different from the id generated for the shortcode in Twig.

Another example could be when assets depend on the parameters provided for the shortcode.

if ($sc->getParameter('animate') == 'true') {
   $this->shortcode->addAssets('css', 'plugin://shortcode-owl-carousel/css/animate.css');
}

@nbusseneau
Copy link
Author

I see, I missed that part while glancing at the code! Thanks for the clarification. You're right: in this case, none of the above workarounds will help :/

@pamtbaau
Copy link
Contributor

pamtbaau commented Dec 22, 2020

@rhukster / @mahagr
Interesting twist to the bug: The above issue with Owl Carousel shortcode does not occur when the shortcode is added to the Twig of a child module of a modular page. All assets are added correctly.

@rhukster
Copy link
Member

Have this In it list to look at tomorrow.

@vied12
Copy link

vied12 commented Oct 31, 2022

Do you have any update on this topic? Since I move to php8 and probably did some updates on Grav, Owl Carousel shortcode is not adding any asset. Thank you for your answers 🙏

@rhukster
Copy link
Member

sorry never did get around to it. just too many balls to juggle. would be nice if some others could jump in and debug this as I just don't have enough hours in the day to do it all.

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

No branches or pull requests

5 participants