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

Improve support for snippets throughout Winter CMS #36

Draft
wants to merge 21 commits into
base: main
Choose a base branch
from

Conversation

LukeTowers
Copy link
Member

This merges the work from https://github.com/inetis-ch/oc-richeditorsnippets-plugin by @inetis-ch & @matt-pawley into the core Winter.Pages plugin.

The following features are added by this PR:

  • Support for a snippets button in the Froala WYSIWYG richeditor field that when clicked presents a list of registered snippets to the user and enables them to inject snippets into their richeditor content.
  • Support for a parseSnippets Twig filter that is used to filter content produced by said snippets button in order to actually render the snippets.

Plugin.php Outdated
@@ -236,7 +265,8 @@ public function registerMarkupTags(): array
{
return [
'filters' => [
'staticPage' => ['Winter\Pages\Classes\Page', 'url']
'staticPage' => ['Winter\Pages\Classes\Page', 'url'],
'parseSnippets' => ['Winter\Pages\Classes\Snippet', 'parse'],
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need documentation for this filter

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LukeTowers I think we should use a modified version of the original readme.md file
ref. https://github.com/inetis-ch/oc-richeditorsnippets-plugin

Do you have a suggestion on where to integrate this content ?

assets/js/froala-snippets.js Outdated Show resolved Hide resolved
assets/js/froala-snippets.js Outdated Show resolved Hide resolved
assets/js/froala-snippets.js Outdated Show resolved Hide resolved
assets/js/froala-snippets.js Outdated Show resolved Hide resolved
classes/Snippet.php Outdated Show resolved Hide resolved
classes/SnippetLoader.php Outdated Show resolved Hide resolved

// Make an unique alias for this snippet based on its name and parameters
#$snippetInfo['code'] = uniqid($snippetInfo['code'] . '_' . md5(serialize($snippetInfo['properties'])) . '_');
// the line above was commented out to allow the overriden partials in theme to be used for the component alias
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mjauvin could you explain this further to me?

Copy link
Member

@mjauvin mjauvin Jun 18, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's been a while and I don't recall specifically, but the snippet code value provided in the content (richeditor or elsewhere) cannot be modified, otherwise it breaks.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I experimented a bit and the problem is if you modify the snippet code as above, the overridden markup is not found in the theme because the "alias" is totally different.

For example, let's say you have a component named "FAQ" and it's used as a snippet, if the theme creates a "partials/faq/default.htm" file to override the default component markup, obfuscating the snippet code with uniqid($snippetInfo['code'] . '_' . md5(serialize($snippetInfo['properties'])) . '_'); will prevent that overriden markup to be found (it would search "partials/{uniqueid}" which obviously won't exist.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And I'm guessing the original reason for this code was to support multiple instances of the same component provided snippet in the same field?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using different snippet code would achieve the same goal.

classes/SnippetLoader.php Outdated Show resolved Hide resolved
classes/SnippetLoader.php Outdated Show resolved Hide resolved
classes/SnippetLoader.php Outdated Show resolved Hide resolved
// the line above was commented out to allow the overriden partials in theme to be used for the component alias

static::attachComponentSnippetToController($snippetInfo, $controller, true);
static::cacheSnippet($snippetInfo['code'], $snippetInfo);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
static::cacheSnippet($snippetInfo['code'], $snippetInfo);
static::cacheSnippet($snippetInfo);

* Store a component snippet to the cache.
* The cache is not actually saved; saveCachedSnippets() must be called to persist the cache.
*/
protected static function cacheSnippet(string $alias, array $snippetInfo): void
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
protected static function cacheSnippet(string $alias, array $snippetInfo): void
protected static function cacheSnippet(array $snippetInfo): void

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there's no need to pass a seperate variable for the alias, what do you think?

*/
protected static function cacheSnippet(string $alias, array $snippetInfo): void
{
static::$pageSnippetsCache[$alias] = $snippetInfo;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
static::$pageSnippetsCache[$alias] = $snippetInfo;
$alias = $snippetInfo['code'];
static::$pageSnippetsCache[$alias] = $snippetInfo;

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

Successfully merging this pull request may close these issues.

2 participants