[New Block] Add post time to read block #43403
Conversation
|
Size Change: +577 B (0%) Total Size: 1.33 MB
ℹ️ View Unchanged
|
t-hamano
force-pushed
the
add/block-post-time-to-read
branch
from
a9a92f8
to
06f27dd
Compare
t-hamano
changed the title
t-hamano
requested review from
spacedmonkey,
ajitbohra,
ryanwelcher,
juanmaguitar and
fabiankaegy
as code owners
|
That's a fun block! Thank you. I added it to the single post via the post editor.
For my test, I also wanted to use it on my home page with the post grid, now I got different values: On the front end is shows "less than a minute" which is obviously different from the 9 min on the single post.
There is also a missing 'in' there: "You can read this post in less than a minute" Maybe there is a way to get the value stored in the post_meta table so it can be used like any other post_meta information, like post_date, or post_author. |
t-hamano
added
the
Needs Technical Feedback
|
Thank you for the review, @bph !
Fixed 👍
I would like to hear from the developers if it is possible to add new meta information when saving the post and if this would have any significant impact. Also, when someone moves from the classic editor to Gutenberg, there will be no meta information initially. I will ask for feedback once in the editor chat. |
| 'You can read this post in %d minute.', | ||
| 'You can read this post in %d minutes.', |
There was a problem hiding this comment.
I think the text could be configurable, as it's unlikely to suit everyone. I'm not sure if there are any other blocks that have a way to configure text where there's a variable involved though. That seems like a challenge!
There was a problem hiding this comment.
I initially considered introducing a prefix and suffix like core/post-terms.
However, a common string would not cover all cases of singular and plural, or all cases where the time required is less than a minute.
I also considered the following sidebar setting, but this is complicated 🤔
There was a problem hiding this comment.
I thought the easiest way to start might be just a prefix, and making the unit of time an implicit part of the text that can't be changed. The only confusing thing would be how internationalization works. User entered text can't be localized at the moment, but the 'minute' or 'minutes' would be. 🤔
Lets see if we can get some assistance from a designer on this particular matter (cc @WordPress/gutenberg-design).
There was a problem hiding this comment.
Maybe the block should just print the time, then if you want a prefix/suffix you can place it in a Row alongside Paragraphs?
There was a problem hiding this comment.
Maybe the block should just print the time, then if you want a prefix/suffix you can place it in a Row alongside Paragraphs?
Yes, I think such an approach is possible.
However, given that the core/post-terms block has a prefix/suffix, I think the current approach makes sense. Also, if this block displayed only time, adding a prefix or suffix would create multiple paragraphs. I think it should consist of a single paragraph, including prefixes and suffixes.
There was a problem hiding this comment.
Good point.
The only confusing thing would be how internationalization works. User entered text can't be localized at the moment
If the prefix/suffix had a placeholder, would that be localized? If so I believe that would solve any issues?
There was a problem hiding this comment.
If the prefix/suffix had a placeholder, would that be localized? If so I believe that would solve any issues?
Yes, the prefix/suffix placeholder will be localized as you can see here.
However, the prefix/suffix with text entered are stored as attributes and aren't localized.
Therefore, for example, if you enter prefix / suffix in English and switch to a language other than English, only the time will be localized and the correct text will not be generated.
In early versions, there was an implementation to make all paragraphs subject to localization, without prefix/suffix. However, this would be inconvenient because the user wouldn't be able to change the text. (This may be solved by introducing filter hooks.)
|
Thank you for the review, @talldan !
Yes, I think this is a major point. I think it would be ideal to implement this block as a dynamic block and work in a template (or in a query loop block). However, as far as I know, there is no function like If it were to be implemented in PHP, I think the following step would be a good idea, what do you think?
|
Yep, something like that sounds great. You can implement the function in the An alternative is to implement everything in the block's index.php file, but one thing to note is that the backporting process is different. The php files in the block library package are automatically copied to the core codebase, there's no manual backport. Because of that you won't be able to use the |
|
Thank you for the review, @talldan !
Yes, this is probably the biggest problem here. |
|
This is a fun little utility block! I took it for a spin:
It works well. The crash happens most easily if you select the block and press Enter, though it happened also seemingly a bit at random. Got this in the console: The icon looks pretty good. I suspect a clock is useful enough that more blocks, or even parts of the UI could have this. But since it seems appropriate for the block at hand, let's use it! You can probably add it to the icon library, though. The metrics were pretty good, but here's a new version that matches similar circle icons: Let me know if that works for you. Question regarding this conversation: instead of including the full text of "This post can be read in n minutes", which runs into translation and theoretically even post type challenges, could we literally output only the time to read? I.e.
Perhaps even with a toggle to choose between the full On the other hand, that appears to be partially handled by prefix/suffixes also (#40559), so perhaps that route is the way to go after all? Something to consider. |
|
I too received a "This block has encountered an error and cannot be previewed." error. |
|
Thanks for the feedback, @jasmussen, @paaljoachim! @talldan gutenberg/packages/edit-site/src/components/code-editor/index.js Lines 36 to 47 in af5802a Both the post editor and the site editor will retrieve the content correctly. |
|
@jasmussen
|
|
I have added the prefix and suffix.
At least in English, I believe that any variation can produce the correct sentence. I think there might be no problem if this text is properly translated: gutenberg/packages/block-library/src/post-time-to-read/edit.js Lines 91 to 96 in 5221b9d Without prefix and suffix
With prefix and suffix
|
t-hamano
changed the title
t-hamano
changed the title
|
@talldan Perhaps the original logic is here: This is probably what |
|
@bph |
|
Retesting using I see this:
Prefix "less than a minute" Suffix. Should Suffix be with a small s? I added a prefix of "It will take" and the Suffix "to read this text." Result on the frontend. Backend text says: Frontend text says: The same backend text should also be seen on the frontend. |
t-hamano
force-pushed
the
add/block-post-time-to-read
branch
from
74cbb73
to
39a840b
Compare
|
I have fixed the lint error. It was simply an omission to update |
|
Do you think this is ready to merge now @t-hamano? |
|
@talldan ✅ Merged the latest trunk and it is working as expected However, I found that wp_get_word_count_type(), introduced in WordPress 6.2, belongs to the
|
|
Thanks for all your work on this, tests are green so I'll merge it 👍 |
|
I see that it was merged into the milestone 5.3 but I can't find it in the 5.3 RC1 changelog. Does this need to be backported to the Gutenberg RC or am I overlooking something? @richtabor @talldan ? |
|
@bph
|
|
Thank you so much! @t-hamano |
|
@t-hamano thank you for all the hard work going through the various iterations. It's a great block. |
|
I have opened a ticket to add the |
Close #42542
Note: The latest status is summarized in this comment.
What?
This PR adds a new "Post Time To Read" block.
For now, I'm keeping implementation to a minimum. If it makes sense to incorporate this block, I would like to work on additional implementation.
Why?
With #41611, "Time to read" has been added to the Detail panel.
By using the same information in the block, the site owner can tell visitors in advance how long it will take to finish reading.
Implementation Details
Calculation of time to read
The time required indicated by the block must match the "Time to read" in the Details panel. This PR uses the same
@wordpress/wordcountpackage to calculate.The
countmethod of this package is very powerful and supports word-by-word or character-by-character calculations. The calculation criteria can be controlled by the translation function, so the time required can be calculated appropriately according to the site language:gutenberg/packages/block-library/src/post-time-to-read/edit.js
Lines 39 to 44 in 0161182
This block stores only the calculated time (minutesToRead) in the attribute and outputs the appropriate string through dynamic rendering.@update: Time is no longer stored as an attribute since time is now computed by dynamic rendering.
Why dynamic rendering?When the block was saved as static markup by using__(), the block could be broken if the site language is changed, as mentioned in #43050.Therefore, We need to use the translation function on the server side.About demeritThe biggest disadvantage of this implementation is that time is stored as attributes. The time displayed on the front end is updated only when the post is saved.This means that it will not work properly in templates such assingle.htmlor in query loops. This issue is similar to what was mentioned in #29739 when the TOC block was rebuilt with static markup.@update: Time is now fully computed by dynamic rendering. Therefore, it works not only in the post editor, but also in the site editor templates.
Testing Instructions
First, try using this block on gutenberg.run.
http://gutenberg.run/43403
Next steps and suggestions
If it makes sense to bundle this block, I would like to proceed with this PR according to the following steps and suggestions.
Add E2E tests
We will need to check that the correct attributes (
minutesToRead) have been saved after inserting content of any length.Add block supports
As discussed in #43241, we need to add all the necessary block support to address Design Tooling Consistency.
Add filter hook
The time string should be displayed appropriately depending on the language of the site, but it might make sense to provide hooks such as the following:
✅ Create a block icon (Done)
I could not find a suitable icon from the
@wordpress/iconspackage, so I created a temporary icon.Need help from designers to improve this icon or create a completely new icon 🙏
@update: As this comment states, @jasmussen provided the icons 🎉
✅ Server-side time calculation (Done)
As mentioned earlier, this block will only work on a single post.
Many consumers will probably want to place the block in a template such as
single.html. To do so, I think we would need to implement the exact same logic on the server side as@wordpress/countdoes.Screenshots or screencast