-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Post editor: iframe if all blocks are v3 #48286
Conversation
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.
As always thank you for working on this :) I'm really excited about this!
I know this is still in draft mode and you haven't asked for reviews yet. I added one comment where something is currently still missing but otherwise this is looking great from a code perspective :)
Size Change: +2.39 kB (0%) Total Size: 1.39 MB
ℹ️ View Unchanged
|
Flaky tests detected in 5f13723. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5155101187
|
40cc5b0
to
0b4e818
Compare
61ece7e
to
4ee7c72
Compare
4ee7c72
to
afac048
Compare
Here's the plan: I'm going to force some e2e tests that I'm having a difficult time with to run without the iframe and address this later in follow-up PRs. I think it's important to get this PR in asap to get some testing before beta 1. It's just too overwhelming for me to fix the remaining e2e while continuously running into merge conflicts with trunk and people adding more e2e tests. The only thing this PR does is updating all blocks to v3 and fixing e2e tests. |
Perfect, thanks for the clarification @ellatrix 🙏 |
@ndiego I think this conflicts with the information you posted on the Make blog, doesn't it?
Addition of styles is clear; addition of block registration and block-specific code is also clear. But what about sidebar panels, data store registration and so on? |
What do you mean? This is all editor UI, and it's all editor APIs. |
Thanks for continuing to help clarify these new changes. I have no questions related to styles: I'm asking about non-block-related scripts which need to load in the editors. Unless I'm reading it incorrectly, the dev note on the Make blog, which only really details how to approach styles, says that developers should be using Your comment (#48286 (comment)) states “ If both of those are correct, then which hook do we use to load non-block scripts in the framed editors? |
If you need scripts/styles for the editor UI, use If you need scripts/styles for the editor content (the part in the |
Yes. I understand it's a bit confusing that you have to use |
That's great, thanks Justin and Ella. |
@ellatrix @ndiego @justintadlock Sorry for my late answer here. After some investigation and debugging I was able to trace the source of the issue to my code. (During debugging I've come across an issue of identifying styles, so I've logged a suggection at #53756) |
Hi @ellatrix, Thank you for all of your hard work on this. The consistency of the iframed editor being available across different contexts is a huge step up in what can be done by 3rd party devs moving forward. I noticed that whenever custom fields are enabled in the post editor, the editor is no longer iframed. This is causing some issues with a plugin I'm working on. Is there a reason for this or is this just something that hasn't been looked at yet? |
Yeah, this is intentional for backward compatibility. Whenever metaboxes are present, the Editor is also no longer iframed. |
Thanks @ndiego! Editing my post here because I realized I didn't ask questions I meant to ask. Do potential issues with backward compatibility have to do with the metabox section being in the iframe as well? What could potentially break if the editor is iframed but the metabox section is left outside of the iframe? @ellatrix & @ndiego - does this mean that there is no immediate plan to move toward an iframed editor in all editor contexts, including when metaboxes are present? Just my opinion, but with almost every editor context iframed now, it feels a bit unintuitive for that singular context to be un-iframed. Also, since posts/pages can easily be very long with the block editor, having custom fields that far out of reach seems unintuitive as well. I'm not really sure if this is the right place for posting ideas (and please let me know if there's a better way to address things like this), but here are some ideas I have: Idea 1) When enabled, the custom fields section could be a separate { position: sticky; bottom: 0; } element outside of the editor iframe. Similar to how it is now except it would always be stuck to the bottom of the .interface-interface-skeleton__content container. It actually seems like the structure is already there for this when the editor is iframed (ie. metaboxes are disabled), but the metabox data just isn't populated so the dom structure content is blank. This is the current dom structure when the editor is iframed: If this is possible without unacceptably breaking backward compatibility, it would make custom fields more accessible while still not taking up too much space in the editor when collapsed. It also leaves the door open for eventually adding a show custom fields option to the site editor which might be desirable at some point. I haven't checked how the custom fields section appears on mobile but I assume it would be more manageable as a separate section in that context as well. Idea 2) Add a togglable modal that contains the custom field section. This would most likely be located in the post sidebar. Ideally, custom fields make the most semantic sense there anyway (if the section wasn't so large). If either of these are possible without breaking backward compatibility, I think idea 1 is better as it would feel familiar, and would allow for an iframed editor and easier access to custom fields for users. Let me know your thoughts! |
I can't speak for @ellatrix but backward compatibility is the reason things are moving so slowly here. There are millions of WordPress users who run sites that use metaboxes, often with sophisticated implementations that were originally designed for a "Classic" environment. Custom fields is Core functionality, so I could see that being migrated sooner rather than later, but that does not solve third-party metaboxes. As I am sure Ella can attest, this is an extremely tricky thing to solve without breaking things. From a Developer Relations perspective, the goal now is to help/encourage plugin authors to migrate away from metaboxes and update all custom blocks to API v3. |
@briangrider See #34421 for a previous attempt. Actually there's no backward compatibility issues with metaboxes and the iframe. The metaboxes must be rendered outside the iframe of course. The problem is that there's now two scrollable areas that need to fit into that space, which is a no-go. Alternatively we need there to be some sort of accordion to switch between content and meta boxes, or they need to be moved to a modal (that would be my preference since we're talking about a legacy API). If you have idea, I'm happy to review any PR |
Thank you for your response @ellatrix. For your previous attempt, was it just an issue of having more important things to take care of that made you put it on hold? I've never contributed to Core but would love to eventually. I can't jump on this right now but if it hasn't been addressed in a few months, I'll take a look at the code and see if I can add anything valuable to the conversation. |
Given that there are no compatibility issues with metaboxes and iframes, could we explore the option of allowing to opt-in for iframe functionality even when metaboxes are active and maybe enabling the tab feature from #34421? Otherwise the only way to use the iframe editor is to not use plugins that rely on metaboxes, which in my opinion is way worse than the alternative. |
Agree with @abredikis, we need some way to enable the iframed editor even with metaboxes present, just had this issue with a website I'm building, used v3 on all blocks to make sure we get an iframed editor, and everything worked just fine, until we had to install an SEO plugin, but doing so disables the iframed editor because they make use of metaboxes. |
See #59242 for the latest on meta boxes. |
What?
This PR reintroduces the iframe for the post editor, but only if all registered blocks are v3 or higher.
Previously: #48076, #46212, #21102.
Fixes #33346, #6156, #21193.
Why?
How?
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast