-
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
Add prefix / suffix to Post Date block #61920
base: trunk
Are you sure you want to change the base?
Add prefix / suffix to Post Date block #61920
Conversation
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @TimBHowe! In case you missed it, we'd love to have you join us in our Slack community. If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information. |
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 exactly like one would expect and the code looks good to me.
I personally think this is a very valuable enhancement. If you try to accomplish the same thing today in a block theme you would need to add a paragraph before / after the post date block. But that comes with the downside of the label still getting rendered when the block is not getting rendered. Like in the case when the modified date is the same as the publish date.
Having said all that I definitely am looking for additional reviews from the core maintainers.
One downside of the prefix / suffix always is internationalization. What may be a prefix in one language is a suffix in another language etc.
Pinging @WordPress/gutenberg-core for reviews :)
return sprintf( | ||
'<div %1$s><time datetime="%2$s">%3$s</time></div>', | ||
'<div %1$s>%2$s<time datetime="%3$s">%4$s</time>%5$s</div>', |
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.
I think prefix and suffix would need esc_html
. A contributor user could add scripts there.
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.
It would need to be wo_kses_post
because the rich text string may contain formats such as bold / italics etc.
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.
Applied wp_kses_post
to the Prefix and Suffix for the Post Date block output 4d1ca75
Maybe we could achieve a similar solution in a future by using block bindings or bits (see #39831) instead of adding a prefix and a suffix. 🤔 Pinging @dmsnell or @SantosGuillamot get some opinions (if they are able to provide it, of course) |
To be honest, this one feels weird to me. Not because it's not valuable but because it sets a precedent. Is this the only block where you would need a prefix/suffix. There are two alternatives to this:
|
@youknowriad thats not technically true. There are other blocks that already use this approach: https://github.com/WordPress/gutenberg/blob/trunk/packages/block-library/src/post-terms/edit.js And the example you mention about using a row with a paragraph to achieve this is also sadly not really a good option. Like described above the issue there is that the modified date variation of the post data block doesn't render any output if the modified date is the same as the publish date. In that case the label you add via another block in a row before the post date still renders but the actual information doesn't render. We have that same issue with block bindings. And block pattern overrides. Not all data always exists. And currently there are no solutions for possibly empty fields which should get omitted entirely. Having said all that this doesn't mean I think this is the perfect approach. I'm not sure how to solve it. But the other solutions all have issues that this implementation here doesn't have. |
Thanks! Bits definitely are going to solve this need, as the current system is overly cumbersome and inconvenient. Working on a Make proposal as we speak, but it won't be here in WordPress 6.6 for sure. |
@fabiankaegy How would you achieve this design?
The solution in this PR is a bit misguided if this is the main argument for us because it achieves the result for a very small set of designs. I agree that this is a gap in our APIs and code but don't you think something like the Block visibility plugin would be a better approach for this kind of use-cases? |
@youknowriad yeah that is exactly why I didn't approve the PR but instead pinged the room. I don't think it is the right solution because it doesn't allow for these kinds of layouts without custom CSS. But I did want to raise it because it is a clear gap that I am seeing in many projects. And currently my solution on those projects is essentially adding a prefix via the render block filter in PHP. I think this PR showcases the gap in our current API well and I think it is a problem worth looking into :) |
Potentially related: #61508 |
Hello, i faced this situation this week on some projects, but it's a daily need. 1/ About prefix, i use a csss solution, just for the pleasure to test if it still works :-)
2/ Hide Created when Edited is available
3/ About internationalization, i fixed this with the Block Visiblity plugin also, and another css class (see 1/ via a dedicated class per locale)
Hope my feedback helps :-) |
I created an alternative solution to also review.
Hopefully this help, and let me know if there is anything else I can do to contribute |
This is the third block I believe that needs the prefix / suffix. All of these should be handled in a similar way. |
@vipul0425 and @t-hamano |
Does anyone know where there is a search/discussion going on about "Bits"? I remember seeing it but couldn't track it down... |
@dmsnell how confident are you in bits landing eventually? If it's "medium" or higher then I'd say let's hold off on this PR and its alternatives and wait for bits. |
@noisysocks I feel like Bits are inevitable, and while I don't know if I'd say that they will be fully in 6.7, I think some start of them will be. It's a broad-spanning project that essentially began when we introduced Blocks and proverbially kicked out Shortcodes. What's most likely to appear first is the ability for WordPress to find and render them, since this is all basically possible today. It can be sped up with help to push forward WordPress/wordpress-develop#6395, which is a fix to prevent Core from corrupting the syntax that is now almost definitely going to be the Bits syntax. |
For various blocks where it would be helpful to add prefix and a suffix could the default Pagination block Previews/Next Page feature be used? Do we need to wait for a totally new Bits syntax? When Bits is ready in perhaps 6-10 months perhaps one can convert over where the Previews/Net Page feature is used into using the new Bits syntax? |
There's now an official proposal for Bits, where I encourage we all participate to move the project forward. It's probably not too late to have the server preserve Bits syntax on save for 6.6, because the change necessary to do that is fixing an existing bug. If it's in, then the Post Date block can start exploring with others (such as the work @ellatrix is doing, or @SantosGuillamot with Block Bindings). |
What?
Add prefix / suffix to Post Date block
Why?
#47738
The Post Date block doesn't let you add a prefix or suffix. You can manually add a text before the block but you are using the Last Modified Date and it is empty then the text you added will appear with no date output.
This would allow using 2 Post Date blocks on the same post, which may solve #46645 :
How?
This pull request adds an optional prefix / suffix to Post Date, the same way the Category block does (see PR #40559).
For instance :
This would allow to use 2 Post Date blocks on the same post, which may solve #46645 :
Published on : 2 Febuary 2023
Last Modified : 3 Febuary 2023
Testing Instructions
Screenshots or screencast