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

Add new hook for Gutenberg latest posts block #26628

Merged
merged 3 commits into from
Nov 9, 2020
Merged

Add new hook for Gutenberg latest posts block #26628

merged 3 commits into from
Nov 9, 2020

Conversation

romain-d
Copy link
Contributor

@romain-d romain-d commented Nov 2, 2020

Hello,

following my error on the WordPress trac, I redo this PR here.
FYI https://core.trac.wordpress.org/ticket/51694

Add a hook to changes arguments of Gutenberg latests posts block query

Romain DORR, Be API

Copy link
Member

@noisysocks noisysocks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @romain-d. Looks good, two notes:

  1. Let's drop core and simply name this filter block_latest_posts_query_args. In WordPress, filters that don't have a prefix are implicitly core filters.

  2. Could you please add a doc comment before the apply_filter which explains what this filter does? You can see an example of what one of these looks like here. In this case we'll want @since 5.7.0.

@romain-d
Copy link
Contributor Author

romain-d commented Nov 2, 2020

Hi @noisysocks ,

Thanks for the feedback, I made the requested changes.

Best,
Romain DORR, Be API.

@noisysocks
Copy link
Member

@romain-d: Could you please rebase master into this branch? There are some unrelated E2E test failures that are preventing us from getting a ✅.

@gziolo gziolo added [Feature] Extensibility The ability to extend blocks or the editing experience [Block] Latest Posts Affects the Latest Posts Block Needs Dev Note Requires a developer note for a major WordPress release cycle labels Nov 7, 2020
Copy link
Member

@noahtallen noahtallen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice stuff!

@noahtallen noahtallen merged commit 6d475af into WordPress:master Nov 9, 2020
@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Nov 9, 2020
@github-actions github-actions bot added this to the Gutenberg 9.4 milestone Nov 9, 2020
@youknowriad
Copy link
Contributor

I'm not really sure adding an adhoc hook for a specific block is a good idea? were there any discussion about this?

@gziolo
Copy link
Member

gziolo commented Nov 18, 2020

There is already a general-purpose hook for modifying block's data: render_block_data. There are more filters that let you hook into the rendering process. It would be great to know what type of customization they aren't covering.

@mcsf
Copy link
Contributor

mcsf commented Nov 18, 2020

I'm not really sure adding an adhoc hook for a specific block is a good idea? were there any discussion about this?

Agreed. Unless there is a really compelling for this block-specific hook, it seems important to revert so as not to open a precedent.

@noahtallen
Copy link
Member

It seems like the use case would be adding or removing an argument to the query which gets the latest posts for the block. That doesn't seem possible with the existing hooks

@youknowriad
Copy link
Contributor

Sometimes, it's not about it being possible or not. We need to make hard decisions. In this particular instance, someone could just replace the latest post block by his own block or override the render function. Yes, it's not as simple as a simple hook but it's better than having adhoc hook that we'll be forced to maintain. Especially since the Query block is meant to replace that block anyway.

@noahtallen
Copy link
Member

@youknowriad I added a revert here since I merged this: #27078

@youknowriad
Copy link
Contributor

Thank you Noah

noahtallen added a commit that referenced this pull request Nov 18, 2020
@noisysocks noisysocks mentioned this pull request Jan 28, 2021
9 tasks
@youknowriad youknowriad removed the Needs Dev Note Requires a developer note for a major WordPress release cycle label Jun 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Latest Posts Affects the Latest Posts Block [Feature] Extensibility The ability to extend blocks or the editing experience First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants