-
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
Use controlled InnerBlocks in reusable blocks #23601
Use controlled InnerBlocks in reusable blocks #23601
Conversation
Size Change: -52 B (0%) Total Size: 1.13 MB
ℹ️ View Unchanged
|
The test failures seem legitimate but I'd love @ZebulanStanphill to validate that the fix actually solves the problem identified in #21427 before I clean those up. |
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.
On the bright side, the reusable block no longer appears empty for a moment when switching between editing and not editing.
However, it looks like things are overall a lot more broken at the moment.
Try inserting two of the same reusable block, and then editing one of them. The other one will disappear until you stop editing.
Also, editing the reusable block is pretty broken. Try typing a bunch of characters in the reusable block and then saving; the saved version will be missing the last few characters. This lines up with the current end-to-end test failures.
Maybe some of this has to do with handleModifyBlocks
no longer being called upon the input
event? Not really sure.
This one is super interesting. I wonder if @noahtallen has any ideas about this — perhaps there's something about the way controlled InnerBlocks handles state that would implicitly "link" the two instances together?
This is definitely concerning, and does seem likely related to onInsert vs onChange. It's possible onChange needs to be called more frequently, or perhaps that controlled InnerBlocks may need to support onInsert in some way. |
@chrisvanpatten I think controlled |
I just checked, and it turns out the can't-have-more-than-one kind of problem isn't exclusive to reusable blocks. A similar bug happens with Template Part (recently renamed to Section) blocks. See #22639. |
Rad! I didn't know! Going to need to go back and update some of my own code… |
Unfortunately, adding |
@ZebulanStanphill I think it did help? In the first commit, "Reusable blocks › can be inserted and edited" and "Reusable blocks › can be converted to a regular block" failed, but in the The remaining issue definitely stems from #22639 though. |
@chrisvanpatten Huh. I tested out the branch myself earlier and the behavior didn't seem any different. But I just tested again and now it looks like the typing-quickly issue is fixed. I guess it might have been a browser cache issue on my end? Anyway, you're right; that issue is fixed. 😃 |
I bet the issue you linked to covers everything! Another observation is that there is a significant amount of state in |
91676c0
to
62852c5
Compare
62852c5
to
cd545a7
Compare
Just noting that this would also solve these additional issues:
|
Not entirely sure what you mean, but a reusable block containing multiple blocks at the top-level has always been allowed and intended. The reusable block itself acts like a parent block, albeit one that does not produce any wrapping markup on the front-end. |
@ZebulanStanphill Good to know! Then disregard that sentence :) |
cd545a7
to
a2f7f0b
Compare
I missed this branch. I opened a new fresh one (similar in purpose here) #27887 |
Thanks for taking over here, @youknowriad! |
Description
A small PR branched from #21427 which switches reusable blocks from a nested BlockEditorProvider area to the controlled InnerBlocks API introduced in #21368.
How has this been tested?
Tested in browser.
Types of changes
Non-breaking change to reusable blocks.