-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Post Content/Title: Reflect changes when previewing post #37622
Conversation
@aristath, I wasn't able to provide a lot of context regarding this condition, so I'll post my best guess below. Would you please let me know if I'm correct? gutenberg/packages/block-library/src/post-content/index.php Lines 39 to 41 in 1b571a9
Is this correct? |
Excellent work tracking this down @Mamaduka, thank you! I can confirm this PR fixes the preview of content for published posts. One issue remains: changes to the post title are not reflected in the preview (which makes sense given your fix only applies to To reproduce:
expected result: the updated title should be shown I'm going to dig a bit more into the title issue: still this is a big improvement so we should land it either way before the next release |
In In my testing, changes to the post title are now correctly reflected in the post preview. I'm not sure why this call would ever need to pass a post id, so I removed it. If I missed something here, please feel free to correct me! |
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.
Looks good!
61c1402
to
851a0bc
Compare
Thanks for testing, @adamsilverstein. I also realized that I could omit passing I decided to leave a modified version of my original comment just in case to avoid future regressions. Plus added new commit regarding |
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 👍
Tested and the changes are now correct in the preview and in the saved post.
Should the $post_ID
be removed from the Post Featured Image as well?
The I'm going to merge this PR (the failing test isn't related) and follow up with an excerpt block update. |
@Mamaduka while it is true we don't currently directly support meta revisioning, the specific case of the featured image for post previews was addressed previously in #9151 so we should endeavor to ensure that works correctly with block themes to avoid a regression. |
I tracked the featured image down to what looks like an existing core issue, although I don't quite understand why the issue is triggered by block themes. In Deleting the old autosave revision means that later when I suspect this may also be the source of the failure of the title and content to display the preview data, typically core would automatically substitute the autosave data in the preview context (even if you passed the post id in the calls to get the title/content) I am going to follow up with a trac ticket to fix the autosave removal which I believe is a bug. In the meantime you can test a fix in Gutenberg by adding back the filter that never gets added in core due to the bug: |
Thanks for tracking this down, @adamsilverstein. I wasn't aware of the previous fix.
I'm not getting any errors when previewing posts without a thumbnail set, so probably title and content issues were unrelated. |
@walbo & @Mamaduka - regarding featured images in previews, I have a fix proposed on WordPress/wordpress-develop#2092. If you test this, it should show the correct featured image when changing and previewing. |
Left a comment on the core ticket. Tested and looks good to me. |
Thanks, @adamsilverstein. I also left the comment on the Trac ticket. I think we should also backport the fix. The plugin has to support WP 5.8 with Block Themes. |
When inside the main loop, we want to use a queried object to apply `the_preview` for the current post. We force this behavior by omitting the post ID argument from the `get_the_content` and `get_the_title.` Co-authored-by: Adam Silverstein <adamjs@google.com>
Hi, I've just came across this issue (checking @adamsilverstein's comment referrence) removing the In my theme I am using Post Title block also for displaying blog page title. I've decided for this so a user doesn't have to edit corresponding template (part) in Site Editor when user just wants to change the blog page title. For this I'm using Removing the Maybe I don't know all the reasoning, but in my opinion the context specific PHP functions should stay filterable/modifiable even in blocks. |
Hi, @webmandesign That behavior was a regression introduced in #48001, and it didn't make it a WP release. The expectation is that the Post Title will be used in Query Loop, where the loop block setups all required context and globals for the title block. I recently saw a request for a similar case - #52668. There's also a proposal to stop relying on post-globals and only use provided context - #45828. |
@Mamaduka Thanks for the info. With classic themes we could provide the context for the PHP functions. |
The Query Title block works outside the loop, used for displaying titles for archive and search pages. It uses |
Unfortunately, the Query Title block does not work for me outside the loop on the home page. I'm looking for something similar to the Query Title that uses |
Description
Resolves #37463.
The preview filter applies content from autosave to the queries object (and global post). When we pass a post ID to the
get_the_content
, it uses an instance of the post where those changes aren't applied. Usingnull
instead of$post_id
in this scenario fixes the issue.How has this been tested?
Using TT2 theme.
Test Query Loops
Additionally, check that main queries like index or archive page work as expected.
Screencast
CleanShot.2021-12-24.at.15.11.47.mp4
Types of changes
Bugfix
Checklist:
*.native.js
files for terms that need renaming or removal).