-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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 Comments block: hide the "Comments Closed" message #35743
Conversation
Size Change: +5.33 kB (0%) Total Size: 1.07 MB
ℹ️ View Unchanged
|
I was seeing a notice when the attribute wasn't set. I added a commit to fix that. |
I've been taking a look and it seems to be working fine! I think I am not the best person to review the code, but I checked how it works in the editor 🙂 .
Regarding this, there is still not progress on the Comments Template block, but this is for sure something to consider. I will post it there so it isn't missed. |
Updated the PR based on the suggestion and logic described here, so no CSS is needed. I also updated the original PR description. It should be ready for another review. |
Another thought — what if we got rid of the attribute altogether, and just returned early if comments are closed and there are no comments. |
Yeah, this seems like the right move to 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.
Thank you for the changes!
LGTM 👍
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.
This works well in my testing too. Should be good to merge once tests pass. 👍
Co-authored-by: Ari Stathopoulos <aristath@gmail.com>
e102ee4
to
0c19d32
Compare
Description
Solves #35732.
This PR changes the Post Comments rendering to hide the "Comments are closed" message when the following conditions are met:
How has this been tested?
Screenshots
Types of changes
New feature (non-breaking change which adds functionality)
Checklist:
*.native.js
files for terms that need renaming or removal).