-
Notifications
You must be signed in to change notification settings - Fork 13
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
Add template editor rendering #69
Conversation
Just remove /** DocBlock syntax for comments and use // instead.
This reverts commit 6b7fde5.
This should test that it actually works better than testing helper methods.
Also, add a comment for where the template editor markup will be rendered.
Though this doesn't work yet, it'll need to look for {{}}.
… 'foo' ) The integration test is passing, but there are still edge cases to handle.
… 'foo' ) The integration test is passing, but there are still edge cases to handle.
Simply add a return at the end of the if before it.
Ensure that this renders in {{}} as expected.
It's possible, though probably not advisable, that users have field names with { or }.
If they have a tutorial on Mustach, they might need to render {{example}} without this doing block_field( 'example' ). So they can escape it like \{\{example\}\} and this will strip the escaping on rendering.
Also, don't render the CSS again in the same document.
This is a little hacky, but the tests are passing ;).
To test render_css(). render_markup() is tested in integration tests.
…ndered If there was no template to render, look for CSS to render in the <style>.
} | ||
|
||
if ( ! empty( $this->blocks[ "genesis-custom-blocks/{$name}" ]['templateMarkup'] ) ) { | ||
$this->template_editor->render_markup( $this->blocks[ "genesis-custom-blocks/{$name}" ]['templateMarkup'] ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* | ||
* @inheritdoc | ||
*/ | ||
public function setUp() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The inheriting classes can implement this instead.
A DockBlock was just copied, and it doesn't help much.
I couldn't figure the fields out, there's no official documentation on it, and the type of the option looks wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is working great for me.
I noticed that in the editor preview, the image has a max height on it so it doesn't match the front end preview, but not a big deal because in the acutal editor, when you click outside the block, it displays similar in the editor and front end.
However, it would be a nice touch to account for theme CSS in this, but not necessarily in this PR. For example, in this case, I have a heading in the template editor:
But, it displays this way in the preview:
And this way on the front end:
Really nice job, @kienstra!
@dreamwhisper,
Great idea, your front-end photo looks a lot better, using theme CSS. I'll keep that in mind for future PRs. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great, Ryan. I'm really excited to see this update, I bet a lot of customers will enjoy this.
return; | ||
} | ||
|
||
if ( ! current_user_can( 'edit_posts' ) || ! isset( $templates[0] ) ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if it makes sense to introduce a custom user permission for this? Maybe current_user_can ( 'edit_genesis_custom_blocks' )
or something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm, we could use genesis_custom_block_edit_block, but I'm not too sure it would make a big difference.
Here's the main context they would see this in:
If we used genesis_custom_block_edit_block
, it would mean editors wouldn't see it.
But either way would probably be fine.
php/Blocks/TemplateEditor.php
Outdated
|
||
// Escape characters before { should be stripped, like \{\{example\}\}. | ||
// Like if they have a tutorial on Mustache and need the template to render {{example}}. | ||
echo preg_replace( '#\\\{\\\{(\S+?)\\\}\\\}#', '{{\1}}', $rendered ); // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's a pretty simple regex pattern, nice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice that you could write about mustache too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do wonder if this could use escaping as well. Just concerned that security researchers might have an issue with this, or even the core plugins team.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, good point. Hm...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about df28138? It's escaped with wp_kses_post()
.
php/Blocks/TemplateEditor.php
Outdated
$this->blocks_with_rendered_css[] = $block_name; | ||
|
||
?> | ||
<style><?php echo $css; // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped ?></style> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there any way to escape this? I can't really think of a way that would be straightforward enough. Just concerning anytime output is not escaped.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point. Let me think about that.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about wp_strip_all_tags() with eb4bc47?
The 'Additional CSS' Customizer control does something very similar.
Though it also validates it client-side.
This might be out of scope, but when testing I noticed that changes I make to the markup are not reflected in the frontend preview unless I save. It seems like it would be preferable (to me anyway) to see those previews prior to saving. |
Hi @johnstonphilip,
That's a good point, it's not ideal. The idea is to use the front-end preview that is saved, not just the one that's edited. That's because the back-end only knows about the registered fields that were saved. So if the editor sends new fields that aren't saved to the back-end, there will be an error in the preview. Still, I'll keep that it in mind. It's not a great UX. |
This only escapes markup, not CSS.
It's not ideal, but shouldn't allow JS.
$this->blocks_with_rendered_css[] = $block_name; | ||
|
||
?> | ||
<style><?php echo wp_strip_all_tags( $css ); // phpcs:ignore WordPress.Security.EscapeOutput.OutputNotEscaped ?></style> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@johnstonphilip, thanks a lot!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good stuff @kienstra
@johnstonphilip, thanks a lot for reviewing this! |
Hi @mikemcalister,
Thanks so much for reviewing this! Yeah, I hope they like it 😄 |
Changes
Testing instructions
composer install && npm install && npm run dev
/wp-admin
> Custom Blocks > Add New