Server side renderer: Fix errors in template part editor context #29246
Conversation
This file contains 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
david-szabo97
added
[Type] Bug
david-szabo97
requested review from
mattwiebe,
youknowriad,
vindl,
Copons,
jeyip and
Addison-Stavlo
|
Size Change: +12 B (0%) Total Size: 1.42 MB
ℹ️ View Unchanged
|
david-szabo97
changed the title
wp_id instead of id if wp_id is available
|
@youknowriad Do you think there is a better way to handle this? 🤔 |
|
Do you know how this post id is used server side by the server side render endpoint? It seems the best solution here is to update that endpoint instead? |
|
Or may just don't pass the "id" in these situations because a "template" or "template part" is not really a post. |
|
It uses the I think it's safe to omit the I don't like that we need to add this |
|
@jeyip You need to directly edit a template part. Appearance -> Template Parts -> Edit a template part |
Yeah I had tried that previously as well without luck. My mistake for not posting a GIF of it. I'll try again and capture a video of the results. |
|
Still no luck reproducing it 😞
|
|
Looks like "Navigation block > Page Link" doesn't use ServerSideRender anymore. Other blocks are still failing like RSS and "Navigation block > Page List". Any block will fail that use ServerSideRender:
(Updated PR description) |
There was a problem hiding this comment.
Tested, and all works as expected!
I was able to repro the current issue with RSS blocks in the stand alone template and template part post editors. This PR fixes the issues.
I think this fix is simple enough and a fair solution, but I think we should at least comment as to why we are doing this as it is a bit unexpected for those unfamiliar to FSE templates/parts entities.
TestingLike Addie, I found that everything worked as expected once I reproduced the issue 🙂 Behavior
Requirements
|
david-szabo97
force-pushed
the
fix/sever-side-renderer-in-template-part-editor
branch
from
86c565e to
2a7193e
Compare
david-szabo97
force-pushed
the
fix/sever-side-renderer-in-template-part-editor
branch
from
2a7193e to
a81d4ff
Compare
Do you happen to know examples of blocks that would fail today? Not saying this has to be done immediately, but I'm wondering if there's some sort of design we could implement to handle this situation gracefully, similar to how templates selected from the FSE navigation menu will render content placeholders. I might be off-base with this though, so would love to hear everyone's thoughts. |
I'm not a big fan of this solution either. Using two separate ids is quite confusing, but with that being said, I think that the inline comment helps.
There were some alternative approaches mentioned by @youknowriad.
+1 from me whenever we resolve the discussions around approach # 2 |
"Might" 😄 So far every server side rendered block seem to work fine. |
|
@david-szabo97 Nice! I'll re-test this in a bit. |
|
Re-tested functionality of the calendar, latest comments, and page list blocks from Appearance -> Template Parts -> Edit a template part. Everything works as expected 🎉 |
jeyip
approved these changes
Mar 31, 2021
Addison-Stavlo
approved these changes
Apr 5, 2021
david-szabo97
deleted the
fix/sever-side-renderer-in-template-part-editor
branch
This was referenced May 13, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
Fixes #29237
Fixes #29236
In #27910 we introduced a custom ID for template and template parts. Server side renderer tried to use these IDs but failed since they aren't real post IDs. To fix this, this PR tries to use
wp_idif possible, otherwise fallback toid.How has this been tested?
Types of changes
Bug fix (non-breaking change which fixes an issue)
Checklist: