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

use actuall block markup for the storie previews #35

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

fabiankaegy
Copy link
Collaborator

While working with the BlockBook I came across an issue where I had no way of checking the actually markup produced by the blocks ( within the save method ).

I'm sure there was a reason behind you using the BlockPreview component, but I would love to know what that is, and wether you see a place for something like what I did in this pr?

@youknowriad
Copy link
Owner

Yes, the reason behind using BlockPreview is actually dynamic blocks. Most of them have an edit version that mimics the frontend and relies on a REST API but without running the PHP logic which can't be used here.

It is also the same reason why the BlockPreview component in Gutenberg relies on edit and not save. I think we should just follow whatever Gutenberg decides to use here.

Maybe we could show the "save" as a tab or something.

@fabiankaegy
Copy link
Collaborator Author

I like the idea of the save tab :)

Would you say the implementation I have is okay or do you have a better way to do this than dangerouslySetInnerHTML ?

@youknowriad
Copy link
Owner

I think it's good 👍 .

@fabiankaegy
Copy link
Collaborator Author

@youknowriad I have updated this PR with an implementation of a Save tab. I am not confident in the wether the styles from current Theme get selected or what needs to be done to get them to be applied. Would love your feedback 👍

@fabiankaegy fabiankaegy self-assigned this Aug 11, 2020
@fabiankaegy fabiankaegy requested review from youknowriad and removed request for youknowriad August 19, 2020 11:42
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

Successfully merging this pull request may close these issues.

None yet

2 participants