Post Comment: Handle the case where a comment does not exist #35810
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
gziolo
added
[Type] Code Quality
|
Size Change: +51 B (0%) Total Size: 1.07 MB
ℹ️ View Unchanged
|
gziolo
force-pushed
the
fix/no-comment-found-blocks
branch
from
25b06cb to
06aee5b
Compare
gziolo
force-pushed
the
fix/no-comment-found-blocks
branch
from
06aee5b to
5fecab9
Compare
gziolo
commented
Oct 21, 2021
| const formatOptions = Object.values( settings.formats ).map( | ||
| ( formatOption ) => ( { | ||
| key: formatOption, | ||
| name: dateI18n( formatOption, date ), | ||
| name: dateI18n( formatOption, date || new Date() ), |
There was a problem hiding this comment.
This one was tricky. When there is no valid comment, then we have to provide a fallback value.
|
I don't see the "Comment with invalid ID" like in your video on the frontend. But I also do not see the blocks at all – except for the empty div, which is what I'd expect. I just wanted to double check that was the desired result before approving? |
This is a group and heading I added so I could locate the div on the page 😄 I guess it's all good. I discussed it with @ntsekouras and it feels like the Post Comment as it exists today isn't very useful for theme builders or site owners. We had hard time find a good use case where someone would like to pick a specific comment to display on the page. Even if that would be the case, in the future it could be achieved with the Comments Loop block that is being developed in #35231. What I mean by that is that, the most important thing is that when the comment has broken data, then all nested blocks should render nothing preventing any errors. The only remaining challenge is the case when there is no comments on the site, that will make it difficult to configure the Comment Template block by using placeholders, but I guess it's something we can leave for later. |
|
Oh, I didn't notice that haha.
I think it's probably okay. This is kind of the same as editing a post template where there's no reference content. |
jameskoster
approved these changes
Oct 21, 2021
|
Looks good! Let's mention #35396 and #30671, as they need be updated to have the same behaviour.
Could be a solution to show a message "There are no comments on the site or comments are disabled" on the screen of choosing the comment ID? |
cbravobernal
approved these changes
Oct 21, 2021
Yes and no, it depends 😅 When you prepare a new site and there is no comments it will prevent you from applying modification to the template for a single comment. On the other hand, it isn't that much work to create some dummy comments to remove this limitation 🤷🏻 |
fullofcaffeine
added a commit
that referenced
this pull request
Oct 21, 2021
* trunk: (494 commits) remove consecutive rc warning (#35855) Update Changelog for 11.8.0-rc.2 Bump plugin version to 11.8.0-rc.2 [RNMobile] Disable React Native E2E Tests (iOS) (#35844) Add section about using the schema during development (#35835) Add a method to disable auto-accepting dialogs (#35828) Wrap NavigationContainer with SafeAreaView. (#35570) Update Appium to 1.22.0 (#35829) Post Comment: Handle the case where a comment does not exist (#35810) Clear selected block when clicking on the gray background (#35816) Post excerpt: Don't print the wrapper when there is no excerpt (#35749) [Block] Navigation: Fix padding for social links on mobile (#35824) Fix issue with responsive navigation causing wrapping. (#35820) [Block Editor]: Fix displaying only `none` alignment option (#35822) Add API to access global settings, styles, and stylesheet (#34843) Mobile Release v1.64.1 (#35804) Add resizer to template part focus mode (#35728) Update Changelog for 11.7.1 Gallery block: Only show the gallery upload error message if mixed multiple files uploaded (#35790) Update Changelog for 11.8.0-rc.1 ...
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
There were some issues with how the Post Comment block worked when it has a comment id selected that doesn't exist.
Screen.Recording.2021-10-20.at.12.57.33.mov
Implementation details
The goal of this PR is to ensure that the following blocks print nothing on the frontend when an invalid
There is still going to be an empty
divprinted by the Post Comment block on the fronted. We can address that later.I used the implementation for comments from WordPress core as a reference:
https://github.com/WordPress/wordpress-develop/blob/master/src/wp-includes/comment-template.php
The idea is to use
get_commentin PHP callback to verify if the comment exist and bail out early otherwise.Open question
We should still discuss what happens on the fronted when an invalid comment is selected. At the moment, we render block tittles as placeholders. We also keep all controls in place even though some of them can't be really exercised like the format of the date or linking for the author's display name.
How has this been tested?
Screenshots
Screen.Recording.2021-10-21.at.09.49.49.mov
Types of changes
Bug fix (non-breaking change which fixes an issue).
Checklist:
-->