-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Global Styles: load styles in iframed site editor #28731
Conversation
<BlockList className="edit-site-block-editor__block-list" /> | ||
</WritingFlow> | ||
</DropZoneProvider> | ||
<div ref={ iframeRef }> |
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 don't think we should add a new div here. Used this to demonstrate the fix. Need to look at how can we attach the ref to a DOM node within the iframe.
Size Change: +100 B (0%) Total Size: 1.37 MB
ℹ️ View Unchanged
|
Tested and confirmed that it works. |
f833184
to
8acf40a
Compare
I was hoping there was a way to attach refs to some DOM node within the iframe but I didn't see how so far. Tried using the I ended up going in a different direction: make the iframe updating itself upon style changes provided by the host document ― which was the direction @david-szabo97 pursued at #28730 but we only need to update the editor styles (inlined stylesheet), not the named ones. I don't think this is the final solution but it's working. I can look for a better way to do this in a follow-up PR. |
I believe this should this be patched in the 9.9 plugin and backported to the Core beta, so I'm flagging it as such. Feel free to correct. |
The site editor is not part of the 5.7 beta, hence, I don't think we need to backport this PR. I don't have a good sense about whether we should release a 9.9.1 with this patch or 10.0. I'll let others with more experience weigh in. As a data point, I can share why I'm undecided so I'm able to learn how others think about this: we've had errors in templates that rendered the edit site unusable that didn't warrant a patch release; also, the site editor is an experimental feature. |
@@ -112,6 +114,7 @@ export default function BlockEditor( { setIsInserterOpen } ) { | |||
<Popover.Slot name="block-toolbar" /> | |||
<Iframe | |||
style={ resizedCanvasStyles } | |||
editorStyles={ settings.styles } |
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.
Why is it also passed further down to Canvas
?
@nosolosw Why did you duplicate the logic from |
👋 @ellatrix for context, this is a summary of what needs to happen on the site editor:
I've tried to implement this flow using the hooks and left some comments at #28731 (comment) I wasn't sure how to make it work, though, hence the hotfix. Plan to iterate on this after doing some digging into the different pieces, happy to work on this with you! |
Fixes #28581
It looks like #28233 refactored the
useEditorStyles
feature and removed it from the iframe, hence the styles aren't updated upon user changes. This fixes it by bringing it back.This is what we need to have:
All of them (presets & block styles) can change upon user changes, so both the iframe and the host document need to be updated upon user changes.
Test instructions