Footnotes: Attempt to iterate into nested blocks that use useInnerBlocksProps in the save function#52676
Closed
andrewserong wants to merge 1 commit intotrunkfrom
Closed
Footnotes: Attempt to iterate into nested blocks that use useInnerBlocksProps in the save function#52676andrewserong wants to merge 1 commit intotrunkfrom
andrewserong wants to merge 1 commit intotrunkfrom
Conversation
…cksProps in the save function
andrewserong
added
[Type] Bug
|
Size Change: -2 B (0%) Total Size: 1.43 MB
ℹ️ View Unchanged
|
ellatrix
reviewed
Jul 17, 2023
| for ( let i = 0; i < blocks.length; i++ ) { | ||
| const { name, attributes, innerBlocks } = blocks[ i ]; | ||
| const saveElement = getSaveElement( name, attributes ); | ||
| const saveElement = getSaveElement( name, attributes, innerBlocks ); |
Member
There was a problem hiding this comment.
We should avoid passing this down as block because they'll be serialised, which is unperformant.
ellatrix
reviewed
Jul 17, 2023
| return; | ||
| // `useInnerBlocksProps.save()` will return a `RawHTML` element, | ||
| // so use this as an indicator to recurse into the children. | ||
| return addValuesForBlocks( ...args ); |
Member
There was a problem hiding this comment.
How would you know that this is the correct RawHTML instance for which to process inner blocks?
Member
|
Looks like #52682 has been merged already, closing this one for now. Thanks for jumping in on this one @andrewserong ! |
Contributor
Author
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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?
Fixes #52628 (hopefully!)
Over in #52241
getRichTextValueswas updated to work withInnerBlocks.Content, however as a result, it seems that nested blocks that don't use that component stopped being iterated over for extracting footnotes (e.g. the Group block, which usesuseInnerBlocksProps.save()directly).This PR attempts to recognise where in the block's output inner blocks are being rendered, by looking for
RawHTML— if we match against that, then assume that we need to iterate into the block's inner blocks. The assumption in this PR is that we won't have blocks that useuseInnerBlocksProps.save()andRawHTMLin their output intentionally. This is a bit of a hack, however it is preferable to an alternative:In my first attempt at this PR, I thought I'd try adding a line to
addValuesToBlocksso that it could recurse into itself around here: — basically, after working through all the elements for a block, iterate over its inner blocks. By doing that, we could then remove the explicit check forInnerBlocks.Content. The problem, though, is that this would mean that the iterating over children would always happen after all the markup for the current block is processed. For the Quote block, this resulted in footnotes being in the wrong order, as in that block,InnerBlocks.Contentappears beforeRichtText.Content. So, we need some way to ensure that the iteration always occurs at the correct place within the block's final markup.Looking for
RawHTMLis imperfect, but for the moment appears to achieve better results than we have on trunk. I'm very happy to throw this PR away if folks have better ideas about how to achieve this!Why?
Prior to this PR,
getRichTextValuesonly iterates over blocks that useInnerBlocks.Content(e.g. the Quote block, or List blocks). This means that blocks that are contained in Group blocks are never reached for extracting footnotes, as they do not useInnerBlocks.Content.How?
innerBlockstogetSaveElementso that inner blocks are rendered to aRawHTMLcomponent, so that we can useRawHTMLas a flag for when to iterate over inner blocks.addValuesForElement, useRawHTMLin addition toInnerBlocks.Contentas a flag to continue iterating into inner blocks.Testing Instructions
Screenshots or screencast
0or*