Skip to content
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

Parse blocks to generate the excerpt #11704

Closed
wants to merge 6 commits into from

Conversation

pento
Copy link
Member

@pento pento commented Nov 10, 2018

Now that #11141 has landed, we can use the same logic when generating an excerpt, which allows us to deprecate get_dynamic_blocks_regex().

By using a list of allowed blocks, we can avoid the other issue that strip_dynamic_blocks() is intended to address, when a dynamic block can call get_the_excerpt(), causing an infinite loop.

Out of interest, 198c82f is an alternate method, which just tweaks strip_dynamic_blocks() to use the parser, but I think this is the better method.

@pento pento added the [Feature] Parsing Related to efforts to improving the parsing of a string of data and converting it into a different f label Nov 10, 2018
@pento pento added this to the 4.4 milestone Nov 10, 2018
@pento pento requested review from mcsf, dmsnell and a team November 10, 2018 05:52
@dmsnell
Copy link
Member

dmsnell commented Nov 12, 2018

This hits top-level blocks but in some cases I think it might be problematic (such as the column block).

This would be a good use-case for the filter I've toyed with in #10108. If we applied a pre-render filter to the blocks then we could whitelist or blacklist down the tree. Then we wouldn't have odd behaviors with top-level vs. nested blocks.

@pento
Copy link
Member Author

pento commented Nov 12, 2018

Thanks for the review @dmsnell, that's a good point about nested blocks.

I've actually added render_block filter to Core, which would do a similar job.

https://github.com/WordPress/wordpress-develop/blob/114a1eeb3f96ab33b5432b3296fb2ffb3c5e1bdc/src/wp-includes/blocks.php#L194-L202

@youknowriad youknowriad modified the milestones: 4.4, 4.5 Nov 15, 2018
@gziolo gziolo added the [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later label Nov 19, 2018
@catehstn
Copy link

No rush to get this merged today, pushing to 4.6.

@catehstn catehstn modified the milestones: 4.5, 4.6 Nov 19, 2018
@catehstn
Copy link

@pento is this still relevant or is it replaced by the change in core?

@pento
Copy link
Member Author

pento commented Nov 22, 2018

Core uses the same code as in this PR, so both Gutenberg and core are currently wrong, in their own special way. 🙂

I think we can just implement something like #10108, and modify this change to use that new filter. This can be done in core, we don't need to bother in Gutenberg.

(Let's leave this PR open until there's a corresponding core ticket.)

@mtias mtias modified the milestones: 4.6, 4.7 Nov 22, 2018
@mtias mtias modified the milestones: 4.7, 4.8 Dec 6, 2018
@youknowriad youknowriad modified the milestones: 4.8, 4.9 Dec 19, 2018
@gziolo
Copy link
Member

gziolo commented Jan 29, 2019

I opened a corresponding ticket in core, which means this one can be closed as mentioned above. Let's continue on trac 👍

@gziolo gziolo closed this Jan 29, 2019
@gziolo gziolo deleted the add/strip-dynamic-blocks-parsed branch January 29, 2019 13:02
@Soean
Copy link
Member

Soean commented Jan 29, 2019

The new ticket: https://core.trac.wordpress.org/ticket/46133

@gziolo
Copy link
Member

gziolo commented Jan 29, 2019

Thanks @Soean, I planned to share the link but somehow I missed it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Parsing Related to efforts to improving the parsing of a string of data and converting it into a different f [Priority] Low Used to indicate that the issue at hand isn't a top priority to address and can be handled later
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants