Iframe the template editor #31375
Merged
Iframe the template editor #31375
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: +269 B (0%) Total Size: 1.31 MB
ℹ️ View Unchanged
|
ellatrix
force-pushed
the
try/iframe-template-editor
branch
2 times, most recently
from
be5c897 to
f7faf9b
Compare
youknowriad
added
the
Needs Dev Note
|
I think we should write a dev note about iframing the editors for block authors for the next release. Explain to them why it's important, how it works, how it may break some of their blocks and how to address it. |
|
It would be good to test this PR properly in order to see whether we can include in 10.6 |
youknowriad
reviewed
May 4, 2021
| styles, | ||
| style, | ||
| } ) { | ||
| const ref = useMouseMoveTypingReset(); |
There was a problem hiding this comment.
What is this hook about?
There was a problem hiding this comment.
It's the same one we have in the site editor. It tracks mouse movement outside the iframe for setting isTyping to false. With #31280, it would be integrated in the block editor so the implementor doesn't need to think about it.
youknowriad
reviewed
May 4, 2021
youknowriad
reviewed
May 4, 2021
| // Allow scrolling "through" popovers over the canvas. This is only called | ||
| // for as long as the pointer is over a popover. Do not use React events | ||
| // because it will bubble through portals. | ||
| const toolWrapperRef = useRefEffect( ( node ) => { |
There was a problem hiding this comment.
If feels like this has the potential to break popovers with scrollbars though?
There was a problem hiding this comment.
Those are rendered in the normal popover slot, which is outside this div. This slot is solely used for popovers that are placed over the content.
|
@youknowriad Yes, this will need a dev note. Worth noting that this is for new (site) content and not for post content, so I think we can feel a bit freer to break backward compatibility with this new feature and try to resolve issues in preparation for iframing the post editor. If we don't do this now, we'll find it a lot hard to get any real world testing. |
|
The toolbars visually breaking out of the frame looks a little strange. It seems there's a balance between usability and visual awkwardness to negotiate there. Maybe it's ok... I'm not sure? :)
I like that the back button is now fixed in position. I do think that the frame can probably sit flush against the bottom of the window in desktop orientation though.
I'm sure it's unrelated, but noting just in case: I seem to recall @youknowriad adding an interaction where clicking on the frame would deselect all blocks. That doesn't seem to be working here. |
|
Yes, the difference here is that the iframe is creating the scroll area, not the skeleton area, which is just like the front-end. |
I think it's fine for the block UI to be a bit "magical" :) I'd also agree on removing the bottom padding, so it looks like you'r seeing the top of the document. As far as dev notes, I'd love for them to truly emphasize the benefits of the iframe and indicate those benefits should ideally reach all editors. Hopefully that would also help us avoid a fragmentation of blocks that work in the post editor sans-iframe, but not the site editor. Accurate media queries and sandboxed CSS in both directions have such value they should be everywhere 🎉 |
ntsekouras
reviewed
May 4, 2021
packages/block-editor/src/components/block-selection-clearer/index.js
Outdated
Show resolved
Hide resolved
It would be nice, but then when you're all the way at the bottom, there wouldn't be any padding. I'm not sure how to handle that when the scroll area is within the iframe, but regardless of any fix, the scroll area has to be in the iframe and we can't add the padding inside the iframe. :) |
I think that's ok. It's not really any different to the post editor, no? |
|
Ok, I removed the padding at the bottom. |
ellatrix
force-pushed
the
try/iframe-template-editor
branch
from
21c5559 to
3f3e7c5
Compare
|
One small hair to split on the design side – now that the bottom padding is gone, the border radii on the bottom of the iframe look a little strange, we should probably set that to 0. |
|
@jameskoster I removed the border and border radius on the bottom. |
jameskoster
approved these changes
May 5, 2021
7 tasks
|
Let's give it a try. |
7 tasks
15 tasks
youknowriad
removed
the
Needs Dev Note
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.
Description
(Also enables the device previews which previously didn't work.)
Why is this important?
For the users, it will make the editor more like the front end. Imagine using
vwunits, a cover background, or responsive fonts, this will now work correctly.For block authors, it is perfect for easing into iframe context and making sure blocks work there.
Eventually all editors that have theme styles should be iframed, see #21102 for the reasoning, but it's not trivial to implement in the post editor because of backwards compatibility. All block need to continue to work, but with the iframe, some blocks may need to be updated to not use globals and refs instead.
Since the template editor is new, we don't need all blocks to work here (same as FSE) while it also offers us a nice transitioning place for pushing plugins to work with the iframe.
If we don't do this before we release the template editor, we will need to make blocks work flawlessly that have already been saved here before.
How has this been tested?
Screenshots
Types of changes
Checklist:
*.native.jsfiles for terms that need renaming or removal).