-
Notifications
You must be signed in to change notification settings - Fork 13
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
Dynamic Rendering #297
Dynamic Rendering #297
Conversation
@janboddez Would appreciate your input as well. This PR is only a proof of concept so far, and I tried to come up with a backward compatible solution. |
Whoa! Awesome! This is a huge deal. Thank you so much for tackling it! I'll refrain from reviewing the code itself, my head has been out of this codebase and WordPress in general for a long time, but I have tons of questions! I'll start with just a few:
|
For now it calls the same code it was using. If I merge this, the next PR would improve and rewrite that code. |
That's definitely an endorsement of this approach. I experimented a bit with the idea of including an mf2 parser to try an interpret what is marked up inside. Also, the next version will remove storing the content property in meta, so it can use the presence of the content property to undo the changes. |
Oh, IndieBlocks currently uses the So, as long as that hook is called the same way, I'm good. (If it ever gets deprecated, I'm going to have to fall back to some lower-level WordPress hook, which may prove tricky.) May have to prevent Micropub from re-rendering the content I saved, though. Should be no more than a |
The micropub_post_content filter will stay as it is. In this case, all you would have to do is add a simple line. add_filter( 'micropub_dynamic_render', '__return_false`, 99, 2 ); Or since the 2nd argument will be $post... Built in by default it will default to true if there is a version property in the micropub_auth_response, which I'll be adding from now on, or if there is an mf2_content property, which will be used prior to this version. |
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.
lgtm (but not tested)
@@ -224,11 +227,12 @@ into markdown and saved to readme.md. | |||
|
|||
## Changelog ## | |||
|
|||
### 2.4.0 (202x-xx-xx) ### | |||
### 2.4.0 (2024-xx-xx) ### |
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.
Should this date be filled in before deployment?
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.
Before release, yes, but not before merge. I always set the date before I release the version.
Thanks again for doing this, it's a huge step forward for microformats2 support in WordPress. I skimmed the code, it seems fine, but I don't live in WordPress, so honestly that doesn't mean much. Still, the facts that it's only dynamic going forward, IndieBlocks-compatible, and preserves the existing rendering for now are all great. If @pfefferle's on board, then I am too. Exciting! |
This is an intermediate step to add dynamic rendering. It keeps pre-2.4.0 posts using the static rendering, but everything new, that has the version tag within the Micropub auth response would use dynamic rendering unless either the Post Kinds plugin is enabled or some other unknown plugin changes the micropub_dynamic_render to true.
The only other plugin that amends the post content is @janboddez 's IndieBlocks, which converts the incoming Micropub content to blocks.
If this solution gets positive feedback as a solution my next step will be to enhance the rendering code to address all open issues. The idea being that this will render a simplified version of the data, Post Kinds will take that over if it is enabled, as will Indieblocks or a theme by removing the dynamic render filter or adding to the new filter.