-
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
Block editor: render block edit in popover if it's not React #47980
Conversation
Size Change: +598 B (0%) Total Size: 1.33 MB
ℹ️ View Unchanged
|
Flaky tests detected in 8c73e1f. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4165091695
|
@ellatrix I wonder if we need a tracking issue to find out if anything else is impacted.
Re #47907. Makes me wonder how many other components we ourselves have developed could have issues. |
I really like the idea of adding some mechanism like a new apiVersion to allow blocks that do support the iframed editor fully even if they use third-party libraries to be fully rendered inside the iframed editor. We added support for the iframed editor in the Apple Maps Block plugin a while back and it would be great to have a way to tell the editor that it is save to load that in the editor even if it does non react dom manipulation (via useRefEffect and the correct document owner) |
Would be happy to test this with my plugin that is also incompatible with iframes! Still trying to find a workaround, but an issue I reported yesterday might be a good candidate to test this workaround: #47983 In my case the editor side is React, but the iframe seems to break webgl contexts for things like shaders in a renderer. I like the idea of api version being a way to flag this. |
@fabiankaegy Yes, I mentioned in the description that I'd like to bump the API version to 3. |
@ellatrix Yeah sorry if my comment wasn't clear. It was meant as just agreeing with your suggestion :) |
I've been testing the branch this morning to try and validate the approach here, and it does seem to solve the core issue for us at ACF. The only issues I've found so far are around scrolling and the toolbar. It's possible to get into a state where you can't scroll to the bottom of the edit popover if it's contents changes height a lot, and the popover contents shows over the top of the bottom toolbar, so just minor things you're probably already aware of! |
@lgladdy Could you send me another block that behaves like this? :) @fabiankaegy Great! I'll add it and bump all our core blocks to 3. |
@ellatrix Sure! Attached here. Shows off most of our fields (although, not all of the values appear in the preview mode, but you'll see all the fields in edit mode) For anyone else wanting to try this, this standalone ACF Block as a plugin requires ACF PRO 6.0+ |
b9c93fa
to
8c73e1f
Compare
@lgladdy Thanks! Scroll issues should be fixed now. I'll do some more testing for other issues. One thing that's a bit jarring is when you click outside the block it reverts to preview which may change in height. |
See #48076. |
What?
Still a work in progress. This PR attempts to render a block's edit function in a popover outside the iframe if we detect that the block adds elements without React.
I think we should also create a new API version 3 that requires blocks to be compatible with the iframe. Then this back compat layer only runs for API v < 3.
Fixes #47924.
Why?
Some blocks use third party scripts that don't work in the iframe (like a query date picker etc.). Ideally these blocks should be made compatible with the iframe and render UI outside of the iframe. For example, a date picker should use the popover component so it's rendered outside the iframe. We even have our own date picker component that blocks should be using.
How?
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast