Iframe: preload style assets to avoid flash of unstyled content #46706
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Size Change: +15 B (0%) Total Size: 1.32 MB
ℹ️ View Unchanged
|
ellatrix
commented
Dec 21, 2022
| // Correct doctype is required to enable rendering in standards | ||
| // mode. Also preload the styles to avoid a flash of unstyled | ||
| // content. | ||
| srcDoc={ srcDoc } | ||
| title={ __( 'Editor canvas' ) } | ||
| > | ||
| { iframeDocument && | ||
| createPortal( |
There was a problem hiding this comment.
Ideally we do something like hydrateRoot in the future so existing HTML is reused.
youknowriad
reviewed
Dec 21, 2022
youknowriad
reviewed
Dec 21, 2022
| // WARNING: use getEditorSettings from the editor store, not the | ||
| // block editor store. The settings on the block editor store set | ||
| // with a delay and we need the assets on the first render for the | ||
| // iframe srcDoc. |
There was a problem hiding this comment.
I think that's not the right question. We shouldn't be switching stores to use because the settings are not set yet.
IMO, we should ask ourselves whether these settings are supposed to be generic "block editor" settings, if yes, we should continue to use the block editor store and move the useSelect to inside the "iframe" component. If not, we should remove them from the block editor settings entirely.
There was a problem hiding this comment.
Thinking more, these settings are already considered block editor settings so I think we should just move the useSelect inside the iframe
There was a problem hiding this comment.
That's a good point. It shouldn't really be in block editor settings at all I think.
There was a problem hiding this comment.
Seems like we do need it for the BlockPreview component, which is in the block-editor package:
There was a problem hiding this comment.
Can we then restore the block editor selectors and move them inside the iframe component instead, that way they are included by default in all iframes? I guess this also means we need an "isReady" somewhere, I don't really see a way around it personally?
There was a problem hiding this comment.
Done. How does it look to you now?
ellatrix
force-pushed
the
try/preload-iframe-styles
branch
from
9ad4751
to
449d81a
Compare
|
Let's see if this breaks any e2e tests now that the iframe is not rendered on first editor render. |
ellatrix
force-pushed
the
try/preload-iframe-styles
branch
from
af3cfec
to
0dd8203
Compare
youknowriad
reviewed
Dec 23, 2022
| const settings = select( store ).getSettings(); | ||
| return { | ||
| styles: settings.styles, | ||
| assets: settings.__unstableResolvedAssets, | ||
| duotone: settings.__experimentalFeatures?.color?.duotone, |
There was a problem hiding this comment.
It makes me wonder about these two remaining things: styles, duotone. How are they being handled in the case of the regular editors? Wondering if there's a way to move them to the iframe automatically?
There was a problem hiding this comment.
The styles are passed to <EditorStyles>, which is created separately from the Iframe and then passed to it. I guess at some point we can also integrate this.
Don't know about duotone. It seems to create SVG filters and then renders it in the content area. Tbh, that doesn't feel like the iframe's responsibility, Ideally this should be isolated to the duotone hook.
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
Preloads the iframe stylesheets by adding them to the srcDoc. I also had to get assets through editor settings from the editor store because the assets were not in the block editor store on initial render.
Why?
To avoid a flash of unstyled content.
How?
Testing Instructions
Create a new post with a block based theme. When reloading the page, you briefly see the default browser focus outline on the title. Also the appender button is unstyled. This should be gone.
Testing Instructions for Keyboard
Screenshots or screencast